diff mbox

[5/8] apb: fix endianness for APB and PCI config accesses

Message ID 1499810007-28613-6-git-send-email-mark.cave-ayland@ilande.co.uk
State New
Headers show

Commit Message

Mark Cave-Ayland July 11, 2017, 9:53 p.m. UTC 
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/pci-host/apb.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Artyom Tarasenko July 12, 2017, 9:09 a.m. UTC | #1 
On Tue, Jul 11, 2017 at 11:53 PM, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/pci-host/apb.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 622c341..5ad7678 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -561,7 +561,7 @@ static uint64_t apb_config_readl (void *opaque,
>  static const MemoryRegionOps apb_config_ops = {
>      .read = apb_config_readl,
>      .write = apb_config_writel,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_BIG_ENDIAN,

Is this correct? I thought all the PCI config registers are always
little-endian.

>  };
>
>  static void apb_pci_config_write(void *opaque, hwaddr addr,
> @@ -570,7 +570,6 @@ static void apb_pci_config_write(void *opaque, hwaddr addr,
>      APBState *s = opaque;
>      PCIHostState *phb = PCI_HOST_BRIDGE(s);
>
> -    val = qemu_bswap_len(val, size);
>      APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__, addr, val);
>      pci_data_write(phb->bus, addr, val, size);
>  }
> @@ -583,7 +582,6 @@ static uint64_t apb_pci_config_read(void *opaque, hwaddr addr,
>      PCIHostState *phb = PCI_HOST_BRIDGE(s);
>
>      ret = pci_data_read(phb->bus, addr, size);
> -    ret = qemu_bswap_len(ret, size);
>      APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, ret);
>      return ret;
>  }
> @@ -744,7 +742,7 @@ static void pci_pbm_reset(DeviceState *d)
>  static const MemoryRegionOps pci_config_ops = {
>      .read = apb_pci_config_read,
>      .write = apb_pci_config_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>
>  static int pci_pbm_init_device(SysBusDevice *dev)
> --
> 1.7.10.4
>
Mark Cave-Ayland July 14, 2017, 10:01 a.m. UTC | #2 
On 12/07/17 10:09, Artyom Tarasenko wrote:

> On Tue, Jul 11, 2017 at 11:53 PM, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/pci-host/apb.c |    6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
>> index 622c341..5ad7678 100644
>> --- a/hw/pci-host/apb.c
>> +++ b/hw/pci-host/apb.c
>> @@ -561,7 +561,7 @@ static uint64_t apb_config_readl (void *opaque,
>>  static const MemoryRegionOps apb_config_ops = {
>>      .read = apb_config_readl,
>>      .write = apb_config_writel,
>> -    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .endianness = DEVICE_BIG_ENDIAN,
> 
> Is this correct? I thought all the PCI config registers are always
> little-endian.

Yes - these are the registers on the APB itself...

>>  };
>>
>>  static void apb_pci_config_write(void *opaque, hwaddr addr,
>> @@ -570,7 +570,6 @@ static void apb_pci_config_write(void *opaque, hwaddr addr,
>>      APBState *s = opaque;
>>      PCIHostState *phb = PCI_HOST_BRIDGE(s);
>>
>> -    val = qemu_bswap_len(val, size);
>>      APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__, addr, val);
>>      pci_data_write(phb->bus, addr, val, size);
>>  }
>> @@ -583,7 +582,6 @@ static uint64_t apb_pci_config_read(void *opaque, hwaddr addr,
>>      PCIHostState *phb = PCI_HOST_BRIDGE(s);
>>
>>      ret = pci_data_read(phb->bus, addr, size);
>> -    ret = qemu_bswap_len(ret, size);
>>      APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, ret);
>>      return ret;
>>  }
>> @@ -744,7 +742,7 @@ static void pci_pbm_reset(DeviceState *d)
>>  static const MemoryRegionOps pci_config_ops = {
>>      .read = apb_pci_config_read,
>>      .write = apb_pci_config_write,
>> -    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,

... while these are the PCI config registers. It's this endian change
here that makes it possible to remove the qemu_bswap_len() calls
included in the diff above.


ATB,

Mark.
diff mbox

Patch

diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 622c341..5ad7678 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -561,7 +561,7 @@  static uint64_t apb_config_readl (void *opaque,
 static const MemoryRegionOps apb_config_ops = {
     .read = apb_config_readl,
     .write = apb_config_writel,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_BIG_ENDIAN,
 };
 
 static void apb_pci_config_write(void *opaque, hwaddr addr,
@@ -570,7 +570,6 @@  static void apb_pci_config_write(void *opaque, hwaddr addr,
     APBState *s = opaque;
     PCIHostState *phb = PCI_HOST_BRIDGE(s);
 
-    val = qemu_bswap_len(val, size);
     APB_DPRINTF("%s: addr " TARGET_FMT_plx " val %" PRIx64 "\n", __func__, addr, val);
     pci_data_write(phb->bus, addr, val, size);
 }
@@ -583,7 +582,6 @@  static uint64_t apb_pci_config_read(void *opaque, hwaddr addr,
     PCIHostState *phb = PCI_HOST_BRIDGE(s);
 
     ret = pci_data_read(phb->bus, addr, size);
-    ret = qemu_bswap_len(ret, size);
     APB_DPRINTF("%s: addr " TARGET_FMT_plx " -> %x\n", __func__, addr, ret);
     return ret;
 }
@@ -744,7 +742,7 @@  static void pci_pbm_reset(DeviceState *d)
 static const MemoryRegionOps pci_config_ops = {
     .read = apb_pci_config_read,
     .write = apb_pci_config_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static int pci_pbm_init_device(SysBusDevice *dev)