[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC PATCH] Added the logic to decode 32 bit ldr/str post-indexing instructions
Hi, > On 23 Nov 2021, at 08:53, Bertrand Marquis <bertrand.marquis@xxxxxxx> wrote: > > 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. I did some checks and I think the following cases could be handled: ldr x2, [x1], #4 nop ldr w2, [x1], #-4 nop ldrh w2, [x1], #8 nop ldrb w2, [x1], #16 nop str x2, [x1], #4 nop str w2, [x1], #-4 nop strh w2, [x1], #8 nop strb w2, [x1], #16 nop Anything that I could have missed ? > > There are also post instructions with shifting but to be honest I do not > think this is really needed. Please ignore this, there is no post shifting. Once this is done I can test and add a test to XTF on arm (on our side, upstreaming of this is in progress) to make sure this is maintained. Regards Bertrand
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |