All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] vfio: Make BARs native endian
@ 2014-06-18 11:35 Alexey Kardashevskiy
  2014-06-18 18:57 ` Alex Williamson
  0 siblings, 1 reply; 5+ messages in thread
From: Alexey Kardashevskiy @ 2014-06-18 11:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexey Kardashevskiy, Alex Williamson, Alexander Graf, Nikunj A Dadhania

Slow BAR access path is used when VFIO fails to mmap() BAR or TCG is used.
Since this is just a transport between the guest and a device, there is
no need to do endianness swapping.

This changes BARs to use native endianness. Since non-ROM BARs were
doing byte swapping, we need to remove it so does the patch. No change
in behavior is expected though.

ROM BARs were declared little endian but byte swapping was not
implemented for them so they never actually worked on big endian systems.
This fixes endiannes for ROM BARs: this declares them native endian and
fixes access sizes.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/misc/vfio.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 7437c2e..3eb3c71 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -1052,10 +1052,10 @@ static void vfio_bar_write(void *opaque, hwaddr addr,
         buf.byte = data;
         break;
     case 2:
-        buf.word = cpu_to_le16(data);
+        buf.word = data;
         break;
     case 4:
-        buf.dword = cpu_to_le32(data);
+        buf.dword = data;
         break;
     default:
         hw_error("vfio: unsupported write size, %d bytes", size);
@@ -1112,10 +1112,10 @@ static uint64_t vfio_bar_read(void *opaque,
         data = buf.byte;
         break;
     case 2:
-        data = le16_to_cpu(buf.word);
+        data = buf.word;
         break;
     case 4:
-        data = le32_to_cpu(buf.dword);
+        data = buf.dword;
         break;
     default:
         hw_error("vfio: unsupported read size, %d bytes", size);
@@ -1142,7 +1142,7 @@ static uint64_t vfio_bar_read(void *opaque,
 static const MemoryRegionOps vfio_bar_ops = {
     .read = vfio_bar_read,
     .write = vfio_bar_write,
-    .endianness = DEVICE_LITTLE_ENDIAN,
+    .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static void vfio_pci_load_rom(VFIODevice *vdev)
@@ -1204,21 +1204,42 @@ static void vfio_pci_load_rom(VFIODevice *vdev)
 static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)
 {
     VFIODevice *vdev = opaque;
-    uint64_t val = ((uint64_t)1 << (size * 8)) - 1;
+    union {
+        uint8_t byte;
+        uint16_t word;
+        uint32_t dword;
+        uint64_t qword;
+    } buf;
+    uint64_t data = 0;
 
     /* Load the ROM lazily when the guest tries to read it */
     if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
         vfio_pci_load_rom(vdev);
     }
 
-    memcpy(&val, vdev->rom + addr,
+    memcpy(&buf, vdev->rom + addr,
            (addr < vdev->rom_size) ? MIN(size, vdev->rom_size - addr) : 0);
 
+    switch (size) {
+    case 1:
+        data = buf.byte;
+        break;
+    case 2:
+        data = buf.word;
+        break;
+    case 4:
+        data = buf.dword;
+        break;
+    default:
+        hw_error("vfio: unsupported read size, %d bytes", size);
+        break;
+    }
+
     DPRINTF("%s(%04x:%02x:%02x.%x, 0x%"HWADDR_PRIx", 0x%x) = 0x%"PRIx64"\n",
             __func__, vdev->host.domain, vdev->host.bus, vdev->host.slot,
-            vdev->host.function, addr, size, val);
+            vdev->host.function, addr, size, data);
 
-    return val;
+    return data;
 }
 
 static void vfio_rom_write(void *opaque, hwaddr addr,
@@ -1229,7 +1250,7 @@ static void vfio_rom_write(void *opaque, hwaddr addr,
 static const MemoryRegionOps vfio_rom_ops = {
     .read = vfio_rom_read,
     .write = vfio_rom_write,
-    .endianness = DEVICE_LITTLE_ENDIAN,
+    .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static bool vfio_blacklist_opt_rom(VFIODevice *vdev)
-- 
2.0.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] vfio: Make BARs native endian
  2014-06-18 11:35 [Qemu-devel] [PATCH] vfio: Make BARs native endian Alexey Kardashevskiy
@ 2014-06-18 18:57 ` Alex Williamson
  2014-06-19  0:42   ` Alexey Kardashevskiy
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Williamson @ 2014-06-18 18:57 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-devel, Nikunj A Dadhania, Alexander Graf

On Wed, 2014-06-18 at 21:35 +1000, Alexey Kardashevskiy wrote:
> Slow BAR access path is used when VFIO fails to mmap() BAR or TCG is used.

TCG can use the mmap too

> Since this is just a transport between the guest and a device, there is
> no need to do endianness swapping.
> 
> This changes BARs to use native endianness. Since non-ROM BARs were
> doing byte swapping, we need to remove it so does the patch. No change
> in behavior is expected though.
> 
> ROM BARs were declared little endian but byte swapping was not
> implemented for them so they never actually worked on big endian systems.
> This fixes endiannes for ROM BARs: this declares them native endian and
> fixes access sizes.

So the only actual behavior change we expect from this is to fix ROM
access on big endian where we had an unbalanced set of byte swaps.  BAR
access worked because the byte swapping in the read/write handler
canceled the byte swapping in QEMU because we declared the BARs as
little endian.  Otherwise we're just removing canceling byte swaps for
big endian.  Is that correct?

Also, as the kernel patch appears to not produce any behavioral change,
only eliminates canceling swaps, there's no dependency between patches,
right?  Thanks,

Alex

> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  hw/misc/vfio.c | 41 +++++++++++++++++++++++++++++++----------
>  1 file changed, 31 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index 7437c2e..3eb3c71 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -1052,10 +1052,10 @@ static void vfio_bar_write(void *opaque, hwaddr addr,
>          buf.byte = data;
>          break;
>      case 2:
> -        buf.word = cpu_to_le16(data);
> +        buf.word = data;
>          break;
>      case 4:
> -        buf.dword = cpu_to_le32(data);
> +        buf.dword = data;
>          break;
>      default:
>          hw_error("vfio: unsupported write size, %d bytes", size);
> @@ -1112,10 +1112,10 @@ static uint64_t vfio_bar_read(void *opaque,
>          data = buf.byte;
>          break;
>      case 2:
> -        data = le16_to_cpu(buf.word);
> +        data = buf.word;
>          break;
>      case 4:
> -        data = le32_to_cpu(buf.dword);
> +        data = buf.dword;
>          break;
>      default:
>          hw_error("vfio: unsupported read size, %d bytes", size);
> @@ -1142,7 +1142,7 @@ static uint64_t vfio_bar_read(void *opaque,
>  static const MemoryRegionOps vfio_bar_ops = {
>      .read = vfio_bar_read,
>      .write = vfio_bar_write,
> -    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
>  static void vfio_pci_load_rom(VFIODevice *vdev)
> @@ -1204,21 +1204,42 @@ static void vfio_pci_load_rom(VFIODevice *vdev)
>  static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      VFIODevice *vdev = opaque;
> -    uint64_t val = ((uint64_t)1 << (size * 8)) - 1;
> +    union {
> +        uint8_t byte;
> +        uint16_t word;
> +        uint32_t dword;
> +        uint64_t qword;
> +    } buf;
> +    uint64_t data = 0;
>  
>      /* Load the ROM lazily when the guest tries to read it */
>      if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
>          vfio_pci_load_rom(vdev);
>      }
>  
> -    memcpy(&val, vdev->rom + addr,
> +    memcpy(&buf, vdev->rom + addr,
>             (addr < vdev->rom_size) ? MIN(size, vdev->rom_size - addr) : 0);
>  
> +    switch (size) {
> +    case 1:
> +        data = buf.byte;
> +        break;
> +    case 2:
> +        data = buf.word;
> +        break;
> +    case 4:
> +        data = buf.dword;
> +        break;
> +    default:
> +        hw_error("vfio: unsupported read size, %d bytes", size);
> +        break;
> +    }
> +
>      DPRINTF("%s(%04x:%02x:%02x.%x, 0x%"HWADDR_PRIx", 0x%x) = 0x%"PRIx64"\n",
>              __func__, vdev->host.domain, vdev->host.bus, vdev->host.slot,
> -            vdev->host.function, addr, size, val);
> +            vdev->host.function, addr, size, data);
>  
> -    return val;
> +    return data;
>  }
>  
>  static void vfio_rom_write(void *opaque, hwaddr addr,
> @@ -1229,7 +1250,7 @@ static void vfio_rom_write(void *opaque, hwaddr addr,
>  static const MemoryRegionOps vfio_rom_ops = {
>      .read = vfio_rom_read,
>      .write = vfio_rom_write,
> -    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
>  static bool vfio_blacklist_opt_rom(VFIODevice *vdev)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] vfio: Make BARs native endian
  2014-06-18 18:57 ` Alex Williamson
