Re: [RFC PATCH] Added the logic to decode 32 bit ldr/str post-indexing instructions

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Stefano,

> On 23 Nov 2021, at 04:13, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> 
> On Mon, 22 Nov 2021, Julien Grall wrote:
>> On Mon, 22 Nov 2021 at 19:59, Stefano Stabellini <sstabellini@xxxxxxxxxx> 
>> wrote:
>>> 
>>> On Mon, 22 Nov 2021, Ayan Kumar Halder wrote:
>>>> Stefano > It doesn't look like we are setting dabt->write anywhere.
>>>> 
>>>> Ayan > Yes, this is a miss. Depending on the opc, this should be upadeted
>>>> accordingly in decode_64bit_loadstore_postindexing().
>>>> 
>>>> Stefano > Also, is info.gpa in try_handle_mmio already updated in the
>>>> pre-index
>>>> case? If not, do we need to apply the offset manually here?
>>>> 
>>>> Ayan > Sorry, I did not understand you. This patch is only related to the 
>>>> post
>>>> indexing ldr/str issue. Why do we need to check for pre-indexing ?
>>> 
>>> I thought you were trying to handle both post-indexing and pre-indexing.
>>> It is OK if you intend to only handle post-indexing but considering that
>>> most of the code is shared between the two, we might as well also make
>>> pre-indexing work (unless it turns out it is more difficult).
>> 
>> Wouldn't this effectively be dead code?

I agree this would be dead code. Pre-indexing is handled by the hardware, only 
post are not.

>> 
>>> 
>>> In the pre-indexing case, I would imagine we need to update the base
>>> address before taking any other actions.
>> 
>>> From my understanding, this would have already been performed by the
>> HW when the syndrome is valid. This may also be the case for
>> the non-valid case, but I haven't checked the Arm Arm.
> 
> It is not clear to me either, that's why I wrote "I would imagine"... I
> was guessing that it is not done by the HW in the non-valid case but I
> don't know.

This should be handled by the hardware here, so only post actions should
be handled here.

> 
> Of course, if it is already done by the HW, that's all the better: no
> need for us to do anything.

I am wondering though if other types of accesses could not be handled here
without major modification of the code like other sizes then 32bit.

There are also post instructions with shifting but to be honest I do not think 
this is really needed.

Regards
Bertrand