* [Qemu-devel] [PATCH] ahci: fix sysbus support
@ 2014-02-10 23:02 Rob Herring
2014-02-11 0:32 ` Peter Maydell
2014-03-18 15:46 ` Peter Maydell
0 siblings, 2 replies; 5+ messages in thread
From: Rob Herring @ 2014-02-10 23:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Rob Herring
From: Rob Herring <rob.herring@linaro.org>
Non-PCI AHCI support is broken due to assertion failures when trying
to convert AHCIState to a PCIDevice pointer as AHCIState can have
different container structs. Fix this by using the non-asserting object
cast and checking the returned pointer is not NULL.
The AddressSpace pointer is also being initialized to NULL and causing
dma_memory_map call to fail. Fix this by initializing to
address_space_memory for sysbus instances.
Also correct AHCI_VMSTATE to use the correct container SysbusAHCIState
for sysbus instances.
Signed-off-by: Rob Herring <rob.herring@linaro.org>
---
hw/ide/ahci.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index fbea9e8..55f984e 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -118,11 +118,11 @@ static uint32_t ahci_port_read(AHCIState *s, int port, int offset)
static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
{
AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
- PCIDevice *pci_dev = PCI_DEVICE(d);
+ PCIDevice *pci_dev = (PCIDevice *)object_dynamic_cast(OBJECT(d), TYPE_PCI_DEVICE);
DPRINTF(0, "raise irq\n");
- if (msi_enabled(pci_dev)) {
+ if (pci_dev && msi_enabled(pci_dev)) {
msi_notify(pci_dev, 0);
} else {
qemu_irq_raise(s->irq);
@@ -132,10 +132,11 @@ static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
static void ahci_irq_lower(AHCIState *s, AHCIDevice *dev)
{
AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
+ PCIDevice *pci_dev = (PCIDevice *)object_dynamic_cast(OBJECT(d), TYPE_PCI_DEVICE);
DPRINTF(0, "lower irq\n");
- if (!msi_enabled(PCI_DEVICE(d))) {
+ if (!pci_dev || !msi_enabled(pci_dev)) {
qemu_irq_lower(s->irq);
}
}
@@ -1311,7 +1312,7 @@ static const VMStateDescription vmstate_sysbus_ahci = {
.name = "sysbus-ahci",
.unmigratable = 1, /* Still buggy under I/O load */
.fields = (VMStateField []) {
- VMSTATE_AHCI(ahci, AHCIPCIState),
+ VMSTATE_AHCI(ahci, SysbusAHCIState),
VMSTATE_END_OF_LIST()
},
};
@@ -1328,7 +1329,7 @@ static void sysbus_ahci_realize(DeviceState *dev, Error **errp)
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
SysbusAHCIState *s = SYSBUS_AHCI(dev);
- ahci_init(&s->ahci, dev, NULL, s->num_ports);
+ ahci_init(&s->ahci, dev, &address_space_memory, s->num_ports);
sysbus_init_mmio(sbd, &s->ahci.mem);
sysbus_init_irq(sbd, &s->ahci.irq);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] ahci: fix sysbus support
2014-02-10 23:02 [Qemu-devel] [PATCH] ahci: fix sysbus support Rob Herring
@ 2014-02-11 0:32 ` Peter Maydell
2014-03-18 0:11 ` Rob Herring
2014-03-18 7:28 ` Paolo Bonzini
2014-03-18 15:46 ` Peter Maydell
1 sibling, 2 replies; 5+ messages in thread
From: Peter Maydell @ 2014-02-11 0:32 UTC (permalink / raw)
To: Rob Herring
Cc: Rob Herring, Jason Baron, QEMU Developers, Andreas Färber
On 10 February 2014 23:02, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@linaro.org>
>
> Non-PCI AHCI support is broken due to assertion failures when trying
> to convert AHCIState to a PCIDevice pointer as AHCIState can have
> different container structs. Fix this by using the non-asserting object
> cast and checking the returned pointer is not NULL.
>
> The AddressSpace pointer is also being initialized to NULL and causing
> dma_memory_map call to fail. Fix this by initializing to
> address_space_memory for sysbus instances.
>
> Also correct AHCI_VMSTATE to use the correct container SysbusAHCIState
> for sysbus instances.
>
> Signed-off-by: Rob Herring <rob.herring@linaro.org>
> ---
> hw/ide/ahci.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index fbea9e8..55f984e 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -118,11 +118,11 @@ static uint32_t ahci_port_read(AHCIState *s, int port, int offset)
> static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
> {
> AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
> - PCIDevice *pci_dev = PCI_DEVICE(d);
> + PCIDevice *pci_dev = (PCIDevice *)object_dynamic_cast(OBJECT(d), TYPE_PCI_DEVICE);
>
> DPRINTF(0, "raise irq\n");
>
> - if (msi_enabled(pci_dev)) {
> + if (pci_dev && msi_enabled(pci_dev)) {
> msi_notify(pci_dev, 0);
> } else {
> qemu_irq_raise(s->irq);
> @@ -132,10 +132,11 @@ static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
> static void ahci_irq_lower(AHCIState *s, AHCIDevice *dev)
> {
> AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
> + PCIDevice *pci_dev = (PCIDevice *)object_dynamic_cast(OBJECT(d), TYPE_PCI_DEVICE);
>
> DPRINTF(0, "lower irq\n");
>
> - if (!msi_enabled(PCI_DEVICE(d))) {
> + if (!pci_dev || !msi_enabled(pci_dev)) {
> qemu_irq_lower(s->irq);
> }
> }
Cc'ing Andreas in case he knows a neater way of doing those casts.
> @@ -1311,7 +1312,7 @@ static const VMStateDescription vmstate_sysbus_ahci = {
> .name = "sysbus-ahci",
> .unmigratable = 1, /* Still buggy under I/O load */
> .fields = (VMStateField []) {
> - VMSTATE_AHCI(ahci, AHCIPCIState),
> + VMSTATE_AHCI(ahci, SysbusAHCIState),
> VMSTATE_END_OF_LIST()
> },
...I wonder if that fixes the "buggy under I/O load" issue the
comment is talking about. Jason, do you still have whatever
testcase you were using when you added that comment?
thanks
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] ahci: fix sysbus support
2014-02-11 0:32 ` Peter Maydell
@ 2014-03-18 0:11 ` Rob Herring
2014-03-18 7:28 ` Paolo Bonzini
1 sibling, 0 replies; 5+ messages in thread
From: Rob Herring @ 2014-03-18 0:11 UTC (permalink / raw)
To: Peter Maydell
Cc: Rob Herring, Jason Baron, QEMU Developers, Andreas Färber
On Mon, Feb 10, 2014 at 6:32 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 10 February 2014 23:02, Rob Herring <robherring2@gmail.com> wrote:
>> From: Rob Herring <rob.herring@linaro.org>
>>
>> Non-PCI AHCI support is broken due to assertion failures when trying
>> to convert AHCIState to a PCIDevice pointer as AHCIState can have
>> different container structs. Fix this by using the non-asserting object
>> cast and checking the returned pointer is not NULL.
>>
>> The AddressSpace pointer is also being initialized to NULL and causing
>> dma_memory_map call to fail. Fix this by initializing to
>> address_space_memory for sysbus instances.
>>
>> Also correct AHCI_VMSTATE to use the correct container SysbusAHCIState
>> for sysbus instances.
>>
>> Signed-off-by: Rob Herring <rob.herring@linaro.org>
>> ---
>> hw/ide/ahci.c | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index fbea9e8..55f984e 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -118,11 +118,11 @@ static uint32_t ahci_port_read(AHCIState *s, int port, int offset)
>> static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
>> {
>> AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
>> - PCIDevice *pci_dev = PCI_DEVICE(d);
>> + PCIDevice *pci_dev = (PCIDevice *)object_dynamic_cast(OBJECT(d), TYPE_PCI_DEVICE);
>>
>> DPRINTF(0, "raise irq\n");
>>
>> - if (msi_enabled(pci_dev)) {
>> + if (pci_dev && msi_enabled(pci_dev)) {
>> msi_notify(pci_dev, 0);
>> } else {
>> qemu_irq_raise(s->irq);
>> @@ -132,10 +132,11 @@ static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
>> static void ahci_irq_lower(AHCIState *s, AHCIDevice *dev)
>> {
>> AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
>> + PCIDevice *pci_dev = (PCIDevice *)object_dynamic_cast(OBJECT(d), TYPE_PCI_DEVICE);
>>
>> DPRINTF(0, "lower irq\n");
>>
>> - if (!msi_enabled(PCI_DEVICE(d))) {
>> + if (!pci_dev || !msi_enabled(pci_dev)) {
>> qemu_irq_lower(s->irq);
>> }
>> }
>
> Cc'ing Andreas in case he knows a neater way of doing those casts.
Any comments on this? If not, can someone apply this?
>> @@ -1311,7 +1312,7 @@ static const VMStateDescription vmstate_sysbus_ahci = {
>> .name = "sysbus-ahci",
>> .unmigratable = 1, /* Still buggy under I/O load */
>> .fields = (VMStateField []) {
>> - VMSTATE_AHCI(ahci, AHCIPCIState),
>> + VMSTATE_AHCI(ahci, SysbusAHCIState),
>> VMSTATE_END_OF_LIST()
>> },
>
> ...I wonder if that fixes the "buggy under I/O load" issue the
> comment is talking about. Jason, do you still have whatever
> testcase you were using when you added that comment?
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] ahci: fix sysbus support
2014-02-11 0:32 ` Peter Maydell
2014-03-18 0:11 ` Rob Herring
@ 2014-03-18 7:28 ` Paolo Bonzini
1 sibling, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2014-03-18 7:28 UTC (permalink / raw)
To: Peter Maydell, Rob Herring
Cc: Rob Herring, Jason Baron, QEMU Developers, Andreas Färber
Il 11/02/2014 01:32, Peter Maydell ha scritto:
>> > @@ -1311,7 +1312,7 @@ static const VMStateDescription vmstate_sysbus_ahci = {
>> > .name = "sysbus-ahci",
>> > .unmigratable = 1, /* Still buggy under I/O load */
>> > .fields = (VMStateField []) {
>> > - VMSTATE_AHCI(ahci, AHCIPCIState),
>> > + VMSTATE_AHCI(ahci, SysbusAHCIState),
>> > VMSTATE_END_OF_LIST()
>> > },
> ...I wonder if that fixes the "buggy under I/O load" issue the
> comment is talking about. Jason, do you still have whatever
> testcase you were using when you added that comment?
No, because PCI AHCI is also buggy under I/O load.
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] ahci: fix sysbus support
2014-02-10 23:02 [Qemu-devel] [PATCH] ahci: fix sysbus support Rob Herring
2014-02-11 0:32 ` Peter Maydell
@ 2014-03-18 15:46 ` Peter Maydell
1 sibling, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2014-03-18 15:46 UTC (permalink / raw)
To: QEMU Developers; +Cc: Rob Herring, Andreas Färber
On 10 February 2014 23:02, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@linaro.org>
>
> Non-PCI AHCI support is broken due to assertion failures when trying
> to convert AHCIState to a PCIDevice pointer as AHCIState can have
> different container structs. Fix this by using the non-asserting object
> cast and checking the returned pointer is not NULL.
>
> The AddressSpace pointer is also being initialized to NULL and causing
> dma_memory_map call to fail. Fix this by initializing to
> address_space_memory for sysbus instances.
>
> Also correct AHCI_VMSTATE to use the correct container SysbusAHCIState
> for sysbus instances.
>
> Signed-off-by: Rob Herring <rob.herring@linaro.org>
Andreas just confirmed on IRC that the cast syntax you've
used is fine, so I'll take this into target-arm.next
(since Highbank is the only user of sysbus-ahci). I
expect to get this into rc1.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-03-18 15:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-10 23:02 [Qemu-devel] [PATCH] ahci: fix sysbus support Rob Herring
2014-02-11 0:32 ` Peter Maydell
2014-03-18 0:11 ` Rob Herring
2014-03-18 7:28 ` Paolo Bonzini
2014-03-18 15:46 ` Peter Maydell
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.