[RFC,02/11] hw/pci: define msi_nonbroken in pci-stub

Commit Message

Philippe Mathieu-Daudé May 8, 2017, 11:39 p.m. UTC

This field is accessed in hw/intc/arm_gicv[23*].c

Signed-off-by: Philippe Mathieu-Daudé <[e-mail]>
---
 hw/pci/pci-stub.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Thomas Huth May 9, 2017, 5:19 a.m. UTC | #1

On 09.05.2017 01:39, Philippe Mathieu-Daudé wrote:
> This field is accessed in hw/intc/arm_gicv[23*].c

default-configs/arm-softmmu.mak sets CONFIG_PCI, so this should not be
necessary, I think. Otherwise, you should extend your patch description,
to elaborate on what you're trying to do here.

 Thomas

Philippe Mathieu-Daudé May 9, 2017, 4:49 p.m. UTC | #2

Hi Thomas,

On 05/09/2017 02:19 AM, Thomas Huth wrote:
> On 09.05.2017 01:39, Philippe Mathieu-Daudé wrote:
>> This field is accessed in hw/intc/arm_gicv[23*].c
>
> default-configs/arm-softmmu.mak sets CONFIG_PCI, so this should not be
> necessary, I think. Otherwise, you should extend your patch description,
> to elaborate on what you're trying to do here.
>
>  Thomas

Sure, I'm willing to add basic support for the TI Hercules MCU which is 
a big endian SoC (without PCI support).

I'm using the following config:

$ cat default-configs/armeb-softmmu.mak
# Default configuration for armeb-softmmu
CONFIG_ECC=y
CONFIG_SERIAL=y
CONFIG_PTIMER=y
CONFIG_SD=y
CONFIG_SSI=y
#CONFIG_USB=y
CONFIG_PLATFORM_BUS=y
CONFIG_ARM_V7M=y
CONFIG_ARM_GIC=y
CONFIG_ARM_TIMER=y
CONFIG_PL011=y
CONFIG_PL022=y
CONFIG_PL031=y
CONFIG_PL041=y
CONFIG_PL050=y
CONFIG_PL061=y
CONFIG_PL080=y
CONFIG_PL181=y
CONFIG_PL190=y
CONFIG_PL310=y
CONFIG_PL330=y
CONFIG_BITBANG_I2C=y
CONFIG_GPIO_KEY=y
CONFIG_HERCULES=y

I had to modify default-configs/arm-softmmu.mak to restrict a bit 
hw/arm/Makefile.objs, see:

https://github.com/qemu/qemu/compare/master...philmd:armeb_wip

In this branch I run:

$ ../configure --target-list=armeb-softmmu && make -j
[...]
   LINK    armeb-softmmu/qemu-system-armeb
../hw/intc/arm_gicv2m.o: In function `gicv2m_realize':
qemu/hw/intc/arm_gicv2m.c:154: undefined reference to `msi_nonbroken'
../hw/intc/arm_gicv3_its_common.o: In function `gicv3_its_init_mmio':
qemu/hw/intc/arm_gicv3_its_common.c:110: undefined reference to 
`msi_nonbroken'
collect2: error: ld returned 1 exit status
Makefile:201: recipe for target 'qemu-system-armeb' failed
make[1]: *** [qemu-system-armeb] Error 1
Makefile:327: recipe for target 'subdir-armeb-softmmu' failed
make: *** [subdir-armeb-softmmu] Error 2

This MCU does not use a GIC so I can also remove it, but this error 
seems odd to me.

As I understand the 'msi_nonbroken' is a kludge (there is no intrinsic 
relationship between PCI/MSI and this particular interrupt controller).
Since it is declared as extern in "hw/pci/msi.h" why not define it in 
pci-stub?

Regards,

Phil.

Thomas Huth May 9, 2017, 6:24 p.m. UTC | #3

Hi Philippe,

On 09.05.2017 18:49, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 05/09/2017 02:19 AM, Thomas Huth wrote:
>> On 09.05.2017 01:39, Philippe Mathieu-Daudé wrote:
>>> This field is accessed in hw/intc/arm_gicv[23*].c
>>
>> default-configs/arm-softmmu.mak sets CONFIG_PCI, so this should not be
>> necessary, I think. Otherwise, you should extend your patch description,
>> to elaborate on what you're trying to do here.
>>
>>  Thomas
> 
> Sure, I'm willing to add basic support for the TI Hercules MCU which is
> a big endian SoC (without PCI support).
> 
> I'm using the following config:
> 
> $ cat default-configs/armeb-softmmu.mak

$ cat default-configs/armeb-softmmu.mak
cat: default-configs/armeb-softmmu.mak: No such file or directory

... so you're planning for a new armeb config file here? That's the kind
of information that I was missing in the patch description.

[...]
> As I understand the 'msi_nonbroken' is a kludge (there is no intrinsic
> relationship between PCI/MSI and this particular interrupt controller).
> Since it is declared as extern in "hw/pci/msi.h" why not define it in
> pci-stub?

Sure, your patch is fine, IMHO it just lacks some rationale in its
description.

 Thomas

Patch

diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
index 36d2c430c5..ecad664946 100644
--- a/hw/pci/pci-stub.c
+++ b/hw/pci/pci-stub.c
@@ -24,6 +24,9 @@ 
 #include "qapi/qmp/qerror.h"
 #include "hw/pci/pci.h"
 #include "qmp-commands.h"
+#include "hw/pci/msi.h"
+
+bool msi_nonbroken;
 
 PciInfoList *qmp_query_pci(Error **errp)
 {