@ 2014-06-19  0:42   ` Alexey Kardashevskiy
  2014-06-19  1:53     ` Alexey Kardashevskiy
  0 siblings, 1 reply; 5+ messages in thread
From: Alexey Kardashevskiy @ 2014-06-19  0:42 UTC (permalink / raw)
  To: Alex Williamson; +Cc: qemu-devel, Nikunj A Dadhania, Alexander Graf

On 06/19/2014 04:57 AM, Alex Williamson wrote:
> On Wed, 2014-06-18 at 21:35 +1000, Alexey Kardashevskiy wrote:
>> Slow BAR access path is used when VFIO fails to mmap() BAR or TCG is used.
> 
> TCG can use the mmap too

Oh, right.


> 
>> Since this is just a transport between the guest and a device, there is
>> no need to do endianness swapping.
>>
>> This changes BARs to use native endianness. Since non-ROM BARs were
>> doing byte swapping, we need to remove it so does the patch. No change
>> in behavior is expected though.
>>
>> ROM BARs were declared little endian but byte swapping was not
>> implemented for them so they never actually worked on big endian systems.
>> This fixes endiannes for ROM BARs: this declares them native endian and
>> fixes access sizes.
> 
> So the only actual behavior change we expect from this is to fix ROM
> access on big endian where we had an unbalanced set of byte swaps.  BAR
> access worked because the byte swapping in the read/write handler
> canceled the byte swapping in QEMU because we declared the BARs as
> little endian.  Otherwise we're just removing canceling byte swaps for
> big endian.  Is that correct?

