1. Patchew
  2. Libvirt
  3. [PATCH 00/19] hw/microblaze: Allow running cross-endian vCPUs
  4. View patch

[PATCH 07/19] hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit

Philippe Mathieu-Daudé posted 19 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH 07/19] hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit
Posted by Philippe Mathieu-Daudé 1 year, 3 months ago 
All these MemoryRegionOps read() and write() handlers are
implemented expecting 32-bit accesses. Clarify that setting
.impl.min/max_access_size fields.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/xilinx_uartlite.c | 4 ++++
 hw/intc/xilinx_intc.c     | 4 ++++
 hw/net/xilinx_ethlite.c   | 4 ++++
 hw/timer/xilinx_timer.c   | 4 ++++
 4 files changed, 16 insertions(+)

diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
index a69ad769cc4..892efe81fee 100644
--- a/hw/char/xilinx_uartlite.c
+++ b/hw/char/xilinx_uartlite.c
@@ -170,6 +170,10 @@ static const MemoryRegionOps uart_ops = {
     .read = uart_read,
     .write = uart_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
     .valid = {
         .min_access_size = 1,
         .max_access_size = 4,
diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index 2b8246f6206..1762b34564e 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
@@ -144,6 +144,10 @@ static const MemoryRegionOps pic_ops = {
     .read = pic_read,
     .write = pic_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 11eb53c4d60..ede7c172748 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -170,6 +170,10 @@ static const MemoryRegionOps eth_ops = {
     .read = eth_read,
     .write = eth_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index 0822345779c..28ac95edea1 100644
--- a/hw/timer/xilinx_timer.c
+++ b/hw/timer/xilinx_timer.c
@@ -193,6 +193,10 @@ static const MemoryRegionOps timer_ops = {
     .read = timer_read,
     .write = timer_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
-- 
2.45.2
Re: [PATCH 07/19] hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit
Posted by Philippe Mathieu-Daudé 1 year, 3 months ago 
On 5/11/24 14:04, Philippe Mathieu-Daudé wrote:
> All these MemoryRegionOps read() and write() handlers are
> implemented expecting 32-bit accesses. Clarify that setting
> .impl.min/max_access_size fields.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/char/xilinx_uartlite.c | 4 ++++
>   hw/intc/xilinx_intc.c     | 4 ++++
>   hw/net/xilinx_ethlite.c   | 4 ++++
>   hw/timer/xilinx_timer.c   | 4 ++++
>   4 files changed, 16 insertions(+)
> 
> diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
> index a69ad769cc4..892efe81fee 100644
> --- a/hw/char/xilinx_uartlite.c
> +++ b/hw/char/xilinx_uartlite.c
> @@ -170,6 +170,10 @@ static const MemoryRegionOps uart_ops = {
>       .read = uart_read,
>       .write = uart_write,
>       .endianness = DEVICE_NATIVE_ENDIAN,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
>       .valid = {
>           .min_access_size = 1,
>           .max_access_size = 4,

To have qtests working I need to squash:

-- >8 --
diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index 3b92fa5d506..6d9291c8ae2 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -57,7 +57,7 @@ static const uint8_t kernel_pls3adsp1800[] = {
      0xb0, 0x00, 0x84, 0x00,                 /* imm   0x8400 */
      0x30, 0x60, 0x00, 0x04,                 /* addik r3,r0,4 */
      0x30, 0x80, 0x00, 0x54,                 /* addik r4,r0,'T' */
-    0xf0, 0x83, 0x00, 0x00,                 /* sbi   r4,r3,0 */
+    0xf8, 0x83, 0x00, 0x00,                 /* swi   r4,r3,0 */
      0xb8, 0x00, 0xff, 0xfc                  /* bri   -4  loop */
  };

@@ -65,7 +65,7 @@ static const uint8_t kernel_plml605[] = {
      0xe0, 0x83, 0x00, 0xb0,                 /* imm   0x83e0 */
      0x00, 0x10, 0x60, 0x30,                 /* addik r3,r0,0x1000 */
      0x54, 0x00, 0x80, 0x30,                 /* addik r4,r0,'T' */
-    0x00, 0x00, 0x83, 0xf0,                 /* sbi   r4,r3,0 */
+    0x00, 0x00, 0x83, 0xf8,                 /* swi   r4,r3,0 */
      0xfc, 0xff, 0x00, 0xb8                  /* bri   -4  loop */
  };
---

to access the uart by 32-bit instead of 8-bit.
Re: [PATCH 07/19] hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit
Posted by Philippe Mathieu-Daudé 1 year, 3 months ago 
On 5/11/24 23:24, Philippe Mathieu-Daudé wrote:
> On 5/11/24 14:04, Philippe Mathieu-Daudé wrote:
>> All these MemoryRegionOps read() and write() handlers are
>> implemented expecting 32-bit accesses. Clarify that setting
>> .impl.min/max_access_size fields.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/char/xilinx_uartlite.c | 4 ++++
>>   hw/intc/xilinx_intc.c     | 4 ++++
>>   hw/net/xilinx_ethlite.c   | 4 ++++
>>   hw/timer/xilinx_timer.c   | 4 ++++
>>   4 files changed, 16 insertions(+)
>>
>> diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
>> index a69ad769cc4..892efe81fee 100644
>> --- a/hw/char/xilinx_uartlite.c
>> +++ b/hw/char/xilinx_uartlite.c
>> @@ -170,6 +170,10 @@ static const MemoryRegionOps uart_ops = {
>>       .read = uart_read,
>>       .write = uart_write,
>>       .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .impl = {
>> +        .min_access_size = 4,

Odd. The change makes the qtests pass, but here I'm modifying .impl,
not .valid... Since .valid.min_access_size = 1, SBI is a valid
opcode, no need to use SWI.

>> +        .max_access_size = 4,
>> +    },
>>       .valid = {
>>           .min_access_size = 1,
>>           .max_access_size = 4,
> 
> To have qtests working I need to squash:
> 
> -- >8 --
> diff --git a/tests/qtest/boot-serial-test.c 
> b/tests/qtest/boot-serial-test.c
> index 3b92fa5d506..6d9291c8ae2 100644
> --- a/tests/qtest/boot-serial-test.c
> +++ b/tests/qtest/boot-serial-test.c
> @@ -57,7 +57,7 @@ static const uint8_t kernel_pls3adsp1800[] = {
>       0xb0, 0x00, 0x84, 0x00,                 /* imm   0x8400 */
>       0x30, 0x60, 0x00, 0x04,                 /* addik r3,r0,4 */
>       0x30, 0x80, 0x00, 0x54,                 /* addik r4,r0,'T' */
> -    0xf0, 0x83, 0x00, 0x00,                 /* sbi   r4,r3,0 */
> +    0xf8, 0x83, 0x00, 0x00,                 /* swi   r4,r3,0 */
>       0xb8, 0x00, 0xff, 0xfc                  /* bri   -4  loop */
>   };
> 
> @@ -65,7 +65,7 @@ static const uint8_t kernel_plml605[] = {
>       0xe0, 0x83, 0x00, 0xb0,                 /* imm   0x83e0 */
>       0x00, 0x10, 0x60, 0x30,                 /* addik r3,r0,0x1000 */
>       0x54, 0x00, 0x80, 0x30,                 /* addik r4,r0,'T' */
> -    0x00, 0x00, 0x83, 0xf0,                 /* sbi   r4,r3,0 */
> +    0x00, 0x00, 0x83, 0xf8,                 /* swi   r4,r3,0 */
>       0xfc, 0xff, 0x00, 0xb8                  /* bri   -4  loop */
>   };
> ---
> 
> to access the uart by 32-bit instead of 8-bit.
Re: [PATCH 07/19] hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit
Posted by Philippe Mathieu-Daudé 1 year, 1 month ago 
On 5/11/24 23:27, Philippe Mathieu-Daudé wrote:
> On 5/11/24 23:24, Philippe Mathieu-Daudé wrote:
>> On 5/11/24 14:04, Philippe Mathieu-Daudé wrote:
>>> All these MemoryRegionOps read() and write() handlers are
>>> implemented expecting 32-bit accesses. Clarify that setting
>>> .impl.min/max_access_size fields.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   hw/char/xilinx_uartlite.c | 4 ++++
>>>   hw/intc/xilinx_intc.c     | 4 ++++
>>>   hw/net/xilinx_ethlite.c   | 4 ++++
>>>   hw/timer/xilinx_timer.c   | 4 ++++
>>>   4 files changed, 16 insertions(+)
>>>
>>> diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
>>> index a69ad769cc4..892efe81fee 100644
>>> --- a/hw/char/xilinx_uartlite.c
>>> +++ b/hw/char/xilinx_uartlite.c
>>> @@ -170,6 +170,10 @@ static const MemoryRegionOps uart_ops = {
>>>       .read = uart_read,
>>>       .write = uart_write,
>>>       .endianness = DEVICE_NATIVE_ENDIAN,
>>> +    .impl = {
>>> +        .min_access_size = 4,
> 
> Odd. The change makes the qtests pass, but here I'm modifying .impl,
> not .valid... Since .valid.min_access_size = 1, SBI is a valid
> opcode, no need to use SWI.

Which proves this device is only mapped in little-endian.

> 
>>> +        .max_access_size = 4,
>>> +    },
>>>       .valid = {
>>>           .min_access_size = 1,
>>>           .max_access_size = 4,
>>
>> To have qtests working I need to squash:
>>
>> -- >8 --
>> diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial- 
>> test.c
>> index 3b92fa5d506..6d9291c8ae2 100644
>> --- a/tests/qtest/boot-serial-test.c
>> +++ b/tests/qtest/boot-serial-test.c
>> @@ -57,7 +57,7 @@ static const uint8_t kernel_pls3adsp1800[] = {
>>       0xb0, 0x00, 0x84, 0x00,                 /* imm   0x8400 */
>>       0x30, 0x60, 0x00, 0x04,                 /* addik r3,r0,4 */
>>       0x30, 0x80, 0x00, 0x54,                 /* addik r4,r0,'T' */
>> -    0xf0, 0x83, 0x00, 0x00,                 /* sbi   r4,r3,0 */
>> +    0xf8, 0x83, 0x00, 0x00,                 /* swi   r4,r3,0 */
>>       0xb8, 0x00, 0xff, 0xfc                  /* bri   -4  loop */
>>   };
>>
>> @@ -65,7 +65,7 @@ static const uint8_t kernel_plml605[] = {
>>       0xe0, 0x83, 0x00, 0xb0,                 /* imm   0x83e0 */
>>       0x00, 0x10, 0x60, 0x30,                 /* addik r3,r0,0x1000 */
>>       0x54, 0x00, 0x80, 0x30,                 /* addik r4,r0,'T' */
>> -    0x00, 0x00, 0x83, 0xf0,                 /* sbi   r4,r3,0 */
>> +    0x00, 0x00, 0x83, 0xf8,                 /* swi   r4,r3,0 */
>>       0xfc, 0xff, 0x00, 0xb8                  /* bri   -4  loop */
>>   };
>> ---
>>
>> to access the uart by 32-bit instead of 8-bit.
Re: [PATCH 07/19] hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit
Posted by Anton Johansson via 1 year, 3 months ago 
On 05/11/24, Philippe Mathieu-Daudé wrote:
> All these MemoryRegionOps read() and write() handlers are
> implemented expecting 32-bit accesses. Clarify that setting
> .impl.min/max_access_size fields.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/char/xilinx_uartlite.c | 4 ++++
>  hw/intc/xilinx_intc.c     | 4 ++++
>  hw/net/xilinx_ethlite.c   | 4 ++++
>  hw/timer/xilinx_timer.c   | 4 ++++
>  4 files changed, 16 insertions(+)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.