[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


  • To: Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Tue, 23 Nov 2021 09:19:27 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=DlYhhnkxV5+rn3Am7ZGdO/r2/K+pzQACH2qYqOI0kYE=; b=guBXJ+7Z/1oEZgRM2gmG8n7RSkWDQMOftNh3793lr7PeTJENn2Jvt85IE8EpDy2mE5L1qQdWw3z3aGsQofrB8JwgIKMncq/QRt8gVaUVC2NOYkyijDxG68TNCS+qNUIgrGigEkXrZuMOgjwCrePKsuE58sJUPoFCVqpRmBeWauibefNRb2+T5A7Y/+busNSrDbylo/xVts74hRuhRN5+IGxKhxim3e7Y4enBOyeD9EvnFvUDPpLWpHTjnxZJ5QmfB4YWCL56UfOrPTglPjrxAMOgEN9mf/WuOtz0BpMOkbALd8l/D1UFN69K0jVsBd7dxBithc0DCrfHwLFTu6j4TA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=bL59AMrwCazS3PIenXaoybmhrElS2pbQ7yGA1GpAZQBc4snt+1Wbt/d9rCVkNR/u0n7PM7kfvrHPE9SZQwbtHAH8dKn0GEKmU/zZhOy5jUxBVzs7yGsdbFr6iu7R3mNxU5jKvi4LyrSKu859yGa+pwvLkGvsZX4jtj33Qe39VHsa4IjGuSdoaXzWaTnQoVHo0UTtvEReghr/R+NBFdzfBkfzt/Jm7P9Wec3rZVrVo73kvGcbb2zXZas5xisc5x7GhU0KAeiwdjeqP7B4ruLDHfncdkHsdUm4w4IXAb4NtCgL2rZmjB9xLDw73mFOBicJqJmRkMCjVLaPWSnaHhmPOQ==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Julien Grall <julien.grall.oss@xxxxxxxxx>, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Chen <Wei.Chen@xxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Rahul Singh <Rahul.Singh@xxxxxxx>
  • Delivery-date: Tue, 23 Nov 2021 09:19:57 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHX3WXgMxQOWtsAQkCagYrL+eVmYKwLGnaAgASCuICAAF76gIAACHQAgACBjgCAAE4VAIAAB2AA
  • Thread-topic: [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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.