Yes.

> 
> Also, as the kernel patch appears to not produce any behavioral change,
> only eliminates canceling swaps, there's no dependency between patches,
> right?  Thanks,

That is right.

> 
> Alex
> 
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>  hw/misc/vfio.c | 41 +++++++++++++++++++++++++++++++----------
>>  1 file changed, 31 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
>> index 7437c2e..3eb3c71 100644
>> --- a/hw/misc/vfio.c
>> +++ b/hw/misc/vfio.c
>> @@ -1052,10 +1052,10 @@ static void vfio_bar_write(void *opaque, hwaddr addr,
>>          buf.byte = data;
>>          break;
>>      case 2:
>> -        buf.word = cpu_to_le16(data);
>> +        buf.word = data;
>>          break;
>>      case 4:
>> -        buf.dword = cpu_to_le32(data);
>> +        buf.dword = data;
>>          break;
>>      default:
>>          hw_error("vfio: unsupported write size, %d bytes", size);
>> @@ -1112,10 +1112,10 @@ static uint64_t vfio_bar_read(void *opaque,
>>          data = buf.byte;
>>          break;
>>      case 2:
>> -        data = le16_to_cpu(buf.word);
>> +        data = buf.word;
>>          break;
>>      case 4:
>> -        data = le32_to_cpu(buf.dword);
>> +        data = buf.dword;
>>          break;
>>      default:
>>          hw_error("vfio: unsupported read size, %d bytes", size);
>> @@ -1142,7 +1142,7 @@ static uint64_t vfio_bar_read(void *opaque,
>>  static const MemoryRegionOps vfio_bar_ops = {
>>      .read = vfio_bar_read,
>>      .write = vfio_bar_write,
>> -    .endianness = DEVICE_LITTLE_ENDIAN,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>  };
>>  
>>  static void vfio_pci_load_rom(VFIODevice *vdev)
>> @@ -1204,21 +1204,42 @@ static void vfio_pci_load_rom(VFIODevice *vdev)
>>  static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)
>>  {
>>      VFIODevice *vdev = opaque;
>> -    uint64_t val = ((uint64_t)1 << (size * 8)) - 1;
>> +    union {
>> +        uint8_t byte;
>> +        uint16_t word;
>> +        uint32_t dword;
>> +        uint64_t qword;
>> +    } buf;
>> +    uint64_t data = 0;
>>  
>>      /* Load the ROM lazily when the guest tries to read it */
>>      if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
>>          vfio_pci_load_rom(vdev);
>>      }
>>  
>> -    memcpy(&val, vdev->rom + addr,
>> +    memcpy(&buf, vdev->rom + addr,
>>             (addr < vdev->rom_size) ? MIN(size, vdev->rom_size - addr) : 0);
>>  
>> +    switch (size) {
>> +    case 1:
>> +        data = buf.byte;
>> +        break;
>> +    case 2:
>> +        data = buf.word;
>> +        break;
>> +    case 4:
>> +        data = buf.dword;
>> +        break;
>> +    default:
>> +        hw_error("vfio: unsupported read size, %d bytes", size);
>> +        break;
>> +    }
>> +
>>      DPRINTF("%s(%04x:%02x:%02x.%x, 0x%"HWADDR_PRIx", 0x%x) = 0x%"PRIx64"\n",
>>              __func__, vdev->host.domain, vdev->host.bus, vdev->host.slot,
>> -            vdev->host.function, addr, size, val);
>> +            vdev->host.function, addr, size, data);
>>  
>> -    return val;
>> +    return data;
>>  }
>>  
>>  static void vfio_rom_write(void *opaque, hwaddr addr,
>> @@ -1229,7 +1250,7 @@ static void vfio_rom_write(void *opaque, hwaddr addr,
>>  static const MemoryRegionOps vfio_rom_ops = {
>>      .read = vfio_rom_read,
>>      .write = vfio_rom_write,
>> -    .endianness = DEVICE_LITTLE_ENDIAN,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>  };
>>  
>>  static bool vfio_blacklist_opt_rom(VFIODevice *vdev)
> 
> 
> 


-- 
Alexey

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] vfio: Make BARs native endian
  2014-06-19  0:42   ` Alexey Kardashevskiy
@ 2014-06-19  1:53     ` Alexey Kardashevskiy
  2014-06-20  3:21       ` Alex Williamson
  0 siblings, 1 reply; 5+ messages in thread
From: Alexey Kardashevskiy @ 2014-06-19  1:53 UTC (permalink / raw)
  To: Alex Williamson; +Cc: qemu-devel, Nikunj A Dadhania, Alexander Graf

On 06/19/2014 10:42 AM, Alexey Kardashevskiy wrote:
> On 06/19/2014 04:57 AM, Alex Williamson wrote:
>> On Wed, 2014-06-18 at 21:35 +1000, Alexey Kardashevskiy wrote:
>>> Slow BAR access path is used when VFIO fails to mmap() BAR or TCG is used.
>>
>> TCG can use the mmap too
> 
> Oh, right.

I'll remove "tcg" and what else is missing here? I thought the original
commit log already had all answers on the questions you askes (or clarified
:) )
Thanks!

> 
> 
>>
>>> Since this is just a transport between the guest and a device, there is
>>> no need to do endianness swapping.
>>>
>>> This changes BARs to use native endianness. Since non-ROM BARs were
>>> doing byte swapping, we need to remove it so does the patch. No change
>>> in behavior is expected though.
>>>
>>> ROM BARs were declared little endian but byte swapping was not
>>> implemented for them so they never actually worked on big endian systems.
>>> This fixes endiannes for ROM BARs: this declares them native endian and
>>> fixes access sizes.
>>
>> So the only actual behavior change we expect from this is to fix ROM
>> access on big endian where we had an unbalanced set of byte swaps.  BAR
>> access worked because the byte swapping in the read/write handler
>> canceled the byte swapping in QEMU because we declared the BARs as
>> little endian.  Otherwise we're just removing canceling byte swaps for
>> big endian.  Is that correct?
> 
> Yes.
> 
>>
>> Also, as the kernel patch appears to not produce any behavioral change,
>> only eliminates canceling swaps, there's no dependency between patches,
>> right?  Thanks,
> 
> That is right.






> 
>>
>> Alex
>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>  hw/misc/vfio.c | 41 +++++++++++++++++++++++++++++++----------
>>>  1 file changed, 31 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
>>> index 7437c2e..3eb3c71 100644
>>> --- a/hw/misc/vfio.c
>>> +++ b/hw/misc/vfio.c
>>> @@ -1052,10 +1052,10 @@ static void vfio_bar_write(void *opaque, hwaddr addr,
>>>          buf.byte = data;
>>>          break;
>>>      case 2:
>>> -        buf.word = cpu_to_le16(data);
>>> +        buf.word = data;
>>>          break;
>>>      case 4:
>>> -        buf.dword = cpu_to_le32(data);
>>> +        buf.dword = data;
>>>          break;
>>>      default:
>>>          hw_error("vfio: unsupported write size, %d bytes", size);
>>> @@ -1112,10 +1112,10 @@ static uint64_t vfio_bar_read(void *opaque,
>>>          data = buf.byte;
>>>          break;
>>>      case 2:
>>> -        data = le16_to_cpu(buf.word);
>>> +        data = buf.word;
>>>          break;
>>>      case 4:
>>> -        data = le32_to_cpu(buf.dword);
>>> +        data = buf.dword;
>>>          break;
>>>      default:
>>>          hw_error("vfio: unsupported read size, %d bytes", size);
>>> @@ -1142,7 +1142,7 @@ static uint64_t vfio_bar_read(void *opaque,
>>>  static const MemoryRegionOps vfio_bar_ops = {
>>>      .read = vfio_bar_read,
>>>      .write = vfio_bar_write,
>>> -    .endianness = DEVICE_LITTLE_ENDIAN,
>>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>>  };
>>>  
>>>  static void vfio_pci_load_rom(VFIODevice *vdev)
>>> @@ -1204,21 +1204,42 @@ static void vfio_pci_load_rom(VFIODevice *vdev)
>>>  static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)
>>>  {
>>>      VFIODevice *vdev = opaque;
>>> -    uint64_t val = ((uint64_t)1 << (size * 8)) - 1;
>>> +    union {
>>> +        uint8_t byte;
>>> +        uint16_t word;
>>> +        uint32_t dword;
>>> +        uint64_t qword;
>>> +    } buf;
>>> +    uint64_t data = 0;
>>>  
>>>      /* Load the ROM lazily when the guest tries to read it */
>>>      if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
>>>          vfio_pci_load_rom(vdev);
>>>      }
>>>  
>>> -    memcpy(&val, vdev->rom + addr,
>>> +    memcpy(&buf, vdev->rom + addr,
>>>             (addr < vdev->rom_size) ? MIN(size, vdev->rom_size - addr) : 0);
>>>  
>>> +    switch (size) {
>>> +    case 1:
>>> +        data = buf.byte;
>>> +        break;
>>> +    case 2:
>>> +        data = buf.word;
>>> +        break;
>>> +    case 4:
>>> +        data = buf.dword;
>>> +        break;
>>> +    default:
>>> +        hw_error("vfio: unsupported read size, %d bytes", size);
>>> +        break;
>>> +    }
>>> +
>>>      DPRINTF("%s(%04x:%02x:%02x.%x, 0x%"HWADDR_PRIx", 0x%x) = 0x%"PRIx64"\n",
>>>              __func__, vdev->host.domain, vdev->host.bus, vdev->host.slot,
>>> -            vdev->host.function, addr, size, val);
>>> +            vdev->host.function, addr, size, data);
>>>  
>>> -    return val;
>>> +    return data;
>>>  }
>>>  
>>>  static void vfio_rom_write(void *opaque, hwaddr addr,
>>> @@ -1229,7 +1250,7 @@ static void vfio_rom_write(void *opaque, hwaddr addr,
>>>  static const MemoryRegionOps vfio_rom_ops = {
>>>      .read = vfio_rom_read,
>>>      .write = vfio_rom_write,
>>> -    .endianness = DEVICE_LITTLE_ENDIAN,
>>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>>  };
>>>  
>>>  static bool vfio_blacklist_opt_rom(VFIODevice *vdev)
>>
>>
>>
> 
> 


-- 
Alexey

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] vfio: Make BARs native endian
  2014-06-19  1:53     ` Alexey Kardashevskiy
@ 2014-06-20  3:21       ` Alex Williamson
  0 siblings, 0 replies; 5+ messages in thread
From: Alex Williamson @ 2014-06-20  3:21 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-devel, Nikunj A Dadhania, Alexander Graf

On Thu, 2014-06-19 at 11:53 +1000, Alexey Kardashevskiy wrote:
> On 06/19/2014 10:42 AM, Alexey Kardashevskiy wrote:
> > On 06/19/2014 04:57 AM, Alex Williamson wrote:
> >> On Wed, 2014-06-18 at 21:35 +1000, Alexey Kardashevskiy wrote:
> >>> Slow BAR access path is used when VFIO fails to mmap() BAR or TCG is used.
> >>
> >> TCG can use the mmap too
> > 
> > Oh, right.
> 
> I'll remove "tcg" and what else is missing here? I thought the original
> commit log already had all answers on the questions you askes (or clarified
> :) )

Yeah, it seems like it's correct otherwise and the patch itself looks
ok.  Thanks,

Alex

> >>> Since this is just a transport between the guest and a device, there is
> >>> no need to do endianness swapping.
> >>>
> >>> This changes BARs to use native endianness. Since non-ROM BARs were
> >>> doing byte swapping, we need to remove it so does the patch. No change
> >>> in behavior is expected though.
> >>>
> >>> ROM BARs were declared little endian but byte swapping was not
> >>> implemented for them so they never actually worked on big endian systems.
> >>> This fixes endiannes for ROM BARs: this declares them native endian and
> >>> fixes access sizes.
> >>
> >> So the only actual behavior change we expect from this is to fix ROM
> >> access on big endian where we had an unbalanced set of byte swaps.  BAR
> >> access worked because the byte swapping in the read/write handler
> >> canceled the byte swapping in QEMU because we declared the BARs as
> >> little endian.  Otherwise we're just removing canceling byte swaps for
> >> big endian.  Is that correct?
> > 
> > Yes.
> > 
> >>
> >> Also, as the kernel patch appears to not produce any behavioral change,
> >> only eliminates canceling swaps, there's no dependency between patches,
> >> right?  Thanks,
> > 
> > That is right.
> 
> 
> 
> 
> 
> 
> > 
> >>
> >> Alex
> >>
> >>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >>> ---
> >>>  hw/misc/vfio.c | 41 +++++++++++++++++++++++++++++++----------
> >>>  1 file changed, 31 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> >>> index 7437c2e..3eb3c71 100644
> >>> --- a/hw/misc/vfio.c
> >>> +++ b/hw/misc/vfio.c
> >>> @@ -1052,10 +1052,10 @@ static void vfio_bar_write(void *opaque, hwaddr addr,
> >>>          buf.byte = data;
> >>>          break;
> >>>      case 2:
> >>> -        buf.word = cpu_to_le16(data);
> >>> +        buf.word = data;
> >>>          break;
> >>>      case 4:
> >>> -        buf.dword = cpu_to_le32(data);
> >>> +        buf.dword = data;
> >>>          break;
> >>>      default:
> >>>          hw_error("vfio: unsupported write size, %d bytes", size);
> >>> @@ -1112,10 +1112,10 @@ static uint64_t vfio_bar_read(void *opaque,
> >>>          data = buf.byte;
> >>>          break;
> >>>      case 2:
> >>> -        data = le16_to_cpu(buf.word);
> >>> +        data = buf.word;
> >>>          break;
> >>>      case 4:
> >>> -        data = le32_to_cpu(buf.dword);
> >>> +        data = buf.dword;
> >>>          break;
> >>>      default:
> >>>          hw_error("vfio: unsupported read size, %d bytes", size);
> >>> @@ -1142,7 +1142,7 @@ static uint64_t vfio_bar_read(void *opaque,
> >>>  static const MemoryRegionOps vfio_bar_ops = {
> >>>      .read = vfio_bar_read,
> >>>      .write = vfio_bar_write,
> >>> -    .endianness = DEVICE_LITTLE_ENDIAN,
> >>> +    .endianness = DEVICE_NATIVE_ENDIAN,
> >>>  };
> >>>  
> >>>  static void vfio_pci_load_rom(VFIODevice *vdev)
> >>> @@ -1204,21 +1204,42 @@ static void vfio_pci_load_rom(VFIODevice *vdev)
> >>>  static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)
> >>>  {
> >>>      VFIODevice *vdev = opaque;
> >>> -    uint64_t val = ((uint64_t)1 << (size * 8)) - 1;
> >>> +    union {
> >>> +        uint8_t byte;
> >>> +        uint16_t word;
> >>> +        uint32_t dword;
> >>> +        uint64_t qword;
> >>> +    } buf;
> >>> +    uint64_t data = 0;
> >>>  
> >>>      /* Load the ROM lazily when the guest tries to read it */
> >>>      if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
> >>>          vfio_pci_load_rom(vdev);
> >>>      }
> >>>  
> >>> -    memcpy(&val, vdev->rom + addr,
> >>> +    memcpy(&buf, vdev->rom + addr,
> >>>             (addr < vdev->rom_size) ? MIN(size, vdev->rom_size - addr) : 0);
> >>>  
> >>> +    switch (size) {
> >>> +    case 1:
> >>> +        data = buf.byte;
> >>> +        break;
> >>> +    case 2:
> >>> +        data = buf.word;
> >>> +        break;
> >>> +    case 4:
> >>> +        data = buf.dword;
> >>> +        break;
> >>> +    default:
> >>> +        hw_error("vfio: unsupported read size, %d bytes", size);
> >>> +        break;
> >>> +    }
> >>> +
> >>>      DPRINTF("%s(%04x:%02x:%02x.%x, 0x%"HWADDR_PRIx", 0x%x) = 0x%"PRIx64"\n",
> >>>              __func__, vdev->host.domain, vdev->host.bus, vdev->host.slot,
> >>> -            vdev->host.function, addr, size, val);
> >>> +            vdev->host.function, addr, size, data);
> >>>  
> >>> -    return val;
> >>> +    return data;
> >>>  }
> >>>  
> >>>  static void vfio_rom_write(void *opaque, hwaddr addr,
> >>> @@ -1229,7 +1250,7 @@ static void vfio_rom_write(void *opaque, hwaddr addr,
> >>>  static const MemoryRegionOps vfio_rom_ops = {
> >>>      .read = vfio_rom_read,
> >>>      .write = vfio_rom_write,
> >>> -    .endianness = DEVICE_LITTLE_ENDIAN,
> >>> +    .endianness = DEVICE_NATIVE_ENDIAN,
> >>>  };
> >>>  
> >>>  static bool vfio_blacklist_opt_rom(VFIODevice *vdev)
> >>
> >>
> >>
> > 
> > 
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-06-20  3:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-18 11:35 [Qemu-devel] [PATCH] vfio: Make BARs native endian Alexey Kardashevskiy
2014-06-18 18:57 ` Alex Williamson
2014-06-19  0:42   ` Alexey Kardashevskiy
2014-06-19  1:53     ` Alexey Kardashevskiy
2014-06-20  3:21       ` Alex Williamson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.