Patchwork [v3,3/8] prep_pci: Update I/O to MemoryRegion ops

login
register
mail settings
Submitter Andreas Färber
Date Jan. 13, 2012, 3:09 a.m.
Message ID <1326424168-15705-4-git-send-email-andreas.faerber@web.de>
Download mbox | patch
Permalink /patch/135706/
State New
Headers show

Comments

Andreas Färber - Jan. 13, 2012, 3:09 a.m.
Convert to new-style read/write callbacks.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Avi Kivity <avi@redhat.com>
Cc: Benoît Canet <benoit.canet@gmail.com>
---
 hw/prep_pci.c |   61 +++++++++++++++++++++-----------------------------------
 1 files changed, 23 insertions(+), 38 deletions(-)
Avi Kivity - Jan. 15, 2012, 9:19 a.m.
On 01/13/2012 05:09 AM, Andreas Färber wrote:
> Convert to new-style read/write callbacks.
>
>  
> -static uint32_t PPC_PCIIO_readl (void *opaque, target_phys_addr_t addr)
> +static uint64_t ppc_pci_io_read(void *opaque, target_phys_addr_t addr,
> +                                unsigned int size)
>  {
>      PREPPCIState *s = opaque;
> -    uint32_t val;
> -    val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 4);
> -    return val;
> +    switch (size) {
> +    case 1:
> +    case 2:
> +    case 4:
> +        return pci_data_read(s->bus, PPC_PCIIO_config(addr), size);
> +    default:
> +        abort();
> +    }
>  }

Huh? just call pci_data_read() unconditionally.
Andreas Färber - Jan. 16, 2012, 3:08 p.m.
Am 15.01.2012 10:19, schrieb Avi Kivity:
> On 01/13/2012 05:09 AM, Andreas Färber wrote:
>> Convert to new-style read/write callbacks.
>>
>>  
>> -static uint32_t PPC_PCIIO_readl (void *opaque, target_phys_addr_t addr)
>> +static uint64_t ppc_pci_io_read(void *opaque, target_phys_addr_t addr,
>> +                                unsigned int size)
>>  {
>>      PREPPCIState *s = opaque;
>> -    uint32_t val;
>> -    val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 4);
>> -    return val;
>> +    switch (size) {
>> +    case 1:
>> +    case 2:
>> +    case 4:
>> +        return pci_data_read(s->bus, PPC_PCIIO_config(addr), size);
>> +    default:
>> +        abort();
>> +    }
>>  }
> 
> Huh? just call pci_data_read() unconditionally.

Just so that I understand, is that because PReP is 32-bit ppc? In the
above mechanical conversion, size 8 would abort.

BTW did we agree on an indentation style for switch?

Andreas
Avi Kivity - Jan. 16, 2012, 3:15 p.m.
On 01/16/2012 05:08 PM, Andreas Färber wrote:
> Am 15.01.2012 10:19, schrieb Avi Kivity:
> > On 01/13/2012 05:09 AM, Andreas Färber wrote:
> >> Convert to new-style read/write callbacks.
> >>
> >>  
> >> -static uint32_t PPC_PCIIO_readl (void *opaque, target_phys_addr_t addr)
> >> +static uint64_t ppc_pci_io_read(void *opaque, target_phys_addr_t addr,
> >> +                                unsigned int size)
> >>  {
> >>      PREPPCIState *s = opaque;
> >> -    uint32_t val;
> >> -    val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 4);
> >> -    return val;
> >> +    switch (size) {
> >> +    case 1:
> >> +    case 2:
> >> +    case 4:
> >> +        return pci_data_read(s->bus, PPC_PCIIO_config(addr), size);
> >> +    default:
> >> +        abort();
> >> +    }
> >>  }
> > 
> > Huh? just call pci_data_read() unconditionally.
>
> Just so that I understand, is that because PReP is 32-bit ppc? In the
> above mechanical conversion, size 8 would abort.

The memory core never issues size 8 transactions, since the code is
unprepared for it.  When we will support it, you'll have to explicitly
declare it with .impl.max_access_size = 8 or something.

> BTW did we agree on an indentation style for switch?

What you wrote conforms to the de facto standard.

Patch

diff --git a/hw/prep_pci.c b/hw/prep_pci.c
index edfb25d..5970196 100644
--- a/hw/prep_pci.c
+++ b/hw/prep_pci.c
@@ -44,53 +44,38 @@  static inline uint32_t PPC_PCIIO_config(target_phys_addr_t addr)
     return (addr & 0x7ff) |  (i << 11);
 }
 
-static void PPC_PCIIO_writeb (void *opaque, target_phys_addr_t addr, uint32_t val)
+static void ppc_pci_io_write(void *opaque, target_phys_addr_t addr,
+                             uint64_t val, unsigned int size)
 {
     PREPPCIState *s = opaque;
-    pci_data_write(s->bus, PPC_PCIIO_config(addr), val, 1);
-}
-
-static void PPC_PCIIO_writew (void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-    PREPPCIState *s = opaque;
-    pci_data_write(s->bus, PPC_PCIIO_config(addr), val, 2);
-}
-
-static void PPC_PCIIO_writel (void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-    PREPPCIState *s = opaque;
-    pci_data_write(s->bus, PPC_PCIIO_config(addr), val, 4);
-}
-
-static uint32_t PPC_PCIIO_readb (void *opaque, target_phys_addr_t addr)
-{
-    PREPPCIState *s = opaque;
-    uint32_t val;
-    val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 1);
-    return val;
-}
-
-static uint32_t PPC_PCIIO_readw (void *opaque, target_phys_addr_t addr)
-{
-    PREPPCIState *s = opaque;
-    uint32_t val;
-    val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 2);
-    return val;
+    switch (size) {
+    case 1:
+    case 2:
+    case 4:
+        pci_data_write(s->bus, PPC_PCIIO_config(addr), val, size);
+        break;
+    default:
+        abort();
+    }
 }
 
-static uint32_t PPC_PCIIO_readl (void *opaque, target_phys_addr_t addr)
+static uint64_t ppc_pci_io_read(void *opaque, target_phys_addr_t addr,
+                                unsigned int size)
 {
     PREPPCIState *s = opaque;
-    uint32_t val;
-    val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 4);
-    return val;
+    switch (size) {
+    case 1:
+    case 2:
+    case 4:
+        return pci_data_read(s->bus, PPC_PCIIO_config(addr), size);
+    default:
+        abort();
+    }
 }
 
 static const MemoryRegionOps PPC_PCIIO_ops = {
-    .old_mmio = {
-        .read = { PPC_PCIIO_readb, PPC_PCIIO_readw, PPC_PCIIO_readl, },
-        .write = { PPC_PCIIO_writeb, PPC_PCIIO_writew, PPC_PCIIO_writel, },
-    },
+    .read = ppc_pci_io_read,
+    .write = ppc_pci_io_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
 };