Patchwork [2/2] serial: Declare as little endian

login
register
mail settings
Submitter Blue Swirl
Date Dec. 19, 2010, 10:46 a.m.
Message ID <AANLkTi=s+pH-0f2y-FTjnocRwGPpEicstgW2p_M=QMZm@mail.gmail.com>
Download mbox | patch
Permalink /patch/76116/
State New
Headers show

Comments

Blue Swirl - Dec. 19, 2010, 10:46 a.m.
This patch replaces explicit bswaps with endianness hints to the
mmio layer.

CC: Alexander Graf <agraf@suse.de>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/serial.c |   86 ++++++++++++----------------------------------------------
 1 files changed, 18 insertions(+), 68 deletions(-)

     uint32_t val;
@@ -894,45 +865,24 @@ static uint32_t serial_mm_readl_le(void *opaque,
target_phys_addr_t addr)
     return val;
 }

-static void serial_mm_writel_be(void *opaque, target_phys_addr_t addr,
-                                uint32_t value)
-{
-    SerialState *s = opaque;
-
-    value = bswap32(value);
-    serial_ioport_write(s, addr >> s->it_shift, value);
-}
-
-static void serial_mm_writel_le(void *opaque, target_phys_addr_t addr,
-                                uint32_t value)
+static void serial_mm_writel(void *opaque, target_phys_addr_t addr,
+                             uint32_t value)
 {
     SerialState *s = opaque;

     serial_ioport_write(s, addr >> s->it_shift, value);
 }

