Re: [Xen-devel] [PATCH v7 13/14] arm/mem_access: Add short-descriptor based gpt

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

On 08/08/2017 05:18 PM, Julien Grall wrote:
>
>
> On 08/08/17 16:17, Sergej Proskurin wrote:
>> Hi Julien,
>>
>>
>> On 07/18/2017 02:25 PM, Sergej Proskurin wrote:
>>> This commit adds functionality to walk the guest's page tables using
>>> the
>>> short-descriptor translation table format for both ARMv7 and ARMv8. The
>>> implementation is based on ARM DDI 0487B-a J1-6002 and ARM DDI 0406C-b
>>> B3-1506.
>>>
>>> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
>>> Acked-by: Julien Grall <julien.grall@xxxxxxx>
>>
>> As you have already Acked this patch I would like you to ask whether I
>> should remove your Acked-by for now as I have extended the previous
>> patch by additional casts of the pte.*.base fields to (paddr_t) as
>> discussed in patch 00/14.
>
> I am fine with this, assuming this is the only change made.

The changes are limited to 4 similar casts to (paddr_t) in total and an
additional comment. Here are the only changes in this patch:

diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
index b258248322..7f34a2b1d3 100644
--- a/xen/arch/arm/guest_walk.c
+++ b/xen/arch/arm/guest_walk.c
@@ -112,7 +112,12 @@ static int guest_walk_sd(const struct vcpu *v,
          * level translation table does not need to be page aligned.
          */
         mask = GENMASK(19, 12);
-        paddr = (pte.walk.base << 10) | ((gva & mask) >> 10);
+        /*
+         * Cast pte.walk.base to paddr_t to cope with C type promotion
of types
+         * smaller than int. Otherwise pte.walk.base would be casted to
int and
+         * subsequently sign extended, thus leading to a wrong value.
+         */
+        paddr = ((paddr_t)pte.walk.base << 10) | ((gva & mask) >> 10);

         /* Access the guest's memory to read only one PTE. */
         ret = access_guest_memory_by_ipa(d, paddr, &pte,
sizeof(short_desc_t), false);
@@ -125,7 +130,7 @@ static int guest_walk_sd(const struct vcpu *v,
         if ( pte.pg.page ) /* Small page. */
         {
             mask = (1ULL << L2DESC_SMALL_PAGE_SHIFT) - 1;
-            *ipa = (pte.pg.base << L2DESC_SMALL_PAGE_SHIFT) | (gva & mask);
+            *ipa = ((paddr_t)pte.pg.base << L2DESC_SMALL_PAGE_SHIFT) |
(gva & mask);

             /* Set execute permissions associated with the small page. */
             if ( !pte.pg.xn )
@@ -134,7 +139,7 @@ static int guest_walk_sd(const struct vcpu *v,
         else /* Large page. */
         {
             mask = (1ULL << L2DESC_LARGE_PAGE_SHIFT) - 1;
-            *ipa = (pte.lpg.base << L2DESC_LARGE_PAGE_SHIFT) | (gva &
mask);
+            *ipa = ((paddr_t)pte.lpg.base << L2DESC_LARGE_PAGE_SHIFT) |
(gva & mask);

             /* Set execute permissions associated with the large page. */
             if ( !pte.lpg.xn )
@@ -152,7 +157,7 @@ static int guest_walk_sd(const struct vcpu *v,
         if ( !pte.sec.supersec ) /* Section */
         {
             mask = (1ULL << L1DESC_SECTION_SHIFT) - 1;
-            *ipa = (pte.sec.base << L1DESC_SECTION_SHIFT) | (gva & mask);
+            *ipa = ((paddr_t)pte.sec.base << L1DESC_SECTION_SHIFT) |
(gva & mask);
         }
         else /* Supersection */
         {

Thanks,
~Sergej

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel