[Qemu-devel] [PATCH] target-arm: use extended address bits from supersection short descriptor

[Qemu-devel] [PATCH] target-arm: use extended address bits from supersection short descriptor

[Sergey Fedorov]

---
 target-arm/helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5d0f011..fc3f192 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5333,6 +5333,8 @@ static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type,
         if (desc & (1 << 18)) {
             /* Supersection.  */
             phys_addr = (desc & 0xff000000) | (address & 0x00ffffff);
+            phys_addr |= ((uint64_t)extract32(desc, 20, 4) << 32);
+            phys_addr |= ((uint64_t)extract32(desc, 5, 4) << 36);
             *page_size = 0x1000000;
         } else {
             /* Section.  */
-- 
1.9.1

Re: [Qemu-devel] [PATCH] target-arm: use extended address bits from supersection short descriptor

[Peter Maydell]

On 28 May 2015 at 12:00, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
> ---
>  target-arm/helper.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 5d0f011..fc3f192 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5333,6 +5333,8 @@ static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type,
>          if (desc & (1 << 18)) {
>              /* Supersection.  */
>              phys_addr = (desc & 0xff000000) | (address & 0x00ffffff);
> +            phys_addr |= ((uint64_t)extract32(desc, 20, 4) << 32);
> +            phys_addr |= ((uint64_t)extract32(desc, 5, 4) << 36);
>              *page_size = 0x1000000;
>          } else {
>              /* Section.  */

This looks OK codewise, but I can't apply it because you've
forgotten the signed-off-by line.

Can you also provide a useful commit message, please?
Patches should pretty much always have a long-form
commit message, except possibly when they're part of
a large patch series, and rarely even then. (For instance
in this case it's worth mentioning that supporting these
extra bits is IMPDEF in v6 and in v7-without-LPAE but
required in LPAE.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] target-arm: use extended address bits from supersection short descriptor

[Peter Crosthwaite]

On Thu, May 28, 2015 at 4:00 AM, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
> ---
>  target-arm/helper.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 5d0f011..fc3f192 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5333,6 +5333,8 @@ static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type,
>          if (desc & (1 << 18)) {
>              /* Supersection.  */
>              phys_addr = (desc & 0xff000000) | (address & 0x00ffffff);
> +            phys_addr |= ((uint64_t)extract32(desc, 20, 4) << 32);
> +            phys_addr |= ((uint64_t)extract32(desc, 5, 4) << 36);

Outermost parentheses not needed.

Regards,
Peter

>              *page_size = 0x1000000;
>          } else {
>              /* Section.  */
> --
> 1.9.1
>
>

Re: [Qemu-devel] [PATCH] target-arm: use extended address bits from supersection short descriptor

[Sergey Fedorov]

On 01.06.2015 20:02, Peter Maydell wrote:
> On 28 May 2015 at 12:00, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
>> ---
>>  target-arm/helper.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index 5d0f011..fc3f192 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -5333,6 +5333,8 @@ static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type,
>>          if (desc & (1 << 18)) {
>>              /* Supersection.  */
>>              phys_addr = (desc & 0xff000000) | (address & 0x00ffffff);
>> +            phys_addr |= ((uint64_t)extract32(desc, 20, 4) << 32);
>> +            phys_addr |= ((uint64_t)extract32(desc, 5, 4) << 36);
>>              *page_size = 0x1000000;
>>          } else {
>>              /* Section.  */
> This looks OK codewise, but I can't apply it because you've
> forgotten the signed-off-by line.
>
> Can you also provide a useful commit message, please?
> Patches should pretty much always have a long-form
> commit message, except possibly when they're part of
> a large patch series, and rarely even then. (For instance
> in this case it's worth mentioning that supporting these
> extra bits is IMPDEF in v6 and in v7-without-LPAE but
> required in LPAE.)
>
> thanks
> -- PMM
Thanks, Peter. Your comments are very useful, as always :) Sorry for
forgetting sign-off-by. Will resend the patch soon!

Best,
Sergey

Re: [Qemu-devel] [PATCH] target-arm: use extended address bits from supersection short descriptor

[Sergey Fedorov]

On 01.06.2015 21:41, Peter Crosthwaite wrote:
> On Thu, May 28, 2015 at 4:00 AM, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
>> ---
>>  target-arm/helper.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index 5d0f011..fc3f192 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -5333,6 +5333,8 @@ static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type,
>>          if (desc & (1 << 18)) {
>>              /* Supersection.  */
>>              phys_addr = (desc & 0xff000000) | (address & 0x00ffffff);
>> +            phys_addr |= ((uint64_t)extract32(desc, 20, 4) << 32);
>> +            phys_addr |= ((uint64_t)extract32(desc, 5, 4) << 36);
> Outermost parentheses not needed.

Thanks, Peter. Will fix it v3.

Best,
Sergey

>
> Regards,
> Peter
>
>>              *page_size = 0x1000000;
>>          } else {
>>              /* Section.  */
>> --
>> 1.9.1
>>
>>