Hi Miao,
On Wed, Jan 20, 2016 at 9:58 AM, Miao Yan yanmiaobest@gmail.com wrote:
Hi Bin,
2016-01-19 17:25 GMT+08:00 Bin Meng bmeng.cn@gmail.com:
Hi Miao,
On Tue, Jan 19, 2016 at 10:46 AM, Miao Yan yanmiaobest@gmail.com wrote:
Hi Bin,
2016-01-16 21:23 GMT+08:00 Bin Meng bmeng.cn@gmail.com:
Hi Miao,
On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan yanmiaobest@gmail.com wrote:
Enable ACPI IO space for piix4 (for pc board) and ich9 (for q35 board)
Signed-off-by: Miao Yan yanmiaobest@gmail.com
arch/x86/cpu/qemu/qemu.c | 39 +++++++++++++++++++++++++++++++++ arch/x86/include/asm/arch-qemu/device.h | 8 +++++++ 2 files changed, 47 insertions(+)
diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c index 46111c9..e7d8a6c 100644 --- a/arch/x86/cpu/qemu/qemu.c +++ b/arch/x86/cpu/qemu/qemu.c @@ -15,6 +15,41 @@
static bool i440fx;
+static void enable_pm_piix(void) +{
u8 en;u16 device, cmd;device = x86_pci_read_config16(PIIX_PM, PCI_DEVICE_ID);if (device != PCI_DEVICE_ID_INTEL_82371AB_3)return;Guess the check is already covered in qemu_chipset_init().
Do you mean this check ?
device = x86_pci_read_config16(PCI_BDF(0, 0, 0), PCI_DEVICE_ID); i440fx = (device == PCI_DEVICE_ID_INTEL_82441);So is it guaranteed that PIIX_PM is always on that BDF ?
I believe so. If you look at the codes in qemu.c, the variable "static bool i440fx" is used to distinguish QEMU machine pc and q35. It does not check whether the chipset is i440fx, or PIIX which is the chipset connected to i440fx.
IMO, we are operating on another chipset, and we better make sure it's the one we expect, besides, an extra check won't do any harm.
Yes, that makes sense. So if we go with your way, maybe we need expand "static bool i440fx" to multiple variables and use correct variable to check? But this looks a bit complex than a single variable.
Yes, that's a little bit complex and not necessary if their PCI addresses are fixed . And I don't think we should do it in this patchset.
So how do you suggest we do this ? Either I remove the two checks to make it aligned with the rest or create a separate patch to do the checks ?
I suggest we do it in existing way (single variable).
[snip]
Regards, Bin