[Qemu-devel] [PULL for-1.4 v2] Memory API ioport cleanups

[Qemu-devel] [PULL for-1.4 v2] Memory API ioport cleanups

[Andreas Färber]

Hello,

Here's a bugfix and a few more I/O port conversions to Memory API. Please pull.

Cc: Julien Grall <julien.grall@citrix.com>
Cc: Hervé Poussineau <hpoussin@reactos.org>


The following changes since commit cf7c3f0cb5a7129f57fa9e69d410d6a05031988c:

  virtio-9p: fix compilation error. (2013-01-14 18:52:39 -0600)

are available in the git repository at:

  git://github.com/afaerber/qemu-cpu.git memory-ioport

for you to fetch changes up to c3a29809e4d8924a0cfffd7f1af3c2f3c46f5889:

  acpi_piix4: Do not use old_portio-style callbacks (2013-01-15 19:45:45 +0100)

----------------------------------------------------------------
Hervé Poussineau (2):
      xen_platform: Do not use old_portio-style callbacks
      acpi_piix4: Do not use old_portio-style callbacks

Julien Grall (1):
      hw/dma.c: Fix conversion of ioport_register* to MemoryRegion

 hw/acpi_piix4.c   |   92 ++++++++++++++++++++++++-----------------------------
 hw/dma.c          |   22 ++++++-------
 hw/xen_platform.c |   21 ++++++------
 3 Dateien geändert, 62 Zeilen hinzugefügt(+), 73 Zeilen entfernt(-)

[Qemu-devel] [PATCH 1/3] hw/dma.c: Fix conversion of ioport_register* to MemoryRegion

[Andreas Färber]

From: Julien Grall <julien.grall@citrix.com>

The commit 582299336879504353e60c7937fbc70fea93f3da introduced a 1-shift for
some offset in DMA emulation.

Before the previous commit, which converted ioport_register_* to
MemoryRegion, the DMA controller registered 8 ioports with the following
formula:
base + ((8 + i) << d->shift) where 0 <= i < 8
When an IO occured within a Memory Region, DMA callback receives an
offset relative to the start address. Here the start address is:
base + (8 << d->shift).
The offset should be: (i << d->shift). After the shift is reverted, the
offsets are 0..7 not 1..8.

Fixes LP#1089996.

Reported-by: Andreas Gustafsson <gson@gson.org>
Signed-off-by: Julien Grall <julien.grall@citrix.com>
Tested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/dma.c |   22 +++++++++++-----------
 1 Datei geändert, 11 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)

diff --git a/hw/dma.c b/hw/dma.c
index 0634baa..5bdf435 100644
--- a/hw/dma.c
+++ b/hw/dma.c
@@ -201,7 +201,7 @@ static void write_cont(void *opaque, hwaddr nport, uint64_t data,
 
     iport = (nport >> d->dshift) & 0x0f;
     switch (iport) {
-    case 0x01:                  /* command */
+    case 0x00:                  /* command */
         if ((data != 0) && (data & CMD_NOT_SUPPORTED)) {
             dolog("command %"PRIx64" not supported\n", data);
             return;
@@ -209,7 +209,7 @@ static void write_cont(void *opaque, hwaddr nport, uint64_t data,
         d->command = data;
         break;
 
-    case 0x02:
+    case 0x01:
         ichan = data & 3;
         if (data & 4) {
             d->status |= 1 << (ichan + 4);
@@ -221,7 +221,7 @@ static void write_cont(void *opaque, hwaddr nport, uint64_t data,
         DMA_run();
         break;
 
-    case 0x03:                  /* single mask */
+    case 0x02:                  /* single mask */
         if (data & 4)
             d->mask |= 1 << (data & 3);
         else
@@ -229,7 +229,7 @@ static void write_cont(void *opaque, hwaddr nport, uint64_t data,
         DMA_run();
         break;
 
-    case 0x04:                  /* mode */
+    case 0x03:                  /* mode */
         {
             ichan = data & 3;
 #ifdef DEBUG_DMA
@@ -248,23 +248,23 @@ static void write_cont(void *opaque, hwaddr nport, uint64_t data,
             break;
         }
 
-    case 0x05:                  /* clear flip flop */
+    case 0x04:                  /* clear flip flop */
         d->flip_flop = 0;
         break;
 
-    case 0x06:                  /* reset */
+    case 0x05:                  /* reset */
         d->flip_flop = 0;
         d->mask = ~0;
         d->status = 0;
         d->command = 0;
         break;
 
-    case 0x07:                  /* clear mask for all channels */
+    case 0x06:                  /* clear mask for all channels */
         d->mask = 0;
         DMA_run();
         break;
 
-    case 0x08:                  /* write mask for all channels */
+    case 0x07:                  /* write mask for all channels */
         d->mask = data;
         DMA_run();
         break;
@@ -289,11 +289,11 @@ static uint64_t read_cont(void *opaque, hwaddr nport, unsigned size)
 
     iport = (nport >> d->dshift) & 0x0f;
     switch (iport) {
-    case 0x08:                  /* status */
+    case 0x00:                  /* status */
         val = d->status;
         d->status &= 0xf0;
         break;
-    case 0x0f:                  /* mask */
+    case 0x01:                  /* mask */
         val = d->mask;
         break;
     default:
@@ -468,7 +468,7 @@ void DMA_schedule(int nchan)
 static void dma_reset(void *opaque)
 {
     struct dma_cont *d = opaque;
-    write_cont(d, (0x06 << d->dshift), 0, 1);
+    write_cont(d, (0x05 << d->dshift), 0, 1);
 }
 
 static int dma_phony_handler (void *opaque, int nchan, int dma_pos, int dma_len)
-- 
1.7.10.4

[Qemu-devel] [PATCH 2/3] xen_platform: Do not use old_portio-style callbacks

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/xen_platform.c |   21 ++++++++++-----------
 1 Datei geändert, 10 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)

diff --git a/hw/xen_platform.c b/hw/xen_platform.c
index ca66047..8866468 100644
--- a/hw/xen_platform.c
+++ b/hw/xen_platform.c
@@ -279,7 +279,8 @@ static void platform_fixed_ioport_init(PCIXenPlatformState* s)
 
 /* Xen Platform PCI Device */
 
-static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr)
+static uint64_t xen_platform_ioport_readb(void *opaque, hwaddr addr,
+                                          unsigned int size)
 {
     if (addr == 0) {
         return platform_fixed_ioport_readb(opaque, 0);
@@ -288,30 +289,28 @@ static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr)
     }
 }
 
-static void xen_platform_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
+static void xen_platform_ioport_writeb(void *opaque, hwaddr addr,
+                                       uint64_t val, unsigned int size)
 {
     PCIXenPlatformState *s = opaque;
 
     switch (addr) {
     case 0: /* Platform flags */
-        platform_fixed_ioport_writeb(opaque, 0, val);
+        platform_fixed_ioport_writeb(opaque, 0, (uint32_t)val);
         break;
     case 8:
-        log_writeb(s, val);
+        log_writeb(s, (uint32_t)val);
         break;
     default:
         break;
     }
 }
 
-static MemoryRegionPortio xen_pci_portio[] = {
-    { 0, 0x100, 1, .read = xen_platform_ioport_readb, },
-    { 0, 0x100, 1, .write = xen_platform_ioport_writeb, },
-    PORTIO_END_OF_LIST()
-};
-
 static const MemoryRegionOps xen_pci_io_ops = {
-    .old_portio = xen_pci_portio,
+    .read  = xen_platform_ioport_readb,
+    .write = xen_platform_ioport_writeb,
+    .impl.min_access_size = 1,
+    .impl.max_access_size = 1,
 };
 
 static void platform_ioport_bar_setup(PCIXenPlatformState *d)
-- 
1.7.10.4

[Qemu-devel] [PATCH 3/3] acpi_piix4: Do not use old_portio-style callbacks

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
[AF: Used HWADDR_PRIx for hwaddr PIIX4_DPRINTF()]
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/acpi_piix4.c |   92 +++++++++++++++++++++++++------------------------------
 1 Datei geändert, 41 Zeilen hinzugefügt(+), 51 Zeilen entfernt(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 2f84b4e..0d33849 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -531,68 +531,58 @@ static const MemoryRegionOps piix4_gpe_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static uint32_t pci_up_read(void *opaque, uint32_t addr)
+static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
 {
     PIIX4PMState *s = opaque;
-    uint32_t val;
-
-    /* Manufacture an "up" value to cause a device check on any hotplug
-     * slot with a device.  Extra device checks are harmless. */
-    val = s->pci0_slot_device_present & s->pci0_hotplug_enable;
-
-    PIIX4_DPRINTF("pci_up_read %x\n", val);
-    return val;
-}
-
-static uint32_t pci_down_read(void *opaque, uint32_t addr)
-{
-    PIIX4PMState *s = opaque;
-    uint32_t val = s->pci0_status.down;
+    uint32_t val = 0;
+
+    switch (addr) {
+    case PCI_UP_BASE - PCI_HOTPLUG_ADDR:
+        /* Manufacture an "up" value to cause a device check on any hotplug
+         * slot with a device.  Extra device checks are harmless. */
+        val = s->pci0_slot_device_present & s->pci0_hotplug_enable;
+        PIIX4_DPRINTF("pci_up_read %x\n", val);
+        break;
+    case PCI_DOWN_BASE - PCI_HOTPLUG_ADDR:
+        val = s->pci0_status.down;
+        PIIX4_DPRINTF("pci_down_read %x\n", val);
+        break;
+    case PCI_EJ_BASE - PCI_HOTPLUG_ADDR:
+        /* No feature defined yet */
+        PIIX4_DPRINTF("pci_features_read %x\n", val);
+        break;
+    case PCI_RMV_BASE - PCI_HOTPLUG_ADDR:
+        val = s->pci0_hotplug_enable;
+        break;
+    default:
+        break;
+    }
 
-    PIIX4_DPRINTF("pci_down_read %x\n", val);
     return val;
 }
 
-static uint32_t pci_features_read(void *opaque, uint32_t addr)
+static void pci_write(void *opaque, hwaddr addr, uint64_t data,
+                      unsigned int size)
 {
-    /* No feature defined yet */
-    PIIX4_DPRINTF("pci_features_read %x\n", 0);
-    return 0;
-}
-
-static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
-{
-    acpi_piix_eject_slot(opaque, val);
-
-    PIIX4_DPRINTF("pciej write %x <== %d\n", addr, val);
-}
-
-static uint32_t pcirmv_read(void *opaque, uint32_t addr)
-{
-    PIIX4PMState *s = opaque;
-
-    return s->pci0_hotplug_enable;
+    switch (addr) {
+    case PCI_EJ_BASE - PCI_HOTPLUG_ADDR:
+        acpi_piix_eject_slot(opaque, (uint32_t)data);
+        PIIX4_DPRINTF("pciej write %" HWADDR_PRIx " <== % " PRIu64 "\n",
+                      addr, data);
+        break;
+    default:
+        break;
+    }
 }
 
 static const MemoryRegionOps piix4_pci_ops = {
-    .old_portio = (MemoryRegionPortio[]) {
-        {
-            .offset = PCI_UP_BASE - PCI_HOTPLUG_ADDR,   .len = 4, .size = 4,
-            .read = pci_up_read,
-        },{
-            .offset = PCI_DOWN_BASE - PCI_HOTPLUG_ADDR, .len = 4, .size = 4,
-            .read = pci_down_read,
-        },{
-            .offset = PCI_EJ_BASE - PCI_HOTPLUG_ADDR,   .len = 4, .size = 4,
-            .read = pci_features_read,
-            .write = pciej_write,
-        },{
-            .offset = PCI_RMV_BASE - PCI_HOTPLUG_ADDR,  .len = 4, .size = 4,
-            .read = pcirmv_read,
-        },
-        PORTIO_END_OF_LIST()
-    },
+    .read = pci_read,
+    .write = pci_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
 };
 
 static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
-- 
1.7.10.4

Re: [Qemu-devel] [PULL for-1.4 v2] Memory API ioport cleanups

[Anthony Liguori]

Hi,

Please adjust your pull request script to include a 0/M marker if you're
also including patches in the reply.

No worries about this request, but my new version tracker gets confused
because it marks this whole series as "broken" because it's impossible
for a non-human to tell if this is the cover letter or something silly
like a new top-level patch posted to this thread.

Regards,

Anthony Liguori

Andreas Färber <afaerber@suse.de> writes:

> Hello,
>
> Here's a bugfix and a few more I/O port conversions to Memory API. Please pull.
>
> Cc: Julien Grall <julien.grall@citrix.com>
> Cc: Hervé Poussineau <hpoussin@reactos.org>
>
>
> The following changes since commit cf7c3f0cb5a7129f57fa9e69d410d6a05031988c:
>
>   virtio-9p: fix compilation error. (2013-01-14 18:52:39 -0600)
>
> are available in the git repository at:
>
>   git://github.com/afaerber/qemu-cpu.git memory-ioport
>
> for you to fetch changes up to c3a29809e4d8924a0cfffd7f1af3c2f3c46f5889:
>
>   acpi_piix4: Do not use old_portio-style callbacks (2013-01-15 19:45:45 +0100)
>
> ----------------------------------------------------------------
> Hervé Poussineau (2):
>       xen_platform: Do not use old_portio-style callbacks
>       acpi_piix4: Do not use old_portio-style callbacks
>
> Julien Grall (1):
>       hw/dma.c: Fix conversion of ioport_register* to MemoryRegion
>
>  hw/acpi_piix4.c   |   92 ++++++++++++++++++++++++-----------------------------
>  hw/dma.c          |   22 ++++++-------
>  hw/xen_platform.c |   21 ++++++------
>  3 Dateien geändert, 62 Zeilen hinzugefügt(+), 73 Zeilen entfernt(-)

Re: [Qemu-devel] [PULL for-1.4 v2] Memory API ioport cleanups

[Andreas Färber]

Hi Anthony,

Am 16.01.2013 00:02, schrieb Anthony Liguori:
> 
> Please adjust your pull request script to include a 0/M marker if you're
> also including patches in the reply.

In the past it was you who specifically requested us to always include
the queue of patches part of that pull as reply! Is this no longer desired?

Or how would I generate 0/M from my bash script? git-request-pull
doesn't really fit into the "blurb" of a regular cover letter, coming
with its own stats, so that I generate the mail headers by script.

Regards,
Andreas

> No worries about this request, but my new version tracker gets confused
> because it marks this whole series as "broken" because it's impossible
> for a non-human to tell if this is the cover letter or something silly
> like a new top-level patch posted to this thread.
> 
> Regards,
> 
> Anthony Liguori

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PULL for-1.4 v2] Memory API ioport cleanups

[Anthony Liguori]

Andreas Färber <afaerber@suse.de> writes:

> Hi Anthony,
>
> Am 16.01.2013 00:02, schrieb Anthony Liguori:
>> 
>> Please adjust your pull request script to include a 0/M marker if you're
>> also including patches in the reply.
>
> In the past it was you who specifically requested us to always include
> the queue of patches part of that pull as reply! Is this no longer
> desired?

Yes, it is.  My patches tool handles PULL requests without them fine but
for QEMU, I want pull requests to include the patches being pulled.

If you're including patches (and you should), then the first mail should
indicate that it's the 0th mail of the series.

> Or how would I generate 0/M from my bash script? git-request-pull
> doesn't really fit into the "blurb" of a regular cover letter, coming
> with its own stats, so that I generate the mail headers by script.

I don't send pull requests so I'm the wrong person to ask.  Peter/Stefan, how
do ya'll do it?

Regards,

Anthony Liguori

>
> Regards,
> Andreas
>
>> No worries about this request, but my new version tracker gets confused
>> because it marks this whole series as "broken" because it's impossible
>> for a non-human to tell if this is the cover letter or something silly
>> like a new top-level patch posted to this thread.
>> 
>> Regards,
>> 
>> Anthony Liguori
>
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PULL for-1.4 v2] Memory API ioport cleanups

[Anthony Liguori]

Pulled, thanks.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PULL for-1.4 v2] Memory API ioport cleanups

[Peter Maydell]

On 16 January 2013 00:35, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Andreas Färber <afaerber@suse.de> writes:
>> Or how would I generate 0/M from my bash script? git-request-pull
>> doesn't really fit into the "blurb" of a regular cover letter, coming
>> with its own stats, so that I generate the mail headers by script.
>
> I don't send pull requests so I'm the wrong person to ask.  Peter/Stefan, how
> do ya'll do it?

My script is http://people.linaro.org/~pmaydell/make-pullreq
Most of that is sanity checking, but the actual emails I generate by
doing a 'git format-patch --cover-letter', mangling the generated cover
letter a bit with sed to remove the duplicate set of stats, and then
appending the output of git-request-pull. I then edit the cover letter
by hand as needed for the specific pull.

-- PMM

Re: [Qemu-devel] [PULL for-1.4 v2] Memory API ioport cleanups

[Stefan Hajnoczi]

On Wed, Jan 16, 2013 at 11:12:45AM +0000, Peter Maydell wrote:
> On 16 January 2013 00:35, Anthony Liguori <anthony@codemonkey.ws> wrote:
> > Andreas Färber <afaerber@suse.de> writes:
> >> Or how would I generate 0/M from my bash script? git-request-pull
> >> doesn't really fit into the "blurb" of a regular cover letter, coming
> >> with its own stats, so that I generate the mail headers by script.
> >
> > I don't send pull requests so I'm the wrong person to ask.  Peter/Stefan, how
> > do ya'll do it?
> 
> My script is http://people.linaro.org/~pmaydell/make-pullreq
> Most of that is sanity checking, but the actual emails I generate by
> doing a 'git format-patch --cover-letter', mangling the generated cover
> letter a bit with sed to remove the duplicate set of stats, and then
> appending the output of git-request-pull. I then edit the cover letter
> by hand as needed for the specific pull.

I do the same but manually right now :).

Stefan