-static CPUReadMemoryFunc * const serial_mm_read_be[] = {
-    &serial_mm_readb,
-    &serial_mm_readw_be,
-    &serial_mm_readl_be,
-};
-
-static CPUWriteMemoryFunc * const serial_mm_write_be[] = {
-    &serial_mm_writeb,
-    &serial_mm_writew_be,
-    &serial_mm_writel_be,
-};
-
-static CPUReadMemoryFunc * const serial_mm_read_le[] = {
+static CPUReadMemoryFunc * const serial_mm_read[] = {
     &serial_mm_readb,
-    &serial_mm_readw_le,
-    &serial_mm_readl_le,
+    &serial_mm_readw,
+    &serial_mm_readl,
 };

-static CPUWriteMemoryFunc * const serial_mm_write_le[] = {
+static CPUWriteMemoryFunc * const serial_mm_write[] = {
     &serial_mm_writeb,
-    &serial_mm_writew_le,
-    &serial_mm_writel_le,
+    &serial_mm_writew,
+    &serial_mm_writel,
 };

 SerialState *serial_mm_init (target_phys_addr_t base, int it_shift,
@@ -955,13 +905,13 @@ SerialState *serial_mm_init (target_phys_addr_t
base, int it_shift,

     if (ioregister) {
         if (be) {
-            s_io_memory = cpu_register_io_memory(serial_mm_read_be,
-                                                 serial_mm_write_be, s,
-                                                 DEVICE_NATIVE_ENDIAN);
+            s_io_memory = cpu_register_io_memory(serial_mm_read,
+                                                 serial_mm_write, s,
+                                                 DEVICE_BIG_ENDIAN);
         } else {
-            s_io_memory = cpu_register_io_memory(serial_mm_read_le,
-                                                 serial_mm_write_le, s,
-                                                 DEVICE_NATIVE_ENDIAN);
+            s_io_memory = cpu_register_io_memory(serial_mm_read,
+                                                 serial_mm_write, s,
+                                                 DEVICE_LITTLE_ENDIAN);
         }
         cpu_register_physical_memory(base, 8 << it_shift, s_io_memory);
     }
Alexander Graf - Dec. 19, 2010, 3:25 p.m.
On 19.12.2010, at 11:46, Blue Swirl wrote:

> This patch replaces explicit bswaps with endianness hints to the
> mmio layer.
> 
> CC: Alexander Graf <agraf@suse.de>
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
> hw/serial.c |   86 ++++++++++++----------------------------------------------
> 1 files changed, 18 insertions(+), 68 deletions(-)
> 
> diff --git a/hw/serial.c b/hw/serial.c
> index 2c4af61..70ae26a 100644
> --- a/hw/serial.c
> +++ b/hw/serial.c
> @@ -839,53 +839,24 @@ static void serial_mm_writeb(void *opaque,
> target_phys_addr_t addr,
>     serial_ioport_write(s, addr >> s->it_shift, value & 0xFF);
> }
> 
> -static uint32_t serial_mm_readw_be(void *opaque, target_phys_addr_t addr)
> +static uint32_t serial_mm_readw(void *opaque, target_phys_addr_t addr)
> {
>     SerialState *s = opaque;
>     uint32_t val;
> 
>     val = serial_ioport_read(s, addr >> s->it_shift) & 0xFFFF;
> -    val = bswap16(val);
>     return val;
> }
> 
> -static uint32_t serial_mm_readw_le(void *opaque, target_phys_addr_t addr)
> -{
> -    SerialState *s = opaque;
> -    uint32_t val;
> -
> -    val = serial_ioport_read(s, addr >> s->it_shift) & 0xFFFF;
> -    return val;
> -}
> -
> -static void serial_mm_writew_be(void *opaque, target_phys_addr_t addr,
> -                                uint32_t value)
> -{
> -    SerialState *s = opaque;
> -
> -    value = bswap16(value);
> -    serial_ioport_write(s, addr >> s->it_shift, value & 0xFFFF);
> -}
> -
> -static void serial_mm_writew_le(void *opaque, target_phys_addr_t addr,
> -                                uint32_t value)
> +static void serial_mm_writew(void *opaque, target_phys_addr_t addr,
> +                             uint32_t value)
> {
>     SerialState *s = opaque;
> 
>     serial_ioport_write(s, addr >> s->it_shift, value & 0xFFFF);
> }
> 
> -static uint32_t serial_mm_readl_be(void *opaque, target_phys_addr_t addr)
> -{
> -    SerialState *s = opaque;
> -    uint32_t val;
> -
> -    val = serial_ioport_read(s, addr >> s->it_shift);
> -    val = bswap32(val);
> -    return val;
> -}
> -
> -static uint32_t serial_mm_readl_le(void *opaque, target_phys_addr_t addr)
> +static uint32_t serial_mm_readl(void *opaque, target_phys_addr_t addr)
> {
>     SerialState *s = opaque;
>     uint32_t val;
> @@ -894,45 +865,24 @@ static uint32_t serial_mm_readl_le(void *opaque,
> target_phys_addr_t addr)
>     return val;
> }
> 
> -static void serial_mm_writel_be(void *opaque, target_phys_addr_t addr,
> -                                uint32_t value)
> -{
> -    SerialState *s = opaque;
> -
> -    value = bswap32(value);
> -    serial_ioport_write(s, addr >> s->it_shift, value);
> -}
> -
> -static void serial_mm_writel_le(void *opaque, target_phys_addr_t addr,
> -                                uint32_t value)
> +static void serial_mm_writel(void *opaque, target_phys_addr_t addr,
> +                             uint32_t value)
> {
>     SerialState *s = opaque;
> 
>     serial_ioport_write(s, addr >> s->it_shift, value);
> }
> 
> -static CPUReadMemoryFunc * const serial_mm_read_be[] = {
> -    &serial_mm_readb,
> -    &serial_mm_readw_be,
> -    &serial_mm_readl_be,
> -};
> -
> -static CPUWriteMemoryFunc * const serial_mm_write_be[] = {
> -    &serial_mm_writeb,
> -    &serial_mm_writew_be,
> -    &serial_mm_writel_be,
> -};
> -
> -static CPUReadMemoryFunc * const serial_mm_read_le[] = {
> +static CPUReadMemoryFunc * const serial_mm_read[] = {
>     &serial_mm_readb,
> -    &serial_mm_readw_le,
> -    &serial_mm_readl_le,
> +    &serial_mm_readw,
> +    &serial_mm_readl,
> };
> 
> -static CPUWriteMemoryFunc * const serial_mm_write_le[] = {
> +static CPUWriteMemoryFunc * const serial_mm_write[] = {
>     &serial_mm_writeb,
> -    &serial_mm_writew_le,
> -    &serial_mm_writel_le,
> +    &serial_mm_writew,
> +    &serial_mm_writel,
> };
> 
> SerialState *serial_mm_init (target_phys_addr_t base, int it_shift,
> @@ -955,13 +905,13 @@ SerialState *serial_mm_init (target_phys_addr_t
> base, int it_shift,
> 
>     if (ioregister) {
>         if (be) {
> -            s_io_memory = cpu_register_io_memory(serial_mm_read_be,
> -                                                 serial_mm_write_be, s,
> -                                                 DEVICE_NATIVE_ENDIAN);
> +            s_io_memory = cpu_register_io_memory(serial_mm_read,
> +                                                 serial_mm_write, s,
> +                                                 DEVICE_BIG_ENDIAN);

Have you verified that this works as intended? Usually the be flags in init code are bogus and define the guest endianness.


Alex
Blue Swirl - Dec. 20, 2010, 6:26 p.m.
On Sun, Dec 19, 2010 at 3:25 PM, Alexander Graf <agraf@suse.de> wrote:
>
> On 19.12.2010, at 11:46, Blue Swirl wrote:
>
>> This patch replaces explicit bswaps with endianness hints to the
>> mmio layer.
>>
>> CC: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>> ---
>> hw/serial.c |   86 ++++++++++++----------------------------------------------
>> 1 files changed, 18 insertions(+), 68 deletions(-)
>>
>> diff --git a/hw/serial.c b/hw/serial.c
>> index 2c4af61..70ae26a 100644
>> --- a/hw/serial.c
>> +++ b/hw/serial.c
>> @@ -839,53 +839,24 @@ static void serial_mm_writeb(void *opaque,
>> target_phys_addr_t addr,
>>     serial_ioport_write(s, addr >> s->it_shift, value & 0xFF);
>> }
>>
>> -static uint32_t serial_mm_readw_be(void *opaque, target_phys_addr_t addr)
>> +static uint32_t serial_mm_readw(void *opaque, target_phys_addr_t addr)
>> {
>>     SerialState *s = opaque;
>>     uint32_t val;
>>
>>     val = serial_ioport_read(s, addr >> s->it_shift) & 0xFFFF;
>> -    val = bswap16(val);
>>     return val;
>> }
>>
>> -static uint32_t serial_mm_readw_le(void *opaque, target_phys_addr_t addr)
>> -{
>> -    SerialState *s = opaque;
>> -    uint32_t val;
>> -
>> -    val = serial_ioport_read(s, addr >> s->it_shift) & 0xFFFF;
>> -    return val;
>> -}
>> -
>> -static void serial_mm_writew_be(void *opaque, target_phys_addr_t addr,
>> -                                uint32_t value)
>> -{
>> -    SerialState *s = opaque;
>> -
>> -    value = bswap16(value);
>> -    serial_ioport_write(s, addr >> s->it_shift, value & 0xFFFF);
>> -}
>> -
>> -static void serial_mm_writew_le(void *opaque, target_phys_addr_t addr,
>> -                                uint32_t value)
>> +static void serial_mm_writew(void *opaque, target_phys_addr_t addr,
>> +                             uint32_t value)
>> {
>>     SerialState *s = opaque;
>>
>>     serial_ioport_write(s, addr >> s->it_shift, value & 0xFFFF);
>> }
>>
>> -static uint32_t serial_mm_readl_be(void *opaque, target_phys_addr_t addr)
>> -{
>> -    SerialState *s = opaque;
>> -    uint32_t val;
>> -
>> -    val = serial_ioport_read(s, addr >> s->it_shift);
>> -    val = bswap32(val);
>> -    return val;
>> -}
>> -
>> -static uint32_t serial_mm_readl_le(void *opaque, target_phys_addr_t addr)
>> +static uint32_t serial_mm_readl(void *opaque, target_phys_addr_t addr)
>> {
>>     SerialState *s = opaque;
>>     uint32_t val;
>> @@ -894,45 +865,24 @@ static uint32_t serial_mm_readl_le(void *opaque,
>> target_phys_addr_t addr)
>>     return val;
>> }
>>
>> -static void serial_mm_writel_be(void *opaque, target_phys_addr_t addr,
>> -                                uint32_t value)
>> -{
>> -    SerialState *s = opaque;
>> -
>> -    value = bswap32(value);
>> -    serial_ioport_write(s, addr >> s->it_shift, value);
>> -}
>> -
>> -static void serial_mm_writel_le(void *opaque, target_phys_addr_t addr,
>> -                                uint32_t value)
>> +static void serial_mm_writel(void *opaque, target_phys_addr_t addr,
>> +                             uint32_t value)
>> {
>>     SerialState *s = opaque;
>>
>>     serial_ioport_write(s, addr >> s->it_shift, value);
>> }
>>
>> -static CPUReadMemoryFunc * const serial_mm_read_be[] = {
>> -    &serial_mm_readb,
>> -    &serial_mm_readw_be,
>> -    &serial_mm_readl_be,
>> -};
>> -
>> -static CPUWriteMemoryFunc * const serial_mm_write_be[] = {
>> -    &serial_mm_writeb,
>> -    &serial_mm_writew_be,
>> -    &serial_mm_writel_be,
>> -};
>> -
>> -static CPUReadMemoryFunc * const serial_mm_read_le[] = {
>> +static CPUReadMemoryFunc * const serial_mm_read[] = {
>>     &serial_mm_readb,
>> -    &serial_mm_readw_le,
>> -    &serial_mm_readl_le,
>> +    &serial_mm_readw,
>> +    &serial_mm_readl,
>> };
>>
>> -static CPUWriteMemoryFunc * const serial_mm_write_le[] = {
>> +static CPUWriteMemoryFunc * const serial_mm_write[] = {
>>     &serial_mm_writeb,
>> -    &serial_mm_writew_le,
>> -    &serial_mm_writel_le,
>> +    &serial_mm_writew,
>> +    &serial_mm_writel,
>> };
>>
>> SerialState *serial_mm_init (target_phys_addr_t base, int it_shift,
>> @@ -955,13 +905,13 @@ SerialState *serial_mm_init (target_phys_addr_t
>> base, int it_shift,
>>
>>     if (ioregister) {
>>         if (be) {
>> -            s_io_memory = cpu_register_io_memory(serial_mm_read_be,
>> -                                                 serial_mm_write_be, s,
>> -                                                 DEVICE_NATIVE_ENDIAN);
>> +            s_io_memory = cpu_register_io_memory(serial_mm_read,
>> +                                                 serial_mm_write, s,
>> +                                                 DEVICE_BIG_ENDIAN);
>
> Have you verified that this works as intended? Usually the be flags in init code are bogus and define the guest endianness.

At least the serial console on Sparc64 still works.

Patch

diff --git a/hw/serial.c b/hw/serial.c
index 2c4af61..70ae26a 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -839,53 +839,24 @@  static void serial_mm_writeb(void *opaque,
target_phys_addr_t addr,
     serial_ioport_write(s, addr >> s->it_shift, value & 0xFF);
 }

-static uint32_t serial_mm_readw_be(void *opaque, target_phys_addr_t addr)
+static uint32_t serial_mm_readw(void *opaque, target_phys_addr_t addr)
 {
     SerialState *s = opaque;
     uint32_t val;

     val = serial_ioport_read(s, addr >> s->it_shift) & 0xFFFF;
-    val = bswap16(val);
     return val;
 }

-static uint32_t serial_mm_readw_le(void *opaque, target_phys_addr_t addr)
-{
-    SerialState *s = opaque;
-    uint32_t val;
-
-    val = serial_ioport_read(s, addr >> s->it_shift) & 0xFFFF;
-    return val;
-}
-
-static void serial_mm_writew_be(void *opaque, target_phys_addr_t addr,
-                                uint32_t value)
-{
-    SerialState *s = opaque;
-
-    value = bswap16(value);
-    serial_ioport_write(s, addr >> s->it_shift, value & 0xFFFF);
-}
-
-static void serial_mm_writew_le(void *opaque, target_phys_addr_t addr,
-                                uint32_t value)
+static void serial_mm_writew(void *opaque, target_phys_addr_t addr,
+                             uint32_t value)
 {
     SerialState *s = opaque;

     serial_ioport_write(s, addr >> s->it_shift, value & 0xFFFF);
 }

-static uint32_t serial_mm_readl_be(void *opaque, target_phys_addr_t addr)
-{
-    SerialState *s = opaque;
-    uint32_t val;
-
-    val = serial_ioport_read(s, addr >> s->it_shift);
-    val = bswap32(val);
-    return val;
-}
-
-static uint32_t serial_mm_readl_le(void *opaque, target_phys_addr_t addr)
+static uint32_t serial_mm_readl(void *opaque, target_phys_addr_t addr)
 {
     SerialState *s = opaque;