locore.S - two cosmetic changes

[Antonin Houska]

Kristof Provost <kp at FreeBSD.org> wrote:

> On 26 Oct 2020, at 7:23, Antonin Houska wrote:
> > Hello,
> >
> > while trying to understand the code, I've spotted two things that might need
> > to be improved - please see the attachments.
> >
> > remove_useless_assignment.patch removes setting of register value which is
> > not
> > used until the next value is set. use_constant_not_literal.patch probably
> > needs no explanation.
> >
> That’s gone in as in r367078. Thanks!

Thanks, this is my first contribution to the FreeBSD code :-)

Actually Mitchell Horne replied to my patch too but probably forgot to CC the
mailing list. He suggested that some more replacements of literals by
preprocessor constants can be included in the patch. While thinking about
that, I came across something that I don't understand.

On line 156, the position of the L1 entry for particular virtual address is
computed:

151:    /* Create an L1 page for early devmap */
152:    lla	s1, pagetable_l1
153:    lla	s2, pagetable_l2_devmap	/* Link to next level PN */
154:    srli	s2, s2, PAGE_SHIFT
155:
156:    li	a5, (VM_MAX_KERNEL_ADDRESS - L2_SIZE)

However the corresponding L2 entry is at position 510:

183:    /* Store PTE entry to position */
184:    li	a6, PTE_SIZE
185:    li	a5, 510

I'd expect 511 since the virtual addres is (1 * L2_SIZE) below
VM_MAX_KERNEL_ADDRESS. On the other hand, the (obviously related) virtual
address passed to initriscv() is

249:    li	t0, (VM_EARLY_DTB_ADDRESS)

defined in vmparam.h as

#define	VM_EARLY_DTB_ADDRESS	(VM_MAX_KERNEL_ADDRESS - (2 * L2_SIZE))

So I wonder if the assignment on line 156 should rather be

156:    li	a5, (VM_EARLY_DTB_ADDRESS)

I suspect that the (VM_MAX_KERNEL_ADDRESS - L2_SIZE) expression is a thinko
which does not break anything because

(VM_MAX_KERNEL_ADDRESS - L2_SIZE) >> L1_SHIFT

appears to be equal to

VM_EARLY_DTB_ADDRESS >> L1_SHIFT

Do I miss anything?

-- 
Antonín Houska
www.melesmeles.cz