Re: [PATCH 5/6] target/arm: Implement FEAT_LPA

[Peter Maydell]

On Wed, 8 Dec 2021 at 23:15, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Could you put some content in the commit message, please?
A brief description of what FEAT_LPA is would be helpful,
for instance.

While I was rereading the Arm ARM to review this I noticed
an related check that I think we're missing. In section
D5.2.7 the text describes which levels of translation
tables support Block descriptors. (This varies depending on
whether FEAT_LPA/FEAT_LPA2 are supported, which is why I
ran into it.) e.g for 64KB granule size Block descriptors
are OK in level 2, and if (FEAT_LPA && 52 bit PA size supported)
also in level 1. Finding a block descriptor at a level it
should not be is supposed to cause a level 1 Translation fault.
As far as I can tell we don't check for this, unless it
falls out in the wash in some super-subtle way...

That seems like an unrelated bug to be fixed in a
separate patch (though it does entangle with the LPA stuff
because that will change the conditions for the check).

Side note that might be worth mentioning in the
commit message: FEAT_LPA also extends the nominal field
size of addresses held in the fault registers HPFAR_EL2
and PAR_EL1. Our implementation doesn't explicitly mask
out the high bits of addresses when filling out those
fields, so it doesn't need changes to handle them
expanding to hold bits [51:48] of PAs and IPAs.

For the code:
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM