Re: [SeaBIOS] [PATCH 1/3] Add generic qemu detection
On Mon, Sep 16, 2013 at 12:36:28PM +0200, Gerd Hoffmann wrote: This patch adds support for detecting whenever SeaBIOS runs on qemu or not. This is done by looking at the northbridge (pci device 00:00.0) and check the subsystem id. Most pci devices emulated by qemu -- the two northbridges i440fx and q35 included -- have a subsystem id of 1af4:1100. In case the subsystem ID matches set PF_QEMU, log a message (including the northbridge found while being at it) and also check for kvm. What if other userspace will want to write enough support to run Seabios as a guest? Why kvm detection depends on QEMU? Signed-off-by: Gerd Hoffmann kra...@redhat.com --- src/fw/paravirt.c | 46 +- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c index 2524b7b..357de56 100644 --- a/src/fw/paravirt.c +++ b/src/fw/paravirt.c @@ -12,6 +12,7 @@ #include config.h // CONFIG_QEMU #include hw/cmos.h // CMOS_* #include hw/pci.h // create_pirtable +#include hw/pci_regs.h // PCI_DEVICE_ID #include ioport.h // outw #include malloc.h // malloc_tmp #include memmap.h // add_e820 @@ -35,10 +36,8 @@ int PlatformRunningOn VARFSEG; */ #define KVM_CPUID_SIGNATURE 0x4000 -static void kvm_preinit(void) +static void kvm_detect(void) { -if (!CONFIG_QEMU) -return; unsigned int eax, ebx, ecx, edx; char signature[13]; @@ -54,9 +53,43 @@ static void kvm_preinit(void) } } +static void qemu_detect(void) +{ +if (!CONFIG_QEMU_HARDWARE) +return; + +// check northbridge @ 00:00.0 +u16 v = pci_config_readw(0, PCI_VENDOR_ID); +if (v == 0x || v == 0x) +return; +u16 d = pci_config_readw(0, PCI_DEVICE_ID); +u16 sv = pci_config_readw(0, PCI_SUBSYSTEM_VENDOR_ID); +u16 sd = pci_config_readw(0, PCI_SUBSYSTEM_ID); + +if (sv != 0x1af4 || /* Red Hat, Inc */ +sd != 0x1100) /* Qemu virtual machine */ +return; + +PlatformRunningOn |= PF_QEMU; +switch (d) { +case 0x1237: +dprintf(1, Running on QEMU (i440fx)\n); +break; +case 0x29c0: +dprintf(1, Running on QEMU (q35)\n); +break; +default: +dprintf(1, Running on QEMU (unknown nb: %04x:%04x)\n, v, d); +break; +} +kvm_detect(); +} + void qemu_preinit(void) { +qemu_detect(); + if (!CONFIG_QEMU) return; @@ -65,8 +98,11 @@ qemu_preinit(void) return; } -PlatformRunningOn = PF_QEMU; -kvm_preinit(); +if (!runningOnQEMU()) { +dprintf(1, Warning: No QEMU Northbridge found (isapc?)\n); +PlatformRunningOn |= PF_QEMU; +kvm_detect(); +} // On emulators, get memory size from nvram. u32 rs = ((inb_cmos(CMOS_MEM_EXTMEM2_LOW) 16) -- 1.8.3.1 ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 1/3] Add generic qemu detection
On Mon, Sep 16, 2013 at 03:14:45PM +0200, Gerd Hoffmann wrote: On Mo, 2013-09-16 at 15:34 +0300, Gleb Natapov wrote: On Mon, Sep 16, 2013 at 02:27:37PM +0200, Gerd Hoffmann wrote: On Mo, 2013-09-16 at 14:10 +0300, Gleb Natapov wrote: On Mon, Sep 16, 2013 at 12:36:28PM +0200, Gerd Hoffmann wrote: This patch adds support for detecting whenever SeaBIOS runs on qemu or not. This is done by looking at the northbridge (pci device 00:00.0) and check the subsystem id. Most pci devices emulated by qemu -- the two northbridges i440fx and q35 included -- have a subsystem id of 1af4:1100. In case the subsystem ID matches set PF_QEMU, log a message (including the northbridge found while being at it) and also check for kvm. What if other userspace will want to write enough support to run Seabios as a guest? Why kvm detection depends on QEMU? What other userspace? There is none today. And if that changes some day we'll just call kvm_detect() in the init code for $otheruserspace. Why seabios code has to be changed at all to support other device model emulator (assuming it will emulate same chipset)? That is a purely theoretical thing. In practice seabios mist likely has to be changed for one or another reason. It's not have to be this way if Seabios would check for specific HW features instead of checking for QEMU. For instance if discovery of fw_cfg will not depend on QEMU detection other userspace can emulate and it will be detected without changing Seabios. kvm detection for CONFIG_QEMU builds only. Indeed the current logic is the same. Also note that in theory seabios should not need to care at all whenever it runs on kvm or tcg. In practice qemu has slightly different behavior with kvm enabled and seabios needs adapt to that. That alone is reason enougth that supporting other kvm userspace very likely needs adaptions in seabios, unless your theoretical[1] other userspace is bug compatible with qemu here. I see only one thing that is done based on KVM detection and the logic would be correct for other userspaces that do not provide e820 map. cheers, Gerd [1] Or do you know something you don't wanna tell me? No, all I know is that kvmtool wanted to add seabios support, but I guess nobody works on that these days. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 1/3] Add generic qemu detection
On Tue, Sep 03, 2013 at 02:37:41PM +0200, Gerd Hoffmann wrote: This patch adds support for detecting whenever SeaBIOS runs on qemu or not. This is done by looking at the northbridge (pci device 00:00.0) and check the subsystem id. Most pci devices emulated by qemu -- the two northbridges i440fx and q35 included -- have a subsystem id of 1af4:1100. In case the subsystem ID matches set PF_QEMU, log a message (including the northbridge found while being at it) and also check for kvm. Why condition KVM check on QEMU? Signed-off-by: Gerd Hoffmann kra...@redhat.com --- src/paravirt.c | 42 -- src/paravirt.h | 1 + 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/paravirt.c b/src/paravirt.c index d1a5d3e..ea9fa9a 100644 --- a/src/paravirt.c +++ b/src/paravirt.c @@ -19,6 +19,7 @@ #include acpi.h // acpi_setup #include mptable.h // mptable_setup #include pci.h // create_pirtable +#include pci_regs.h // create_pirtable #include xen.h // xen_biostable_setup // Amount of continuous ram under 4Gig @@ -33,10 +34,8 @@ int PlatformRunningOn VARFSEG; */ #define KVM_CPUID_SIGNATURE 0x4000 -static void kvm_preinit(void) +static void kvm_detect(void) { -if (!CONFIG_QEMU) -return; unsigned int eax, ebx, ecx, edx; char signature[13]; @@ -52,9 +51,43 @@ static void kvm_preinit(void) } } +void qemu_detect(void) +{ +if (!CONFIG_QEMU_HARDWARE) +return; + +// check northbridge @ 00:00.0 +u16 v = pci_config_readw(0, PCI_VENDOR_ID); +if (v == 0x || v == 0x) +return; +u16 d = pci_config_readw(0, PCI_DEVICE_ID); +u16 sv = pci_config_readw(0, PCI_SUBSYSTEM_VENDOR_ID); +u16 sd = pci_config_readw(0, PCI_SUBSYSTEM_ID); + +if (sv != 0x1af4 || /* Red Hat, Inc */ +sd != 0x1100) /* Qemu virtual machine */ +return; + +PlatformRunningOn |= PF_QEMU; +switch (d) { +case 0x1237: +dprintf(1, Running on QEMU (i440fx)\n); +break; +case 0x29c0: +dprintf(1, Running on QEMU (q35)\n); +break; +default: +dprintf(1, Running on QEMU (unknown nb: %04x:%04x)\n, v, d); +break; +} +kvm_detect(); +} + void qemu_preinit(void) { +qemu_detect(); + if (!CONFIG_QEMU) return; @@ -63,9 +96,6 @@ qemu_preinit(void) return; } -PlatformRunningOn = PF_QEMU; -kvm_preinit(); - // On emulators, get memory size from nvram. u32 rs = ((inb_cmos(CMOS_MEM_EXTMEM2_LOW) 16) | (inb_cmos(CMOS_MEM_EXTMEM2_HIGH) 24)); diff --git a/src/paravirt.h b/src/paravirt.h index fce5af9..7fc00b0 100644 --- a/src/paravirt.h +++ b/src/paravirt.h @@ -24,6 +24,7 @@ static inline int runningOnKVM(void) { return CONFIG_QEMU GET_GLOBAL(PlatformRunningOn) PF_KVM; } +void qemu_detect(void); void qemu_preinit(void); void qemu_platform_setup(void); void qemu_cfg_init(void); -- 1.8.3.1 ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Mon, Aug 05, 2013 at 09:32:18PM +0300, Michael S. Tsirkin wrote: As you see we do let people change many parameters that do affect activation. By editing XML user can shoot himself in the foot, we should not prevent that. So that's what I'm saying basically. At the moment there's no way to remove this device from XML. That's just wrong. Can say the same about PV acpi hotpulg device. In QEMU, we have a standard way to specify devices with -device. That should be the interface for anything new really unless there's a very compelling reason for something else. We are disagree on compelling reason in this case obviously. *Not* building it into the PC machine type. It should not be required though. libvirt can pass -device pvpanic by default if nothing is specified in XML. That discussion really has to happen on libvirt list. As Paolo said you are just pushing the problem up the stack where it is harder to solve. I put problem and solve in quotes because I disagree that the problem that need to be solved is identified correctly. The correct problem to be solved IMO is writing Windows driver for the device. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 04:03:17AM -0400, Vadim Rozenfeld wrote: - Original Message - From: Gleb Natapov g...@redhat.com To: Michael S. Tsirkin m...@redhat.com Cc: Daniel P. Berrange berra...@redhat.com, Marcel Apfelbaum marce...@redhat.com, seabios@seabios.org, qemu-de...@nongnu.org, Gerd Hoffmann kra...@redhat.com, Paolo Bonzini pbonz...@redhat.com, Eric Blake ebl...@redhat.com, Andreas Färber afaer...@suse.de Sent: Tuesday, August 6, 2013 5:34:06 PM Subject: Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI On Mon, Aug 05, 2013 at 09:32:18PM +0300, Michael S. Tsirkin wrote: As you see we do let people change many parameters that do affect activation. By editing XML user can shoot himself in the foot, we should not prevent that. So that's what I'm saying basically. At the moment there's no way to remove this device from XML. That's just wrong. Can say the same about PV acpi hotpulg device. In QEMU, we have a standard way to specify devices with -device. That should be the interface for anything new really unless there's a very compelling reason for something else. We are disagree on compelling reason in this case obviously. *Not* building it into the PC machine type. It should not be required though. libvirt can pass -device pvpanic by default if nothing is specified in XML. That discussion really has to happen on libvirt list. As Paolo said you are just pushing the problem up the stack where it is harder to solve. I put problem and solve in quotes because I disagree that the problem that need to be solved is identified correctly. The correct problem to be solved IMO is writing Windows driver for the device. [VR] This one shouldn't be too complicated. Can be done on weekend. Gal says he did it already. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 11:33:10AM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 10:21:52AM +0300, Gleb Natapov wrote: If you see a mouse in a room, how likely is it that there's a single mouse there? This is a PV technology which to me looks like it was rushed through and not only set on by default, but without a way to disable it - apparently on the assumption there's 0 chance it can cause any damage. Now that we do know the chance it's not there, why not go back to the standard interface, and why not give users a chance to enable/disable it? You should be able to disable it with: -device pvpanic,ioport=0 Doesn't work for me. Bug that should be fixed. With this command line _STA should return zero. Besides, both -device pvpanic and use of ioport=0 to disable it are completely undocumented. Not the only undocumented thing in QEMU command line :) BTW pls keep qemu-devel Cc'd. Haven't touched CC list. We have different definition of damage though. Driver bugs, qemu bugs, OSPM bugs all can cause issues like OS crashes, suspend/resume issues, bad performance ... What's your definition of damage? None of those cover the case at hand. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 04:14:36AM -0400, Vadim Rozenfeld wrote: - Original Message - From: Gleb Natapov g...@redhat.com To: Vadim Rozenfeld vroze...@redhat.com Cc: Michael S. Tsirkin m...@redhat.com, Daniel P. Berrange berra...@redhat.com, Marcel Apfelbaum marce...@redhat.com, seabios@seabios.org, qemu-de...@nongnu.org, Gerd Hoffmann kra...@redhat.com, Paolo Bonzini pbonz...@redhat.com, Eric Blake ebl...@redhat.com, Andreas Färber afaer...@suse.de Sent: Tuesday, August 6, 2013 6:05:27 PM Subject: Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI On Tue, Aug 06, 2013 at 04:03:17AM -0400, Vadim Rozenfeld wrote: - Original Message - From: Gleb Natapov g...@redhat.com To: Michael S. Tsirkin m...@redhat.com Cc: Daniel P. Berrange berra...@redhat.com, Marcel Apfelbaum marce...@redhat.com, seabios@seabios.org, qemu-de...@nongnu.org, Gerd Hoffmann kra...@redhat.com, Paolo Bonzini pbonz...@redhat.com, Eric Blake ebl...@redhat.com, Andreas Färber afaer...@suse.de Sent: Tuesday, August 6, 2013 5:34:06 PM Subject: Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI On Mon, Aug 05, 2013 at 09:32:18PM +0300, Michael S. Tsirkin wrote: As you see we do let people change many parameters that do affect activation. By editing XML user can shoot himself in the foot, we should not prevent that. So that's what I'm saying basically. At the moment there's no way to remove this device from XML. That's just wrong. Can say the same about PV acpi hotpulg device. In QEMU, we have a standard way to specify devices with -device. That should be the interface for anything new really unless there's a very compelling reason for something else. We are disagree on compelling reason in this case obviously. *Not* building it into the PC machine type. It should not be required though. libvirt can pass -device pvpanic by default if nothing is specified in XML. That discussion really has to happen on libvirt list. As Paolo said you are just pushing the problem up the stack where it is harder to solve. I put problem and solve in quotes because I disagree that the problem that need to be solved is identified correctly. The correct problem to be solved IMO is writing Windows driver for the device. [VR] This one shouldn't be too complicated. Can be done on weekend. Gal says he did it already. [VR] If so, we can add it to our build and make it public. That's the plan :) -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 10:45:01AM +0200, Andreas Färber wrote: Am 06.08.2013 10:36, schrieb Gleb Natapov: On Tue, Aug 06, 2013 at 11:33:10AM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 10:21:52AM +0300, Gleb Natapov wrote: If you see a mouse in a room, how likely is it that there's a single mouse there? This is a PV technology which to me looks like it was rushed through and not only set on by default, but without a way to disable it - apparently on the assumption there's 0 chance it can cause any damage. Now that we do know the chance it's not there, why not go back to the standard interface, and why not give users a chance to enable/disable it? You should be able to disable it with: -device pvpanic,ioport=0 Doesn't work for me. Bug that should be fixed. With this command line _STA should return zero. Besides, both -device pvpanic and use of ioport=0 to disable it are completely undocumented. Not the only undocumented thing in QEMU command line :) [snip] I disagree: -device adds a device, not removes one. It will still be present. I assume you are answering to the quote about ioport=0, not documentation here. ioport=0 does not removes the device, it disables it. The claim was it cannot be disabled, it can (assuming no bugs), but it should not be. I am neutral as to whether qemu-system-x86_64 should have it enabled by default or not. But if we want to suppress it, then -nodefaults should disable it. Since libvirt uses that though, it would mean libvirt would need to add it back, whether via user's XML domain config or by libvirt itself based on some version/etc. heuristics. There is no clear definition of what -nodefaults should or should not do. Should it disable PV ACPI hotplug device? Should it create a machine with no mmio/io regions registered at all? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 05:26:46PM +0800, Hu Tao wrote: On Tue, Aug 06, 2013 at 11:33:10AM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 10:21:52AM +0300, Gleb Natapov wrote: If you see a mouse in a room, how likely is it that there's a single mouse there? This is a PV technology which to me looks like it was rushed through and not only set on by default, but without a way to disable it - apparently on the assumption there's 0 chance it can cause any damage. Now that we do know the chance it's not there, why not go back to the standard interface, and why not give users a chance to enable/disable it? You should be able to disable it with: -device pvpanic,ioport=0 Doesn't work for me. The internal pvpanic can be disabled by -global pvpanic.ioport=0. -device pvpanic,ioport=0 just adds another pvpanic device. Yeah, good point. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 12:21:48PM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 11:36:25AM +0300, Gleb Natapov wrote: On Tue, Aug 06, 2013 at 11:33:10AM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 10:21:52AM +0300, Gleb Natapov wrote: If you see a mouse in a room, how likely is it that there's a single mouse there? This is a PV technology which to me looks like it was rushed through and not only set on by default, but without a way to disable it - apparently on the assumption there's 0 chance it can cause any damage. Now that we do know the chance it's not there, why not go back to the standard interface, and why not give users a chance to enable/disable it? You should be able to disable it with: -device pvpanic,ioport=0 Doesn't work for me. Bug that should be fixed. With this command line _STA should return zero. It doesn't have anything to do with _STA: device still appears in QOM. You said disabled, not removed. So does -global pvpanic,ioport=0 disables the device for you? It's a QEMU issue, devices that are added with -device are documented in -device help and removed by dropping them from command line. Devices added by default have no way to be dropped from QOM except -nodefaults. Are you saying that because pvpanic is added automatically QEMU -device help does not print help about it? Why not fix that? What QEMU --help issues has to do with deciding which devices should or should not be present by default? Besides, both -device pvpanic and use of ioport=0 to disable it are completely undocumented. Not the only undocumented thing in QEMU command line :) All -device fields are documented with -device help. This was supposed to be the right way to add all new devices. BTW pls keep qemu-devel Cc'd. Haven't touched CC list. We have different definition of damage though. Driver bugs, qemu bugs, OSPM bugs all can cause issues like OS crashes, suspend/resume issues, bad performance ... What's your definition of damage? None of those cover the case at hand. Sigh. These examples demonstrate why would user want to run QEMU without the pvpanic device. He can disable it, but chances the device will cause aforementioned problems are so much smaller (by design mind you) than PV APIC hotplug device that it makes me wonder why haven't you advocate to make PV APIC hotplug device to be configurable with -device too. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 01:13:17PM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 12:29:27PM +0300, Gleb Natapov wrote: On Tue, Aug 06, 2013 at 05:26:46PM +0800, Hu Tao wrote: On Tue, Aug 06, 2013 at 11:33:10AM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 10:21:52AM +0300, Gleb Natapov wrote: If you see a mouse in a room, how likely is it that there's a single mouse there? This is a PV technology which to me looks like it was rushed through and not only set on by default, but without a way to disable it - apparently on the assumption there's 0 chance it can cause any damage. Now that we do know the chance it's not there, why not go back to the standard interface, and why not give users a chance to enable/disable it? You should be able to disable it with: -device pvpanic,ioport=0 Doesn't work for me. The internal pvpanic can be disabled by -global pvpanic.ioport=0. -device pvpanic,ioport=0 just adds another pvpanic device. Yeah, good point. I tried this, this doesn't remove the device - merely sets the port to 0. That's the point. And _STA will report it as disabled. You asked if the device can be enable/disable above, not removed. It can. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 01:23:26PM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 01:14:14PM +0300, Gleb Natapov wrote: On Tue, Aug 06, 2013 at 01:13:17PM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 12:29:27PM +0300, Gleb Natapov wrote: On Tue, Aug 06, 2013 at 05:26:46PM +0800, Hu Tao wrote: On Tue, Aug 06, 2013 at 11:33:10AM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 10:21:52AM +0300, Gleb Natapov wrote: If you see a mouse in a room, how likely is it that there's a single mouse there? This is a PV technology which to me looks like it was rushed through and not only set on by default, but without a way to disable it - apparently on the assumption there's 0 chance it can cause any damage. Now that we do know the chance it's not there, why not go back to the standard interface, and why not give users a chance to enable/disable it? You should be able to disable it with: -device pvpanic,ioport=0 Doesn't work for me. The internal pvpanic can be disabled by -global pvpanic.ioport=0. -device pvpanic,ioport=0 just adds another pvpanic device. Yeah, good point. I tried this, this doesn't remove the device - merely sets the port to 0. That's the point. And _STA will report it as disabled. In ACPI? Why have it at all? You can put whatever you want into ACPI as long as _STA returns disabled status. What's the problem? You asked if the device can be enable/disable above, not removed. It can. I really meant QOM/QMP. _STA is only seen by OSPM. Then you should have said removed. No you cannot, this is not the only such device. I do not see why would you want to though. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 01:03:34PM +0200, Andreas Färber wrote: Am 06.08.2013 12:44, schrieb Gleb Natapov: On Tue, Aug 06, 2013 at 01:19:53PM +0300, Michael S. Tsirkin wrote: It's a QEMU issue, devices that are added with -device are documented in -device help and removed by dropping them from command line. Devices added by default have no way to be dropped from QOM except -nodefaults. Are you saying that because pvpanic is added automatically QEMU -device help does not print help about it? Why not fix that? What QEMU --help issues has to do with deciding which devices should or should not be present by default? No, I'm saying what I said: that there's no way to remove a device added by default except -nodefaults, and no way to find out what does -nodefaults exclude so you can add things you need back selectively. And what are the rules that govern device exclusion from -nodefaults list? Why -nodefaults does not create empty machine? We have -M none to create an empty machine. FWIW -M q35 does not create all Q35 devices, there's -readconfig docs/q35-chipset.cfg for the rest. The criteria certainly is not migratability, since ICH9 AHCI (part of -M q35) is unmigratable, unfortunately. One practical reason not to create everything via config is that we cannot create SysBusDevices via -device when they require MMIO mapping or IRQ setup. For ISADevices such as pvpanic that's not a problem. Anthony has proposed QOM'ifying MemoryRegions and qemu_irq as solution to do the wiring-up from command line or config file, but those attempts got stuck a long time ago. But -M creates not only things that cannot be created from a command line, it includes some default set of devices, so what is the criteria for those? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 01:23:49PM +0200, Andreas Färber wrote: Am 06.08.2013 13:00, schrieb Gleb Natapov: On Tue, Aug 06, 2013 at 12:35:10PM +0200, Andreas Färber wrote: I wonder if IPMI might be such an alternative in the future, in which case we should come up with some way to fully disable pvpanic device creation. CC'ing Corey. IPMI was considered, to complicated for what was needed. Sorry? There's nothing wrong with going for pvpanic as a simple implementation. Sure, why sorry then? :) PV has its benefits. There have been IPMI patchsets on qemu-devel though, and SUSE will be investigating adding some IPMI support too (not sure if identical to the scope of those patchsets), whether IPMI is complicated or not. It's a standard present on physical servers, facilitating unified management of virtual and physical servers, and there's OpenIPMI as implementation. Of course, there is nothing wrong with implementing IPMI either. Many problems that IPMI solves are much simpler to solve in virtualized environment with management software and pvpanic closes one gap between what IPMI provides and virtual machine management can do. My point was, there may be alternative, non-PV implementations to suck such information out of a guest, IPMI being one example of a management interface that exists for physical servers. So it's not necessarily black-or-white, but choices similar to virtio vs. IDE vs. AHCI vs. SCSI. pvpanic not meant to replace IPMI though. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 01:48:17PM +0200, Markus Armbruster wrote: Gleb Natapov g...@redhat.com writes: On Tue, Aug 06, 2013 at 01:03:34PM +0200, Andreas Färber wrote: Am 06.08.2013 12:44, schrieb Gleb Natapov: On Tue, Aug 06, 2013 at 01:19:53PM +0300, Michael S. Tsirkin wrote: It's a QEMU issue, devices that are added with -device are documented in -device help and removed by dropping them from command line. Devices added by default have no way to be dropped from QOM except -nodefaults. Are you saying that because pvpanic is added automatically QEMU -device help does not print help about it? Why not fix that? What QEMU --help issues has to do with deciding which devices should or should not be present by default? No, I'm saying what I said: that there's no way to remove a device added by default except -nodefaults, and no way to find out what does -nodefaults exclude so you can add things you need back selectively. And what are the rules that govern device exclusion from -nodefaults list? Why -nodefaults does not create empty machine? We have -M none to create an empty machine. FWIW -M q35 does not create all Q35 devices, there's -readconfig docs/q35-chipset.cfg for the rest. The criteria certainly is not migratability, since ICH9 AHCI (part of -M q35) is unmigratable, unfortunately. One practical reason not to create everything via config is that we cannot create SysBusDevices via -device when they require MMIO mapping or IRQ setup. Support wiring up a machine without board code, just configuration has been the ever-distant goal of the qdev effort. For ISADevices such as pvpanic that's not a problem. Anthony has proposed QOM'ifying MemoryRegions and qemu_irq as solution to do the wiring-up from command line or config file, but those attempts got stuck a long time ago. But -M creates not only things that cannot be created from a command line, it includes some default set of devices, so what is the criteria for those? I'm not aware of defined, coherent criteria. I can give you descriptive rather than prescriptive, though. Used to be whatever anyone felt users would want. It's now whatever has always been there, plus whatever survives interminable bikeshedding^W^Wvigorous debate. No need to for ^W! It is like disputes in court: whoever has more money wins. In out case whoever has more times wins and I am going on vocation, so... :) -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 01:54:17PM +0200, Gerd Hoffmann wrote: Hi, And what are the rules that govern device exclusion from -nodefaults list? Why -nodefaults does not create empty machine? qemu -nodefaults should give you just cpu + northbridge + southbridge. On modern machine this is everything that most will ever have. qemu should give you a usable virtual machine, so qemu adds some optional devices which are (or used to be) standard in a pc: floppy, cdrom, serial port, parallel port, vga card, nic. Sometimes things are a bit odd for historical reasons: USB controllers are only present in case you ask for them via '-usb' or add them via '-device', even though they are part of the southbridge. vmport is there unconditionally. And I'd rather make that one configurable instead of adding more hard-coded devices. There are more, as I already said hotplug device (I am to lazy to search for more, so I always bring this one). -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 03:05:52PM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 03:00:06PM +0300, Gleb Natapov wrote: On Tue, Aug 06, 2013 at 01:23:49PM +0200, Andreas Färber wrote: Am 06.08.2013 13:00, schrieb Gleb Natapov: On Tue, Aug 06, 2013 at 12:35:10PM +0200, Andreas Färber wrote: I wonder if IPMI might be such an alternative in the future, in which case we should come up with some way to fully disable pvpanic device creation. CC'ing Corey. IPMI was considered, to complicated for what was needed. Sorry? There's nothing wrong with going for pvpanic as a simple implementation. Sure, why sorry then? :) PV has its benefits. PV always seems easier. It sometimes becomes a maintainance problem down the way though. If it is modelled as proper device, why? What PV devices we have that are pain? There have been IPMI patchsets on qemu-devel though, and SUSE will be investigating adding some IPMI support too (not sure if identical to the scope of those patchsets), whether IPMI is complicated or not. It's a standard present on physical servers, facilitating unified management of virtual and physical servers, and there's OpenIPMI as implementation. Of course, there is nothing wrong with implementing IPMI either. Many problems that IPMI solves are much simpler to solve in virtualized environment with management software and pvpanic closes one gap between what IPMI provides and virtual machine management can do. My point was, there may be alternative, non-PV implementations to suck such information out of a guest, IPMI being one example of a management interface that exists for physical servers. So it's not necessarily black-or-white, but choices similar to virtio vs. IDE vs. AHCI vs. SCSI. pvpanic not meant to replace IPMI though. But will you want pvpanic if you have IPMI? Why not? They report to different components. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Tue, Aug 06, 2013 at 03:08:32PM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 02:00:35PM +0300, Gleb Natapov wrote: On Tue, Aug 06, 2013 at 12:35:10PM +0200, Andreas Färber wrote: Am 06.08.2013 11:32, schrieb Gleb Natapov: On Tue, Aug 06, 2013 at 12:21:48PM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 11:36:25AM +0300, Gleb Natapov wrote: On Tue, Aug 06, 2013 at 11:33:10AM +0300, Michael S. Tsirkin wrote: On Tue, Aug 06, 2013 at 10:21:52AM +0300, Gleb Natapov wrote: This is a PV technology which to me looks like it was rushed through and not only set on by default, but without a way to disable it - apparently on the assumption there's 0 chance it can cause any damage. Now that we do know the chance it's not there, why not go back to the standard interface, and why not give users a chance to enable/disable it? You should be able to disable it with: -device pvpanic,ioport=0 Doesn't work for me. Bug that should be fixed. With this command line _STA should return zero. It doesn't have anything to do with _STA: device still appears in QOM. You said disabled, not removed. So does -global pvpanic,ioport=0 disables the device for you? It's a QEMU issue, devices that are added with -device are documented in -device help and removed by dropping them from command line. Devices added by default have no way to be dropped from QOM except -nodefaults. Are you saying that because pvpanic is added automatically QEMU -device help does not print help about it? Why not fix that? What QEMU --help issues has to do with deciding which devices should or should not be present by default? You misunderstand: -device pvpanic,? will document that there is a numeric port property, which as such is self-documenting. But there's no Yes, this is how I found it. way for us to document there that port=0 has special meaning of disable this device in ACPI. Adding capability to describe a property should solve that and is a good idea regardless, no? pvpanic.ioport=uint16 is not very descriptive. Disabling a device usually requires to not include that device (or in the future to unrealize it), which would require some way to suppress having the device created internally by default. As done for floppy, serial, etc. devices in x86 IIUC, which are in the same PIO situation as the pvpanic device, except that they represent physical devices. Adding some -no-pvpanic switch might be an alternative. And if not done already, disabling the pvpanic device should definitely be documented for the man page. We should not add -no-pvpanic! If there is a legitimate use for -no-pvpanic we should go with MST suggestion and do not create it by default. The question is why would anyone use -no-pvpanic? Legit reason, not just to remove pvpanic. To be able to emulate a real hardware system without any PV devices. I think it's a reasonable requirement. Qemu is so far from able to do so that I do not consider it such. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Mon, Aug 05, 2013 at 11:10:55AM +0300, Michael S. Tsirkin wrote: On Mon, Aug 05, 2013 at 03:47:23PM +0800, Hu Tao wrote: pvpanic device is an internal default device in qemu. It may cause problem when upgrading qemu from a version without pvpanic. for example: in Windows(let's say XP) the Device manager will open a new device wizard and the device will appear as an unrecognized device. On a cluster with hundreds of such VMs, If that cluster has a health monitoring service it may show all the VMs in a not healthy state. This patch is a workaround to not show pvpanic in UI to avoid the problem in Windows. Cc: Marcel Apfelbaum marce...@redhat.com Cc: Michael S. Tsirkin m...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Gerd Hoffmann kra...@redhat.com Cc: Eric Blake ebl...@redhat.com Cc: Daniel P. Berrange berra...@redhat.com Cc: Andreas Färber afaer...@suse.de Signed-off-by: Hu Tao hu...@cn.fujitsu.com Quoting from this discussion: That may fix the issue of a windows guest showing the yellow ! mark, but what if, down the road, someone writes an actual windows driver that is aware of that port and how to make a windows BSOD write a panic notification to the port? How does a user go about installing such a driver if the device is not exposed in the user interface list of devices? I think the correct way to address this is: - don't create the device by default, only when -device pvpanic is present - teach management to supply said -device pvpanic for guests which support the pvpanic device That's just pushing the problem elsewhere. How management suppose to know if guest support pvpanic device? What if initially guest did not have a driver, but the it was installed? If device appears for old machine models this is bug, otherwise I fail to see any problem. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Mon, Aug 05, 2013 at 11:03:53AM +0200, Gerd Hoffmann wrote: On 08/05/13 10:16, Gleb Natapov wrote: On Mon, Aug 05, 2013 at 11:10:55AM +0300, Michael S. Tsirkin wrote: On Mon, Aug 05, 2013 at 03:47:23PM +0800, Hu Tao wrote: pvpanic device is an internal default device in qemu. It may cause problem when upgrading qemu from a version without pvpanic. for example: in Windows(let's say XP) the Device manager will open a new device wizard and the device will appear as an unrecognized device. On a cluster with hundreds of such VMs, If that cluster has a health monitoring service it may show all the VMs in a not healthy state. This patch is a workaround to not show pvpanic in UI to avoid the problem in Windows. Cc: Marcel Apfelbaum marce...@redhat.com Cc: Michael S. Tsirkin m...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Gerd Hoffmann kra...@redhat.com Cc: Eric Blake ebl...@redhat.com Cc: Daniel P. Berrange berra...@redhat.com Cc: Andreas Färber afaer...@suse.de Signed-off-by: Hu Tao hu...@cn.fujitsu.com Quoting from this discussion: That may fix the issue of a windows guest showing the yellow ! mark, but what if, down the road, someone writes an actual windows driver that is aware of that port and how to make a windows BSOD write a panic notification to the port? How does a user go about installing such a driver if the device is not exposed in the user interface list of devices? I think the correct way to address this is: - don't create the device by default, only when -device pvpanic is present - teach management to supply said -device pvpanic for guests which support the pvpanic device That's just pushing the problem elsewhere. How management suppose to know if guest support pvpanic device? The problem isn't new and management already does that when figuring whenever the guest should get ide/ahci/virtio-blk/virtio-scsi storage, ac97 or intel-hda sound, rtl8139/e1000/virtio nic, ... All of those are multiple choses and some of then expect you to install drivers actually since they also do not work out of the box with all guests. What if initially guest did not have a driver, but the it was installed? Update the machine config then? What is so special about the pvpanic You cannot change HW after machine is installed without risking Windows reactivation. device that it should be treated differently from everything else? It is simple, it does not have alternative, there is not reason to not have it. So your options are: having the device and installing driver if functionality is needed, or never have it. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] don't expose pvpanic device in the UI
On Mon, Aug 05, 2013 at 06:03:34PM +0300, Michael S. Tsirkin wrote: On Mon, Aug 05, 2013 at 12:20:44PM +0300, Gleb Natapov wrote: On Mon, Aug 05, 2013 at 12:18:26PM +0300, Michael S. Tsirkin wrote: On Mon, Aug 05, 2013 at 11:16:17AM +0300, Gleb Natapov wrote: On Mon, Aug 05, 2013 at 11:10:55AM +0300, Michael S. Tsirkin wrote: On Mon, Aug 05, 2013 at 03:47:23PM +0800, Hu Tao wrote: pvpanic device is an internal default device in qemu. It may cause problem when upgrading qemu from a version without pvpanic. for example: in Windows(let's say XP) the Device manager will open a new device wizard and the device will appear as an unrecognized device. On a cluster with hundreds of such VMs, If that cluster has a health monitoring service it may show all the VMs in a not healthy state. This patch is a workaround to not show pvpanic in UI to avoid the problem in Windows. Cc: Marcel Apfelbaum marce...@redhat.com Cc: Michael S. Tsirkin m...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Gerd Hoffmann kra...@redhat.com Cc: Eric Blake ebl...@redhat.com Cc: Daniel P. Berrange berra...@redhat.com Cc: Andreas Färber afaer...@suse.de Signed-off-by: Hu Tao hu...@cn.fujitsu.com Quoting from this discussion: That may fix the issue of a windows guest showing the yellow ! mark, but what if, down the road, someone writes an actual windows driver that is aware of that port and how to make a windows BSOD write a panic notification to the port? How does a user go about installing such a driver if the device is not exposed in the user interface list of devices? I think the correct way to address this is: - don't create the device by default, only when -device pvpanic is present - teach management to supply said -device pvpanic for guests which support the pvpanic device That's just pushing the problem elsewhere. How management suppose to know if guest support pvpanic device? Same as any PV device really. It's exactly the same problem as with virtio: user configures the XML properly. Virtio has alternatives. I don't see why does it matter. In any case, only *some* virtio devices have alternatives. What about the balloon device? VIRTIO_9P? There are more examples. What about e.g. ivshmem? They take very limited pci resources and/or provide functionality that should not be available for all guests. We do provide ACPI hotplug device unconditionally. What if initially guest did not have a driver, but the it was installed? You can reconfigure XML and reboot. Will it cause Windows reactivation? Maybe after adding several devices? I don't think it will. https://en.wikipedia.org/wiki/Microsoft_Product_Activation says: Display adapter SCSI adapter IDE adapter Network adapter MAC address RAM amount range (e.g. 0-512 MB) Processor type and serial number Hard drive device and volume serial number Optical drive (e.g. DVD-ROM) As you see we do let people change many parameters that do affect activation. By editing XML user can shoot himself in the foot, we should not prevent that. It should not be required though. If device appears for old machine models this is bug, otherwise I fail to see any problem. Care answering the question that Eric Blake posed (above)? Which one? About how to install driver if device is not shown in the gui? I suppose clicking on device driver installer should do the job. Did you try? I do not see any reasonable doubt to even question the possibility :) But see Paolo's reply. I think this requires an EXE installer. Being prompted for driver makes it possible to install one from INF. AFAIK INF is clickable file, but regardless I do not think even _this_ patch is needed. What is needed is Windows driver for the device. How about changing drivers? Selecting one driver from multiple variants? How do you disable it if it's causing trouble, or for testing? Again, as Paolo says you can see it if you really wish. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [SeaBIOS PATCH] boot: fix fw_dev_path pattern for q35-pcihost
On Wed, May 29, 2013 at 10:33:54AM +0800, Amos Kong wrote: On Tue, May 28, 2013 at 06:59:02PM -0400, Kevin O'Connor wrote: On Tue, May 28, 2013 at 08:28:14PM +0800, Amos Kong wrote: Bootindex string passed from qemu: /q35-pcihost@i0cf8/ethernet@2/ethernet-phy@0 We match pci domain by /pci@i0cf8 in SeaBIOS, but fw_dev_path prefix of q35 is /q35-pcihost@i0cf8. So bootindex in qemu commandline doesn't work if it uses q35 machine type. This patch fixes the pattern to match both original pc-i440fx q35 Signed-off-by: Amos Kong ak...@redhat.com --- src/boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot.c b/src/boot.c index cd9d784..f30d47e 100644 --- a/src/boot.c +++ b/src/boot.c @@ -97,7 +97,7 @@ find_prio(const char *glob) return -1; } -#define FW_PCI_DOMAIN /pci@i0cf8 +#define FW_PCI_DOMAIN /*pci*@i0cf8 The seabios pattern matching code isn't that sophisticated - I think this could end up doing something unexpected. Why does it need to change? If we start a guest with default machine type (pc-i440fx), the prefix of bootindex string is /pci@i0cf8, if we start guest with -M q35, the prefix will become /q35-pcihost@i0cf8. We only match /pci@i0cf8 in seabios, it causes boot priority of q35 devices could not be changed. We could not change TYPE_Q35_HOST_DEVICE to 'pci' in qemu to adapt seabios, so fix the pattern. Why couldn't we? QEMU not been able to generate proper device string is QEMU bug. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] KVM call agenda for 2013-05-28
On Wed, May 29, 2013 at 11:45:44AM +0300, Michael S. Tsirkin wrote: On Tue, May 28, 2013 at 07:53:09PM -0400, Kevin O'Connor wrote: On Thu, May 23, 2013 at 03:41:32PM +0300, Michael S. Tsirkin wrote: Juan is not available now, and Anthony asked for agenda to be sent early. So here comes: Agenda for the meeting Tue, May 28: - Generating acpi tables I didn't see any meeting notes, but I thought it would be worthwhile to summarize the call. This is from memory so correct me if I got anything wrong. Anthony believes that the generation of ACPI tables is the task of the firmware. Reasons cited include security implications of running more code in qemu vs the guest context, complexities in running iasl on big-endian machines, possible complexity of having to regenerate tables on a vm reboot, overall sloppiness of doing it in QEMU. Raised that QOM interface should be sufficient. Kevin believes that the bios table code should be moved up into QEMU. Reasons cited include the churn rate in SeaBIOS for this QEMU feature (15-20% of all SeaBIOS commits since integrating with QEMU have been for bios tables; 20% of SeaBIOS commits in last year), complexity of trying to pass all the content needed to generate the tables (eg, device details, power tree, irq routing), complexity of scheduling changes across different repos and synchronizing their rollout, complexity of implemeting the code in both OVMF and SeaBIOS. Kevin wasn't aware of a requirement to regenerate acpi tables on a vm reboot. I think this last one is based on a misunderstanding: it's based on assumption that we we change hardware by hotplug we should regenerate the tables to match. But there's no management that can take advantage of this. Two possible reasonable things we can tell management: - hotplug for device XXX is not supported: restart qemu to make guest use the device - hotplug for device XXX is supported What is proposed here instead is a third option: - hotplug is supported but device is not functional. reboot guest to make it fully functional This will naturally lead to requirement to regenerate tables on reset. And this is what would happen with guest-generated tables, and I consider this a bug, not a feature. +1. This will probably break guest resume too. If you really wanted to update tables dynamically, without restarting qemu, don't stop there, add an interface for guest to update them without reset. I think that's over-endineering and a requirement that's best avoided. There were discussions on potentially introducing a middle component to generate the tables. Coreboot was raised as a possibility, and David thought it would be okay to use coreboot for both OVMF and SeaBIOS. The possibility was also raised of a rom that lives in the qemu repo, is run in the guest, and generates the tables (which is similar to the hvmloader approach that Xen uses). Anthony requested that patches be made that generate the ACPI tables in QEMU for the upcoming hotplug work, so that they could be evaluated to see if they truly do need to live in QEMU or if the code could live in the firmware. There were no objections. -Kevin I volunteered to implement this. Why hotplug should generate ACPI code? It does not do so on real HW. It was also mentioned that this patch does not yet have to fix the cross-version migration issue with fw_cfg. If we agree on a direction, we will fix it then. Lastly, a proposal was made by Michael to make the call bi-weekly instead of weekly, as we were cancelling it too much. There were no objections. Thus, the next call is planned for June 11, 2013. -- MST ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [SeaBIOS PATCH] boot: fix fw_dev_path pattern for q35-pcihost
On Sun, Jun 02, 2013 at 05:59:04PM +0300, Michael S. Tsirkin wrote: On Sun, Jun 02, 2013 at 05:29:45PM +0300, Gleb Natapov wrote: On Wed, May 29, 2013 at 10:33:54AM +0800, Amos Kong wrote: On Tue, May 28, 2013 at 06:59:02PM -0400, Kevin O'Connor wrote: On Tue, May 28, 2013 at 08:28:14PM +0800, Amos Kong wrote: Bootindex string passed from qemu: /q35-pcihost@i0cf8/ethernet@2/ethernet-phy@0 We match pci domain by /pci@i0cf8 in SeaBIOS, but fw_dev_path prefix of q35 is /q35-pcihost@i0cf8. So bootindex in qemu commandline doesn't work if it uses q35 machine type. This patch fixes the pattern to match both original pc-i440fx q35 Signed-off-by: Amos Kong ak...@redhat.com --- src/boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot.c b/src/boot.c index cd9d784..f30d47e 100644 --- a/src/boot.c +++ b/src/boot.c @@ -97,7 +97,7 @@ find_prio(const char *glob) return -1; } -#define FW_PCI_DOMAIN /pci@i0cf8 +#define FW_PCI_DOMAIN /*pci*@i0cf8 The seabios pattern matching code isn't that sophisticated - I think this could end up doing something unexpected. Why does it need to change? If we start a guest with default machine type (pc-i440fx), the prefix of bootindex string is /pci@i0cf8, if we start guest with -M q35, the prefix will become /q35-pcihost@i0cf8. We only match /pci@i0cf8 in seabios, it causes boot priority of q35 devices could not be changed. We could not change TYPE_Q35_HOST_DEVICE to 'pci' in qemu to adapt seabios, so fix the pattern. Why couldn't we? QEMU not been able to generate proper device string is QEMU bug. -- Gleb. I applied a patch that does just that. I noticed :) I have many emails to read :( -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] KVM call agenda for 2013-05-28
On Sun, Jun 02, 2013 at 06:09:50PM +0300, Michael S. Tsirkin wrote: On Sun, Jun 02, 2013 at 06:05:42PM +0300, Gleb Natapov wrote: On Wed, May 29, 2013 at 11:45:44AM +0300, Michael S. Tsirkin wrote: On Tue, May 28, 2013 at 07:53:09PM -0400, Kevin O'Connor wrote: On Thu, May 23, 2013 at 03:41:32PM +0300, Michael S. Tsirkin wrote: Juan is not available now, and Anthony asked for agenda to be sent early. So here comes: Agenda for the meeting Tue, May 28: - Generating acpi tables I didn't see any meeting notes, but I thought it would be worthwhile to summarize the call. This is from memory so correct me if I got anything wrong. Anthony believes that the generation of ACPI tables is the task of the firmware. Reasons cited include security implications of running more code in qemu vs the guest context, complexities in running iasl on big-endian machines, possible complexity of having to regenerate tables on a vm reboot, overall sloppiness of doing it in QEMU. Raised that QOM interface should be sufficient. Kevin believes that the bios table code should be moved up into QEMU. Reasons cited include the churn rate in SeaBIOS for this QEMU feature (15-20% of all SeaBIOS commits since integrating with QEMU have been for bios tables; 20% of SeaBIOS commits in last year), complexity of trying to pass all the content needed to generate the tables (eg, device details, power tree, irq routing), complexity of scheduling changes across different repos and synchronizing their rollout, complexity of implemeting the code in both OVMF and SeaBIOS. Kevin wasn't aware of a requirement to regenerate acpi tables on a vm reboot. I think this last one is based on a misunderstanding: it's based on assumption that we we change hardware by hotplug we should regenerate the tables to match. But there's no management that can take advantage of this. Two possible reasonable things we can tell management: - hotplug for device XXX is not supported: restart qemu to make guest use the device - hotplug for device XXX is supported What is proposed here instead is a third option: - hotplug is supported but device is not functional. reboot guest to make it fully functional This will naturally lead to requirement to regenerate tables on reset. And this is what would happen with guest-generated tables, and I consider this a bug, not a feature. +1. This will probably break guest resume too. If you really wanted to update tables dynamically, without restarting qemu, don't stop there, add an interface for guest to update them without reset. I think that's over-endineering and a requirement that's best avoided. There were discussions on potentially introducing a middle component to generate the tables. Coreboot was raised as a possibility, and David thought it would be okay to use coreboot for both OVMF and SeaBIOS. The possibility was also raised of a rom that lives in the qemu repo, is run in the guest, and generates the tables (which is similar to the hvmloader approach that Xen uses). Anthony requested that patches be made that generate the ACPI tables in QEMU for the upcoming hotplug work, so that they could be evaluated to see if they truly do need to live in QEMU or if the code could live in the firmware. There were no objections. -Kevin I volunteered to implement this. Why hotplug should generate ACPI code? It does not do so on real HW. Hotplug should not generate ACPI code. What is meant here is adding ACPI code to support hotplug of devices behind a PCI to PCI bridge. Ah, OK. This one does not change on reset. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH RFC 0/3] seabios: move acpi table formatting out of bios
On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote: On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote: On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote: Untested yet, but I thought I'd share the BIOS bits so we can agree on direction. In particular check out ROM sizes: - Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom) - Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom) - After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom) - Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom) As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware. This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new /etc/acpi/ directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from /etc/linker-script. It is used to patch in table pointers and checksums. After some thought, there are two additional options worth considering, in that they simplify bios code somewhat: - bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching - further, qemu could do the copy of tables into that address directly This seems more complex than necessary to me. The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg file). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary. +1. This copy of tables into that address directly is just an ad-hoc PV isa DMA device in disguise. Such device was refused when libguestfs asked for it, and they wanted it for much better reason - performance. There is existing mechanism to pass data into firmware. Use it please. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH RFC 0/3] seabios: move acpi table formatting out of bios
On Wed, May 08, 2013 at 01:29:12PM +0300, Michael S. Tsirkin wrote: On Wed, May 08, 2013 at 12:31:50PM +0300, Gleb Natapov wrote: On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote: On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote: On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote: Untested yet, but I thought I'd share the BIOS bits so we can agree on direction. In particular check out ROM sizes: - Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom) - Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom) - After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom) - Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom) As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware. This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new /etc/acpi/ directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from /etc/linker-script. It is used to patch in table pointers and checksums. After some thought, there are two additional options worth considering, in that they simplify bios code somewhat: - bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching - further, qemu could do the copy of tables into that address directly This seems more complex than necessary to me. The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg file). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary. +1. This copy of tables into that address directly is just an ad-hoc PV isa DMA device in disguise. Such device was refused when libguestfs asked for it, and they wanted it for much better reason - performance. There is existing mechanism to pass data into firmware. Use it please. Yes I can code it up using FW_CFG for now. One issue with QEMU_CFG_FILE_DIR is that it's broken wrt migration, unless we pass in very small bits of data which we can guarantee never changes across qemu versions. Shouldn't we guaranty that ACPI tables do not change for the same machine type anyway? Off-list, I suggested fixing it and migrating file content, but Anthony thinks it's a bad idea. Why is this a bad idea to fix device migration? I don't care much. -- Gleb. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH RFC 0/3] seabios: move acpi table formatting out of bios
On Wed, May 08, 2013 at 01:43:25PM +0300, Michael S. Tsirkin wrote: On Wed, May 08, 2013 at 01:34:59PM +0300, Gleb Natapov wrote: On Wed, May 08, 2013 at 01:29:12PM +0300, Michael S. Tsirkin wrote: On Wed, May 08, 2013 at 12:31:50PM +0300, Gleb Natapov wrote: On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote: On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote: On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote: Untested yet, but I thought I'd share the BIOS bits so we can agree on direction. In particular check out ROM sizes: - Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom) - Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom) - After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom) - Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom) As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware. This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new /etc/acpi/ directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from /etc/linker-script. It is used to patch in table pointers and checksums. After some thought, there are two additional options worth considering, in that they simplify bios code somewhat: - bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching - further, qemu could do the copy of tables into that address directly This seems more complex than necessary to me. The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg file). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary. +1. This copy of tables into that address directly is just an ad-hoc PV isa DMA device in disguise. Such device was refused when libguestfs asked for it, and they wanted it for much better reason - performance. There is existing mechanism to pass data into firmware. Use it please. Yes I can code it up using FW_CFG for now. One issue with QEMU_CFG_FILE_DIR is that it's broken wrt migration, unless we pass in very small bits of data which we can guarantee never changes across qemu versions. Shouldn't we guaranty that ACPI tables do not change for the same machine type anyway? That's not practical. They are too big to stay completely unchanged. I will not be surprised if this will cause us problem somehow. Guest will see new tables only after reboot/resume from S4 so damage is limited, but one thing that comes to mind is table's size change. If they grow from one version to the other after resuming a guest from S4 on new QEMU version part of the tables may be corrupted. Off-list, I suggested fixing it and migrating file content, but Anthony thinks it's a bad idea. Why is this a bad idea to fix device migration? You misunderstand I think. Question is whether we should be putting so much info in fw_cfg. If we keep fw_cfg for small things we don't need to migrate it. In that case ACPI tables have to be passed in using some other mechanism. Where this notion that fw_cfg is only for a small things is coming from? I can assure you this was not the case when the device was introduced. In fact it is used today for not so small things like bootindex splash screen bitmaps, option rom loading and kernel/initrd loading. Some of those are bigger then ACPI tables will ever be. And they all should be migrated, so fw_cfg should be fixed anyway. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH RFC 0/3] seabios: move acpi table formatting out of bios
On Wed, May 08, 2013 at 09:15:44PM +0300, Michael S. Tsirkin wrote: On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote: On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote: On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote: Untested yet, but I thought I'd share the BIOS bits so we can agree on direction. In particular check out ROM sizes: - Before patchset with DSDT enabled Total size: 127880 Fixed: 59060 Free: 3192 (used 97.6% of 128KiB rom) - Before patchset with DSDT disabled Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom) - After patchset: Total size: 128776 Fixed: 59100 Free: 2296 (used 98.2% of 128KiB rom) - Legacy disabled at build time: Total size: 119836 Fixed: 58996 Free: 11236 (used 91.4% of 128KiB rom) As can be seen from this, most size savings come from dropping DSDT, but we do save a bit by removing other tables. Of course the real reason to move tables to QEMU is so that ACPI can better match hardware. This patchset adds an option to move all code for formatting acpi tables out of BIOS. With this, QEMU has full control over the table layout. All tables are loaded from the new /etc/acpi/ directory. Any entries in this directory cause BIOS to disable ACPI table generation completely. A generic linker script, controlled by QEMU, is loaded from /etc/linker-script. It is used to patch in table pointers and checksums. After some thought, there are two additional options worth considering, in that they simplify bios code somewhat: - bios could get size from qemu, allocate a buffer (e.g. could be one buffer for all tables) and pass the address to qemu. qemu does all the patching - further, qemu could do the copy of tables into that address directly This seems more complex than necessary to me. The important task is to get the tables generated in QEMU - I'd focus on getting the tables generated in QEMU (one table per fw_cfg file). Once that is done, the SeaBIOS side can be easily implemented, and we can add any enhancements on top if we feel it is necessary. -Kevin I have kind of done this, though only compile-tested for now - still need to update the bios with the new linker interface along the lines suggested by you. If you want to see how the code looks like check out git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi the code is in hw/i386/acpi-build.c and hw/i386/bios-linker-loader.c the history is all messed up now, I'll clean it up shortly. That said, this uses fw_cfg so for this to be acceptable, we need to fix migration with big fw_cfg files. We need to fix it anyway ;) -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 2/2] accept MADT over fw_cfg
On Wed, Apr 24, 2013 at 11:09:16AM +0300, Michael S. Tsirkin wrote: On Tue, Apr 23, 2013 at 08:23:44PM +0300, Michael S. Tsirkin wrote: On Mon, Apr 22, 2013 at 08:38:58PM -0400, Kevin O'Connor wrote: On Mon, Apr 22, 2013 at 10:03:01AM +0300, Michael S. Tsirkin wrote: On Sun, Apr 21, 2013 at 08:39:41PM -0400, Kevin O'Connor wrote: On Sun, Apr 21, 2013 at 11:41:48PM +0300, Michael S. Tsirkin wrote: Okay I'm pretty close to posting some patches that advance this project further, but wanted to check something beforehand: there are several tables that point to other tables (for example: FADT points to DSDT). What I did is provide a list of fixups such that bios can patch in pointers without any need to understand what's what. Thoughts? For the RSDP, RSDT, and FADT I think SeaBIOS should just update those tables to set the pointers within them and then recalculate the checksum. I don't think anything complex is needed - it's easy for SeaBIOS to recognize those special tables and modify them. True, that's simple enough. My worry is we can add more such tables. For example, we can decide to switch to XSDT in the future. I know of the following quirks that would have to be handled: 1 - the RSDP must be in the f-segment (where as all other tables can go into high memory). 2 - the RSDP has a checksum in a different location from the other tables and (with an XSDT) it can have two checksums. 3 - the RSDP has a pointer to the RSDT (and to the XSDT if present). 4 - the RSDT (and XSDT if present) has pointers to all the other tables (except RSDP, RSDT, DSDT, and FACS). The FADT pointer must be first in the list. 5 - the FADT table has pointers to DSDT and FACS. 6 - the FACS table must be 64 byte aligned. So, will a generic scheme really be able to handle all of the above quirks, or will we just be mixing some hardcoded quirks with some generic quirks? And, will the code to handle the above quirks in a generic fashion be of a higher complexity than simply hard-coding it? -Kevin -- So here's an implementation for align and FSEG. Not a big deal as you see. I really have doubts about it however: BIOS still must be able to parse get the resume vector in FACS in order to support wakeup, right? So this means that we need to be able to parse RSDP and FACT. These happen to be the only things that need anything not addressed by ADD and SUB so ... maybe a couple of hardcoded quirks just to allocate these correctly is cleaner. Heh, it's actually pretty easy: let's just ask qemu to give us the address of the resume vector in a file with a pre-defined name. Linker can patch table offset there in the regular way. Seabios can find resume vector address the same way OSPM does: by following pointers in memory. What QEMU has to do with it? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 2/2] accept MADT over fw_cfg
On Tue, May 07, 2013 at 09:54:33PM +0300, Michael S. Tsirkin wrote: On Tue, May 07, 2013 at 09:07:04PM +0300, Gleb Natapov wrote: On Wed, Apr 24, 2013 at 11:09:16AM +0300, Michael S. Tsirkin wrote: On Tue, Apr 23, 2013 at 08:23:44PM +0300, Michael S. Tsirkin wrote: On Mon, Apr 22, 2013 at 08:38:58PM -0400, Kevin O'Connor wrote: On Mon, Apr 22, 2013 at 10:03:01AM +0300, Michael S. Tsirkin wrote: On Sun, Apr 21, 2013 at 08:39:41PM -0400, Kevin O'Connor wrote: On Sun, Apr 21, 2013 at 11:41:48PM +0300, Michael S. Tsirkin wrote: Okay I'm pretty close to posting some patches that advance this project further, but wanted to check something beforehand: there are several tables that point to other tables (for example: FADT points to DSDT). What I did is provide a list of fixups such that bios can patch in pointers without any need to understand what's what. Thoughts? For the RSDP, RSDT, and FADT I think SeaBIOS should just update those tables to set the pointers within them and then recalculate the checksum. I don't think anything complex is needed - it's easy for SeaBIOS to recognize those special tables and modify them. True, that's simple enough. My worry is we can add more such tables. For example, we can decide to switch to XSDT in the future. I know of the following quirks that would have to be handled: 1 - the RSDP must be in the f-segment (where as all other tables can go into high memory). 2 - the RSDP has a checksum in a different location from the other tables and (with an XSDT) it can have two checksums. 3 - the RSDP has a pointer to the RSDT (and to the XSDT if present). 4 - the RSDT (and XSDT if present) has pointers to all the other tables (except RSDP, RSDT, DSDT, and FACS). The FADT pointer must be first in the list. 5 - the FADT table has pointers to DSDT and FACS. 6 - the FACS table must be 64 byte aligned. So, will a generic scheme really be able to handle all of the above quirks, or will we just be mixing some hardcoded quirks with some generic quirks? And, will the code to handle the above quirks in a generic fashion be of a higher complexity than simply hard-coding it? -Kevin -- So here's an implementation for align and FSEG. Not a big deal as you see. I really have doubts about it however: BIOS still must be able to parse get the resume vector in FACS in order to support wakeup, right? So this means that we need to be able to parse RSDP and FACT. These happen to be the only things that need anything not addressed by ADD and SUB so ... maybe a couple of hardcoded quirks just to allocate these correctly is cleaner. Heh, it's actually pretty easy: let's just ask qemu to give us the address of the resume vector in a file with a pre-defined name. Linker can patch table offset there in the regular way. Seabios can find resume vector address the same way OSPM does: by following pointers in memory. Yes, that's what we do now. Good. What QEMU has to do with it? The paragraphs above explain the connection. Do not see any explanation anywhere. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 2/2] accept MADT over fw_cfg
On Tue, May 07, 2013 at 10:33:17PM +0300, Michael S. Tsirkin wrote: On Tue, May 07, 2013 at 10:26:34PM +0300, Gleb Natapov wrote: On Tue, May 07, 2013 at 09:54:33PM +0300, Michael S. Tsirkin wrote: On Tue, May 07, 2013 at 09:07:04PM +0300, Gleb Natapov wrote: On Wed, Apr 24, 2013 at 11:09:16AM +0300, Michael S. Tsirkin wrote: On Tue, Apr 23, 2013 at 08:23:44PM +0300, Michael S. Tsirkin wrote: On Mon, Apr 22, 2013 at 08:38:58PM -0400, Kevin O'Connor wrote: On Mon, Apr 22, 2013 at 10:03:01AM +0300, Michael S. Tsirkin wrote: On Sun, Apr 21, 2013 at 08:39:41PM -0400, Kevin O'Connor wrote: On Sun, Apr 21, 2013 at 11:41:48PM +0300, Michael S. Tsirkin wrote: Okay I'm pretty close to posting some patches that advance this project further, but wanted to check something beforehand: there are several tables that point to other tables (for example: FADT points to DSDT). What I did is provide a list of fixups such that bios can patch in pointers without any need to understand what's what. Thoughts? For the RSDP, RSDT, and FADT I think SeaBIOS should just update those tables to set the pointers within them and then recalculate the checksum. I don't think anything complex is needed - it's easy for SeaBIOS to recognize those special tables and modify them. True, that's simple enough. My worry is we can add more such tables. For example, we can decide to switch to XSDT in the future. I know of the following quirks that would have to be handled: 1 - the RSDP must be in the f-segment (where as all other tables can go into high memory). 2 - the RSDP has a checksum in a different location from the other tables and (with an XSDT) it can have two checksums. 3 - the RSDP has a pointer to the RSDT (and to the XSDT if present). 4 - the RSDT (and XSDT if present) has pointers to all the other tables (except RSDP, RSDT, DSDT, and FACS). The FADT pointer must be first in the list. 5 - the FADT table has pointers to DSDT and FACS. 6 - the FACS table must be 64 byte aligned. So, will a generic scheme really be able to handle all of the above quirks, or will we just be mixing some hardcoded quirks with some generic quirks? And, will the code to handle the above quirks in a generic fashion be of a higher complexity than simply hard-coding it? -Kevin -- So here's an implementation for align and FSEG. Not a big deal as you see. I really have doubts about it however: BIOS still must be able to parse get the resume vector in FACS in order to support wakeup, right? So this means that we need to be able to parse RSDP and FACT. These happen to be the only things that need anything not addressed by ADD and SUB so ... maybe a couple of hardcoded quirks just to allocate these correctly is cleaner. Heh, it's actually pretty easy: let's just ask qemu to give us the address of the resume vector in a file with a pre-defined name. Linker can patch table offset there in the regular way. Seabios can find resume vector address the same way OSPM does: by following pointers in memory. Yes, that's what we do now. Good. What QEMU has to do with it? The paragraphs above explain the connection. Do not see any explanation anywhere. Maybe I don't understand your question then. What exactly would you like to know? My question is why would you ask qemu to give us the address of the resume vector in a file with a pre-defined name. But since you do not do that in your patches I think we are in agreement that this is not needed. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 2/2] accept MADT over fw_cfg
On Tue, May 07, 2013 at 10:49:14PM +0300, Michael S. Tsirkin wrote: On Tue, May 07, 2013 at 10:37:28PM +0300, Gleb Natapov wrote: On Tue, May 07, 2013 at 10:33:17PM +0300, Michael S. Tsirkin wrote: On Tue, May 07, 2013 at 10:26:34PM +0300, Gleb Natapov wrote: On Tue, May 07, 2013 at 09:54:33PM +0300, Michael S. Tsirkin wrote: On Tue, May 07, 2013 at 09:07:04PM +0300, Gleb Natapov wrote: On Wed, Apr 24, 2013 at 11:09:16AM +0300, Michael S. Tsirkin wrote: On Tue, Apr 23, 2013 at 08:23:44PM +0300, Michael S. Tsirkin wrote: On Mon, Apr 22, 2013 at 08:38:58PM -0400, Kevin O'Connor wrote: On Mon, Apr 22, 2013 at 10:03:01AM +0300, Michael S. Tsirkin wrote: On Sun, Apr 21, 2013 at 08:39:41PM -0400, Kevin O'Connor wrote: On Sun, Apr 21, 2013 at 11:41:48PM +0300, Michael S. Tsirkin wrote: Okay I'm pretty close to posting some patches that advance this project further, but wanted to check something beforehand: there are several tables that point to other tables (for example: FADT points to DSDT). What I did is provide a list of fixups such that bios can patch in pointers without any need to understand what's what. Thoughts? For the RSDP, RSDT, and FADT I think SeaBIOS should just update those tables to set the pointers within them and then recalculate the checksum. I don't think anything complex is needed - it's easy for SeaBIOS to recognize those special tables and modify them. True, that's simple enough. My worry is we can add more such tables. For example, we can decide to switch to XSDT in the future. I know of the following quirks that would have to be handled: 1 - the RSDP must be in the f-segment (where as all other tables can go into high memory). 2 - the RSDP has a checksum in a different location from the other tables and (with an XSDT) it can have two checksums. 3 - the RSDP has a pointer to the RSDT (and to the XSDT if present). 4 - the RSDT (and XSDT if present) has pointers to all the other tables (except RSDP, RSDT, DSDT, and FACS). The FADT pointer must be first in the list. 5 - the FADT table has pointers to DSDT and FACS. 6 - the FACS table must be 64 byte aligned. So, will a generic scheme really be able to handle all of the above quirks, or will we just be mixing some hardcoded quirks with some generic quirks? And, will the code to handle the above quirks in a generic fashion be of a higher complexity than simply hard-coding it? -Kevin -- So here's an implementation for align and FSEG. Not a big deal as you see. I really have doubts about it however: BIOS still must be able to parse get the resume vector in FACS in order to support wakeup, right? So this means that we need to be able to parse RSDP and FACT. These happen to be the only things that need anything not addressed by ADD and SUB so ... maybe a couple of hardcoded quirks just to allocate these correctly is cleaner. Heh, it's actually pretty easy: let's just ask qemu to give us the address of the resume vector in a file with a pre-defined name. Linker can patch table offset there in the regular way. Seabios can find resume vector address the same way OSPM does: by following pointers in memory. Yes, that's what we do now. Good. What QEMU has to do with it? The paragraphs above explain the connection. Do not see any explanation anywhere. Maybe I don't understand your question then. What exactly would you like to know? My question is why would you ask qemu to give us the address of the resume vector in a file with a pre-defined name. But since you do not do that in your patches I think we are in agreement that this is not needed. -- Gleb. Generally I'd rather not write summaries of old discussions. But just this once. Not strictly. The thread discussed what should we do with FACS which has alignment requirements: let bios detect FACS and align, or have qemu tell bios align this table at X. If we do later, it appeared we still need knowledge of the FACS layout in bios, for resume, so why try to be generic? Unless, we add another channel to tell bios where resume
Re: [SeaBIOS] [PATCH v16] Add pvpanic device driver
On Sat, Mar 30, 2013 at 09:20:09AM -0400, Kevin O'Connor wrote: On Fri, Mar 29, 2013 at 02:49:12PM +0100, Paolo Bonzini wrote: Il 29/03/2013 14:33, Kevin O'Connor ha scritto: On Fri, Mar 29, 2013 at 04:18:44PM +0800, Hu Tao wrote: pvpanic device is used to notify host(qemu) when guest panic happens. Thanks. However, we're planning a move of ACPI tables from SeaBIOS to QEMU. I think this should wait until after the move. The device should be in QEMU 1.5, and the SSDT probably will still be in SeaBIOS by then (and might even be the last to move, since it's quite complex and dynamic). I don't think it is fair to block this patch on those grounds... What is the user visible impact of not having a panic device? My main concern is that the patch creates a new fw_cfg channel between qemu and seabios thats sole purpose is to alter the OS visible ACPI tables. These types of QEMU-SeaBIOS interfaces are fragile and are (in sum) quite complex. The patch uses existing channel between qemu and seabios, one romfile_loadint() is all it takes. We already have number of interfaces to change OS visible ACPI tables, that's why we want to move ACPI table creation to QEMU in the first place. It is unfortunate to start blocking features now before we have an alternative. When ACPI table creation will move into QEMU the code in this patch will be dropped along with all the other code that serves similar purpose. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v15 2/2] patch dsdt to use passed-in pvpanic ioport
On Thu, Mar 21, 2013 at 05:17:11PM +0800, Hu Tao wrote: On Thu, Mar 21, 2013 at 10:11:54AM +0100, Paolo Bonzini wrote: Il 21/03/2013 10:08, Hu Tao ha scritto: Signed-off-by: Hu Tao hu...@cn.fujitsu.com --- src/acpi-dsdt-isa.dsl | 14 -- src/acpi.c| 9 + 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/acpi-dsdt-isa.dsl b/src/acpi-dsdt-isa.dsl index 87a31b9..43fe719 100644 --- a/src/acpi-dsdt-isa.dsl +++ b/src/acpi-dsdt-isa.dsl @@ -102,7 +102,9 @@ Scope(\_SB.PCI0.ISA) { Device(PEVT) { Name(_HID, QEMU0001) -OperationRegion(PEOR, SystemIO, 0x0505, 0x01) +ACPI_EXTRACT_NAME_WORD_CONST dsdt_isa_pest +Name(PEST, 0x505) +OperationRegion(PEOR, SystemIO, PEST, 0x01) Field(PEOR, ByteAcc, NoLock, Preserve) { PEPT, 8, } @@ -126,7 +128,15 @@ Scope(\_SB.PCI0.ISA) { } Name(_CRS, ResourceTemplate() { -IO(Decode16, 0x0505, 0x0505, 0x01, 0x01) +IO(Decode16, 0x0505, 0x0505, 0x01, 0x01, IO) }) + +CreateWordField(_CRS, IO._MIN, IOMN) +CreateWordField(_CRS, IO._MAX, IOMX) + +Method(_INI, 0, NotSerialized) { +Store(PEST, IOMN) +Store(PEST, IOMX) +} } } diff --git a/src/acpi.c b/src/acpi.c index 119d1c1..42fa06e 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -286,11 +286,20 @@ static const struct pci_device_id fadt_init_tbl[] = { PCI_DEVICE_END }; +static void patch_dsdt(void *dsdt) +{ +u8 *dsdt_ptr = dsdt; +int pvpanic_port = romfile_loadint(etc/pvpanic-port, 0x505); The default must be 0. Also, here: + +Method(_STA, 0, NotSerialized) { +Store(PEPT, Local0) +If (LEqual(Local0, Zero)) { +Return (0x00) +} Else { +Return (0x0F) +} +} + You must change it to look at PEST instead of PEPT (i.e. do not probe, just see if you have a meaningful address). Just squash the patches, it's simpler that way probably. I forgot to add RFC to the title. This patch doesn't work for q35 with custom ioport. Why doesn't it work with q35? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH please reply] make acpi bits GPLv2 compatible
On Wed, Mar 20, 2013 at 05:57:53PM +0200, Michael S. Tsirkin wrote: You are getting this mail because you might have contributed code to one of the files in seabios that we want to reuse in QEMU, when this file was under GPLv3 or LGPLv3. QEMU is GPLv2 at the moment, so as a step in the process of moving acpi tables to qemu, we need to make sure the code we'll be moving is GPLv2 compatible. The code was originally LGPLv2 in bochs so these bits are OK. QEMU generally prefers GPLv2 or later, so this is what this patch does. The plan is therefore: - collect acks from everyone - copy code to QEMU and apply this patch to QEMU copy only If you allow the use of your contribution in QEMU under the terms of GPLv2 or later as proposed by this patch, please respond to this mail including the line: Acked-by: Name email address in the message body. For example: Acked-by: Michael S. Tsirkin m...@redhat.com Thanks! Signed-off-by: Michael S. Tsirkin m...@redhat.com Acked-by: Gleb Natapov g...@redhat.com -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 0/2][RFC] Reduce mptable code dependencies
On Tue, Mar 19, 2013 at 02:09:05AM +0100, Peter Stuge wrote: Kevin O'Connor wrote: Thoughts? I wish this effort could go into coreboot instead, so that it could benefit more than only one machine. The code is QEMU specific. What other machines are you talking about? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 0/2][RFC] Reduce mptable code dependencies
On Mon, Mar 18, 2013 at 08:38:59PM -0400, Kevin O'Connor wrote: These two patches should make the mptable code easier to move out of SeaBIOS and into QEMU. The first patch stops the mptable from describing PCI-to-PCI bridges - I don't believe it was the goal (or a requirement) of the mptable to describe bridges. The second patch synchs the list of PCI irqs between the ACPI code and the mptable code. This has only been lightly tested. Thoughts? I wish I would remember why I added PCI bus enumeration code, but I think you are right. If there is more then one PCI domain then each one of them should be listed, but PCI-to-PCI bridges are part of the same bus. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Add pvevent device driver
On Thu, Mar 14, 2013 at 05:33:19PM +0800, Hu Tao wrote: On Thu, Mar 14, 2013 at 10:57:18AM +0200, Gleb Natapov wrote: On Thu, Mar 14, 2013 at 04:48:47PM +0800, Hu Tao wrote: pvevent device is used to notify host(qemu) when guest panic happens. ref: http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg02293.html Signed-off-by: Hu Tao hu...@cn.fujitsu.com --- src/acpi-dsdt-isa.dsl | 30 ++ 1 file changed, 30 insertions(+) diff --git a/src/acpi-dsdt-isa.dsl b/src/acpi-dsdt-isa.dsl index 23761db..d083245 100644 --- a/src/acpi-dsdt-isa.dsl +++ b/src/acpi-dsdt-isa.dsl @@ -99,4 +99,34 @@ Scope(\_SB.PCI0.ISA) { IRQNoFlags() { 3 } }) } + +Device(PEVT) { +Name(_HID, MSFT0001) We cannot use MSFT! OK, I see now, we have to use QEMU0001 or like. More question: if I request ACPI ID: QEMU from pn...@microsoft.com, who should be CCed, Anthony, qemu-list or any others? Cannot really answer that. I do not think mailing list should be included. My be Anthony should request the ID on behalf of QEMU project. +OperationRegion(PEOR, SystemIO, 0x0505, 0x01) IO port should be received form QEMU by fw_cfg and patched here at run time. If I'm right, io port can be passed to seabios through fw_cfg file interface, but I'm still figuring out how to patch DSDT here and below at run time. Maybe build_ssdt() is close to this. Yes, we already are doing similar things in build_ssdt(). -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Add pvevent device driver
On Thu, Mar 14, 2013 at 10:08:09AM +, David Woodhouse wrote: On Thu, 2013-03-14 at 10:57 +0200, Gleb Natapov wrote: +OperationRegion(PEOR, SystemIO, 0x0505, 0x01) IO port should be received form QEMU by fw_cfg and patched here at run time. Pfft. ACPI table should be received from QEMU. :) That's the feature that, as far as I understand, everyone agreed upon, but since we want to apply these patches before that feature is here we will have to do it old fashioned way for now. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] Moving BIOS tables from SeaBIOS to QEMU
On Sun, Feb 24, 2013 at 01:00:28PM -0500, Kevin O'Connor wrote: On Sat, Feb 23, 2013 at 04:47:26PM +, David Woodhouse wrote: On Sat, 2013-02-23 at 11:38 -0500, Kevin O'Connor wrote: IMO, we need to move the ACPI table creation (and PIR/MPTABLE/SMBIOS) to QEMU and just have QEMU pass the tables to SeaBIOS for it to copy into memory like it does on CSM, coreboot, and Xen. I believe it's on Laszlo's TODO list. Laszlo, what is your plan for doing this? I did a review of the SeaBIOS code to see what information is currently used to generate the ACPI, SMBIOS, MPTABLE, and PIR bios tables. Here's what I came up with: - hardcoded information: Most of the tables are simply hardcoded with various values. This should not be a problem to move to QEMU IIRC SMBIOS has some tables with information about a BIOS. - information passed in from QEMU: RamSize, RamSizeOver4G, fw_cfg (irq0-override, system suspend states, numa memory, additional acpi tables, smbios overrides). These should also be possible to obtain directly within QEMU (though I'm unsure how qemu exposes this information internally). - CPU information: Number of CPUs, the apic id of the CPUs, which CPUs are active, and the cpuid information from the first CPU. Again this should be available in QEMU, but I'm not sure what the internal interfaces look like for obtaining it. - Various hardware probes: The ioapic version, whether or not hpet is present, running on piix4 or ich9, whether or not acpi should be used. Again should be possible to obtain from QEMU with sufficient interfaces. - PCI device info: The list of PCI devices, PCI buses, pin assignments, irq assignments, if hotplug supported, and memory regions. This should mostly be available in QEMU - order of initializing would be important so that the tables were initialized after all PCI devices. Of these, the only thing I see that could be problematic is the PCI irq assignments (used in mptable) and the PCI region space (used in ACPI DSDT _SB.PCI.CRS). These are slightly problematic as they currently rely somewhat on the current SeaBIOS pciinit.c bridge/device setup. However, the mptable irqs is a simple algorithm that could be replicated in QEMU, and it looks to be of dubious value anyway (so could possibly be dropped from the mptable). Also, the PCI region space does not need to be exact, so a heuristic that just ensured it was large enough should suffice. Again IIRC there are still OSes that uses mptable to obtain irq information. See 928d4dffef5c374. Given this, one possible way to migrate the ACPI tables from SeaBIOS would be to: 1 - replace the BDAT PCI range interface in SeaBIOS with a SSDT based template system similar to the way software suspend states are handled in SeaBIOS today. This would eliminate the only runtime references to SeaBIOS memory from ACPI. 2 - relicense the SeaBIOS' acpi.c, mptable.c, pirtable.c, smbios.c code to GPLv2 (from LGPLv3) and copy into QEMU. Only I've claimed a copyright since Fabrice's work (LGPLv2) and I'm willing to relicense. There have been a handful of contributors to these files, but they all look to be regular QEMU contributors so I don't think there would be any objections. Along with the code, the IASL parsing code and associated build python scripts would also need to be copied into QEMU. 3 - update the code to use the internal QEMU interfaces instead of the SeaBIOS interfaces to obtain the information outlined above. 4 - pass the tables from QEMU to SeaBIOS via the fw_cfg interface. The PIR, MPTABLE, and SMBIOS are easy to copy into memory from fw_cfg. The ACPI does have a few tables that are special (RSDP, RSDT, FADT, DSDT, FACS), but it should be easy to detect these and update the pointers in SeaBIOS during the copy to memory. Thoughts? -Kevin -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Mon, Feb 18, 2013 at 02:33:23PM -0500, Kevin O'Connor wrote: On Mon, Feb 18, 2013 at 09:17:05PM +0200, Gleb Natapov wrote: On Mon, Feb 18, 2013 at 02:00:52PM -0500, Kevin O'Connor wrote: Why not fix KVM so that it runs at fff0 after reset? Because KVM uses VMX extension and VMX on CPU without unrestricted guest is not capable of doing so. Recent KVM code should be able to emulate real mode from the fff0 address instead of trying to enter vmx guest mode. I asked Laszlo to check if it is so, but even if KVM in 3.9 will work it will not fix all existent kernels out there. Old behaviour of approximating real mode by vm86 is still supported by using emulate_invalid_guest_state=false kernel module option and it will be nice if it will not break OVMF since it can be used as a workaround in case unemulated instruction is encountered. For old versions of KVM, SeaBIOS can detect the loop and issue a shutdown. Not nice for users to have their reboot turn into a poweroff, but likely better than just a hang. The only thing SeaBIOS could do is setup the segment registers and then jump to fff0, which is a bit of work for the same end result. If it will jump to fff0 KVM will jump to 0 instead :) It should restore pre-CSM loaded OVMF state and reset. I take it you mean copy 0xfffe to 0xe? That would not be fun. SeaBIOS would need to detect that it's in the state (it's definitely not correct to do that on real-hardware or on working kvm instances), then setup a trampoline somewhere outside of 0xe-0xf to do the memcpy, jump to that trampoline, copy the memory, restore segment registers, and then jump to 0xfff0. That's a lot of kvm specific code to add to seabios as a workaround and it seems fragile anyway. Isn't this exactly what qemu_prep_reset() is doing now? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Tue, Feb 19, 2013 at 06:35:03PM +, David Woodhouse wrote: On Tue, 2013-02-19 at 20:13 +0200, Gleb Natapov wrote: I take it you mean copy 0xfffe to 0xe? That would not be fun. SeaBIOS would need to detect that it's in the state (it's definitely not correct to do that on real-hardware or on working kvm instances), then setup a trampoline somewhere outside of 0xe-0xf to do the memcpy, jump to that trampoline, copy the memory, restore segment registers, and then jump to 0xfff0. That's a lot of kvm specific code to add to seabios as a workaround and it seems fragile anyway. Isn't this exactly what qemu_prep_reset() is doing now? No. It doesn't do the trampoline thing because it doesn't *have* to; it's copying an identical copy of the code back over itself. Ah, yes of course. So does CSM takes the whole 0xe-0xf segment or it leaves OVMF code there somewhere. CSM reset code can jump into OVMF code in 0xe-0xf range and let it do the copy. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Tue, Feb 19, 2013 at 06:48:41PM +, David Woodhouse wrote: On Tue, 2013-02-19 at 20:41 +0200, Gleb Natapov wrote: Ah, yes of course. So does CSM takes the whole 0xe-0xf segment or it leaves OVMF code there somewhere. CSM reset code can jump into OVMF code in 0xe-0xf range and let it do the copy. There is no OVMF code there; OVMF doesn't bother to put *anything* into the RAM at 1MiB-δ unless there's a CSM. It runs from ROM and do not shadow itself? CSM code isn't supposed to be hardware-specific, but I suppose for the CSM running under KVM case we could *potentially* have a hack at the reset vector so that when we do find ourselves there under a buggy qemu/KVM implementation, it could set up a trampoline, reset the PAM registers manually (so that the KVM CS base address bug doesn't actually *hurt* us), then try again? Yes, we are trying to come up with qemu/KVM specific hack here. I'd rather implement the 0xcf9 reset properly in qemu though, and make SeaBIOS use that (which it can do *sanely* as a CSM if it's in the ACPI tables). I didn't follow that other discussion about hard/soft reset. How proper 0xcf9 reset will fix the problem? What will it do that system_reset does not? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Mon, Feb 18, 2013 at 07:16:25PM +0100, Laszlo Ersek wrote: On 02/18/13 18:45, Gleb Natapov wrote: On Mon, Feb 18, 2013 at 06:12:55PM +0100, Laszlo Ersek wrote: CS =f000 000f f300 ^^^^ |base limitflags selector This is because real mode is emulated as vm86 mode on intel cpus without unrestricted guest flag. Awesome, this supports my desperate hunch in http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg02689.html. I hope David can confirm in practice! Laszlo explained to me that the problem is that after reset we end up in SeaBIOS reset code instead of OVMF one. This is because kvm starts to execute from 0 instead of fff0 after reset and this memory location is modifying during CSM loading. Seabios solves this problem by detecting reset condition and copying pristine image of itself from the end of 4G to the end of 1M. OVMF should do the same, but with CSM it does not get control back after reset since Seabios reset vector is executed instead. Why not put OVMF reset code at reset vector in CSM built SeaBIOS to solve the problem? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Mon, Feb 18, 2013 at 06:12:55PM +0100, Laszlo Ersek wrote: On 02/18/13 13:53, David Woodhouse wrote: Nevertheless, on my workstation as on yours, we do seem to end up executing from the CSM in RAM when we reset. But on my laptop, it executes the *ROM* as it should. This patch 'fixes' it, and I think it might even be correct in itself, but I don't think it's a correct fix for the problem we're discussing. And I certainly want to know what's different on my laptop that makes it work *without* this patch. Either there's some weirdness with setting the high CS base address, on CPU reset. Or perhaps the contents of the memory region at 0xfff0 have *really* been changed along with the sub-1MiB range. Or maybe the universe just hates us... We're ending up in the wrong place, under 1MB (which is consistent with your reset the PAMs patch -- state of PAMs should only matter below 1MB). I single-stepped qemu-1.3.1 in x86_cpu_reset() / cpu_x86_load_seg_cache(), and we seem to set the correct base. However when I pause the VM when it's spinning in the reset loop, and I issue the following in virsh: # qemu-monitor-command --domain \ fw-mixed.g-f18xfce2012121716.e-upstream --hmp --cmd \ cpu 0 # qemu-monitor-command --domain \ fw-mixed.g-f18xfce2012121716.e-upstream --hmp --cmd \ info registers for EIP and CS I get (from cpu_x86_dump_seg_cache(), in the HF_CS64_MASK clear branch): EAX= EBX= ECX= EDX=0623 ESI= EDI= EBP= ESP= EIP=fff0 EFL=0002 [---] CPL=3 II=0 A20=1 SMM=0 HLT=0 ES = f300 CS =f000 000f f300 ^^^^ |base limitflags selector This is because real mode is emulated as vm86 mode on intel cpus without unrestricted guest flag. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM
On Mon, Feb 18, 2013 at 02:00:52PM -0500, Kevin O'Connor wrote: On Mon, Feb 18, 2013 at 08:31:01PM +0200, Gleb Natapov wrote: Laszlo explained to me that the problem is that after reset we end up in SeaBIOS reset code instead of OVMF one. This is because kvm starts to execute from 0 instead of fff0 after reset and this memory location is modifying during CSM loading. Seabios solves this problem by detecting reset condition and copying pristine image of itself from the end of 4G to the end of 1M. OVMF should do the same, but with CSM it does not get control back after reset since Seabios reset vector is executed instead. Why not put OVMF reset code at reset vector in CSM built SeaBIOS to solve the problem? Why not fix KVM so that it runs at fff0 after reset? Because KVM uses VMX extension and VMX on CPU without unrestricted guest is not capable of doing so. Recent KVM code should be able to emulate real mode from the fff0 address instead of trying to enter vmx guest mode. I asked Laszlo to check if it is so, but even if KVM in 3.9 will work it will not fix all existent kernels out there. Old behaviour of approximating real mode by vm86 is still supported by using emulate_invalid_guest_state=false kernel module option and it will be nice if it will not break OVMF since it can be used as a workaround in case unemulated instruction is encountered. The only thing SeaBIOS could do is setup the segment registers and then jump to fff0, which is a bit of work for the same end result. If it will jump to fff0 KVM will jump to 0 instead :) It should restore pre-CSM loaded OVMF state and reset. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu PATCH v2] add a boot option to do strict boot
On Wed, Jan 09, 2013 at 04:39:13PM +0800, Amos Kong wrote: Current seabios will try to boot from selected devices first, if they are all failed, seabios will also try to boot from un-selected devices. We need to make it configurable. I already posted a seabios patch to add a new device type to halt booting. Qemu can add HALT at the end of bootindex string, then seabios will halt booting after trying to boot from selected devices. This option only effects when boot priority is changed by bootindex options, the old style(-boot order=..) will still try to boot from un-selected devices. v2: add HALT entry in get_boot_devices_list() define boot_strict to bool Signed-off-by: Amos Kong ak...@redhat.com Acked-by: Gleb Natapov g...@redhat.com --- [SeaBIOS PATCH v3] boot: add a new type to halt booting https://github.com/kongove/seabios/commit/39aeded2da6254eab2c34de92371ce1cad5c793e bios.bin: http://kongove.fedorapeople.org/pub/added-halt-type-bios.bin --- qemu-config.c |3 +++ qemu-options.hx |8 ++-- vl.c| 22 +- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/qemu-config.c b/qemu-config.c index 2188c3e..dd6d249 100644 --- a/qemu-config.c +++ b/qemu-config.c @@ -612,6 +612,9 @@ QemuOptsList qemu_boot_opts = { }, { .name = reboot-timeout, .type = QEMU_OPT_STRING, +}, { +.name = strict, +.type = QEMU_OPT_STRING, }, { /*End of list */ } }, diff --git a/qemu-options.hx b/qemu-options.hx index 9df0cde..5408837 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -376,14 +376,14 @@ ETEXI DEF(boot, HAS_ARG, QEMU_OPTION_boot, -boot [order=drives][,once=drives][,menu=on|off]\n - [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time]\n + [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time][,strict=on|off]\n 'drives': floppy (a), hard disk (c), CD-ROM (d), network (n)\n 'sp_name': the file's name that would be passed to bios as logo picture, if menu=on\n 'sp_time': the period that splash picture last if menu=on, unit is ms\n 'rb_timeout': the timeout before guest reboot when boot failed, unit is ms\n, QEMU_ARCH_ALL) STEXI -@item -boot [order=@var{drives}][,once=@var{drives}][,menu=on|off][,splash=@var{sp_name}][,splash-time=@var{sp_time}][,reboot-timeout=@var{rb_timeout}] +@item -boot [order=@var{drives}][,once=@var{drives}][,menu=on|off][,splash=@var{sp_name}][,splash-time=@var{sp_time}][,reboot-timeout=@var{rb_timeout}][,strict=on|off] @findex -boot Specify boot order @var{drives} as a string of drive letters. Valid drive letters depend on the target achitecture. The x86 PC uses: a, b @@ -407,6 +407,10 @@ when boot failed, then reboot. If @var{rb_timeout} is '-1', guest will not reboot, qemu passes '-1' to bios by default. Currently Seabios for X86 system support it. +Do strict boot via @option{strict=on} as far as firmware/BIOS +supports it. This only effects when boot priority is changed by +bootindex options. The default is non-strict boot. + @example # try to boot from network first, then from hard disk qemu-system-i386 -boot order=nc diff --git a/vl.c b/vl.c index 79e5122..1c64449 100644 --- a/vl.c +++ b/vl.c @@ -230,6 +230,7 @@ int ctrl_grab = 0; unsigned int nb_prom_envs = 0; const char *prom_envs[MAX_PROM_ENVS]; int boot_menu; +bool boot_strict = false; uint8_t *boot_splash_filedata; int boot_splash_filedata_size; uint8_t qemu_extra_params_fw[2]; @@ -1052,6 +1053,12 @@ char *get_boot_devices_list(uint32_t *size) *size = total; +if (boot_strict *size 0) { +list[total-1] = '\n'; +list = g_realloc(list, total + 4); +memcpy(list[total], HALT, 4); +*size = total + 4; +} return list; } @@ -2804,7 +2811,7 @@ int main(int argc, char **argv, char **envp) static const char * const params[] = { order, once, menu, splash, splash-time, -reboot-timeout, NULL +reboot-timeout, strict, NULL }; char buf[sizeof(boot_devices)]; char *standard_boot_devices; @@ -2847,6 +2854,19 @@ int main(int argc, char **argv, char **envp) exit(1); } } +if (get_param_value(buf, sizeof(buf), +strict, optarg)) { +if (!strcmp(buf, on)) { +boot_strict = true; +} else if (!strcmp(buf, off
Re: [SeaBIOS] [Qemu-devel] [libvirt] [Qemu PATCH v2] add a boot option to do strict boot
On Wed, Jan 09, 2013 at 12:28:57PM -0500, Laine Stump wrote: On 01/09/2013 10:22 AM, Daniel P. Berrange wrote: On Wed, Jan 09, 2013 at 08:14:07AM -0700, Eric Blake wrote: On 01/09/2013 01:39 AM, Amos Kong wrote: Current seabios will try to boot from selected devices first, if they are all failed, seabios will also try to boot from un-selected devices. We need to make it configurable. I already posted a seabios patch to add a new device type to halt booting. Qemu can add HALT at the end of bootindex string, then seabios will halt booting after trying to boot from selected devices. This option only effects when boot priority is changed by bootindex options, the old style(-boot order=..) will still try to boot from un-selected devices. v2: add HALT entry in get_boot_devices_list() define boot_strict to bool Signed-off-by: Amos Kong ak...@redhat.com --- Libvirt will need to expose an attribute that lets the user control whether to use this new option; how do we probe via QMP whether the new -boot strict=on command-line option is available? While libvirt should make use of this, we don't need to expose it in the XML. This new behaviour is what we wanted to have all along, so we should just enable it. I agree that this is the way it *should* always work, but apparently there are people who depend on the old behavior, so just doing a blanket switch to the new behavior could lead to setups that no longer work properly after an upgrade, which unfortunately means that existing functionality needs to be maintained, and correct functionality must be triggered by a config switch (maybe Gleb can expand on the use cases that require this if more details are needed, as it's him I heard this from) It is common to configure PXE boot as highest prio for easy re-imaging, Usually boot from PXE fails and other bootable device is used instead, but if re-installation is needed it is as easy as configuring PXE server and rebooting the machine. With current behaviour it is enough to boost PXE priority, if we will change it setups that didn't explicitly specify all devices' priorities will stop working. The rule is easy: if exist command line that will work differently before and after the change you probably break someones setup with your patch. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [Resend][Seabios PATCH] don't boot from un-selected devices
On Tue, Dec 25, 2012 at 05:59:04PM +0200, Ronen Hod wrote: On 12/19/2012 11:32 AM, Gleb Natapov wrote: On Wed, Dec 19, 2012 at 03:24:45PM +0800, Amos Kong wrote: Current seabios will try to boot from selected devices first, if they are all failed, seabios will also try to boot from un-selected devices. For example: @ qemu-kvm -boot order=n,menu=on ... Guest will boot from network first, if it's failed, guest will try to boot from other un-selected devices (floppy, cdrom, disk) one by one. Sometimes, user don't want to boot from some devices. This patch changes And sometimes he want. The patch changes behaviour unconditionally. New behaviour should be user selectable. Something line -boot order=strict on qemu command line. Another option would be to add a terminator symbol, say T (I couldn't find a good terminator), so that order=ndT, would mean strict nd. Not very helpful if the order is specified with bootindex like it should. Ronen. seabios to boot only from selected devices. If user choose first boot device from menu, then seabios will try all the devices, even some of them are not selected. Signed-off-by: Amos Kong ak...@redhat.com --- Resend for CCing seabios maillist. --- src/boot.c | 13 - 1 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/boot.c b/src/boot.c index 3ca7960..ee810ac 100644 --- a/src/boot.c +++ b/src/boot.c @@ -424,6 +424,10 @@ interactive_bootmenu(void) maxmenu++; printf(%d. %s\n, maxmenu , strtcpy(desc, pos-description, ARRAY_SIZE(desc))); +/* If user chooses first boot device from menu, we will treat + all the devices as selected. */ +if (pos-priority == DEFAULT_PRIO) +pos-priority = DEFAULT_PRIO - 1; pos = pos-next; } @@ -490,7 +494,10 @@ boot_prep(void) // Map drives and populate BEV list struct bootentry_s *pos = BootList; -while (pos) { + +/* The priority of un-selected device is not changed, + we only boot from user selected devices. */ +while (pos pos-priority != DEFAULT_PRIO) { switch (pos-type) { case IPL_TYPE_BCV: call_bcv(pos-vector.seg, pos-vector.offset); @@ -513,10 +520,6 @@ boot_prep(void) } pos = pos-next; } - -// If nothing added a floppy/hd boot - add it manually. -add_bev(IPL_TYPE_FLOPPY, 0); -add_bev(IPL_TYPE_HARDDISK, 0); } -- 1.7.1 -- Gleb. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Resend][Seabios PATCH] don't boot from un-selected devices
On Tue, Dec 25, 2012 at 11:58:08AM +0800, Amos Kong wrote: On Wed, Dec 19, 2012 at 11:32:08AM +0200, Gleb Natapov wrote: On Wed, Dec 19, 2012 at 03:24:45PM +0800, Amos Kong wrote: Current seabios will try to boot from selected devices first, if they are all failed, seabios will also try to boot from un-selected devices. For example: @ qemu-kvm -boot order=n,menu=on ... Guest will boot from network first, if it's failed, guest will try to boot from other un-selected devices (floppy, cdrom, disk) one by one. Sometimes, user don't want to boot from some devices. This patch changes Hi Gleb, And sometimes he want. The patch changes behaviour unconditionally. New behaviour should be user selectable. Something line -boot order=strict on qemu command line. Sometimes, user don't know which devices are in boot list of seabios, so they could not disable them through qemu cmdline. This is not what I suggested though. And currently we do not have a way to remove one device from the boot process. This is separate issue and requires separate patch. I didn't describe the purpose clearly. Currently we can assign boot order by -boot order=..., if fails to boot from all devices in order parameters, other devices in seabios's boot table will also be tried. order= is an old style. Use bootindex instead. The exact request should be only boot from selected devices. You described the purpose clearly first time. This is how I understood it :) I agree to make it configurable. eg: qemu -boot order=nd,strict=on,menu=on strick: on (only boot from selected devices) strick: off (will try to boot from all devices in seabios' boot table) default strick should be 'off' as current behavior. Yes, this is my suggestion. Thanks, Amos seabios to boot only from selected devices. If user choose first boot device from menu, then seabios will try all the devices, even some of them are not selected. Signed-off-by: Amos Kong ak...@redhat.com --- Resend for CCing seabios maillist. --- src/boot.c | 13 - 1 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/boot.c b/src/boot.c index 3ca7960..ee810ac 100644 --- a/src/boot.c +++ b/src/boot.c @@ -424,6 +424,10 @@ interactive_bootmenu(void) maxmenu++; printf(%d. %s\n, maxmenu , strtcpy(desc, pos-description, ARRAY_SIZE(desc))); +/* If user chooses first boot device from menu, we will treat + all the devices as selected. */ +if (pos-priority == DEFAULT_PRIO) +pos-priority = DEFAULT_PRIO - 1; pos = pos-next; } @@ -490,7 +494,10 @@ boot_prep(void) // Map drives and populate BEV list struct bootentry_s *pos = BootList; -while (pos) { + +/* The priority of un-selected device is not changed, + we only boot from user selected devices. */ +while (pos pos-priority != DEFAULT_PRIO) { switch (pos-type) { case IPL_TYPE_BCV: call_bcv(pos-vector.seg, pos-vector.offset); @@ -513,10 +520,6 @@ boot_prep(void) } pos = pos-next; } - -// If nothing added a floppy/hd boot - add it manually. -add_bev(IPL_TYPE_FLOPPY, 0); -add_bev(IPL_TYPE_HARDDISK, 0); -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Resend][Seabios PATCH] don't boot from un-selected devices
On Wed, Dec 19, 2012 at 03:24:45PM +0800, Amos Kong wrote: Current seabios will try to boot from selected devices first, if they are all failed, seabios will also try to boot from un-selected devices. For example: @ qemu-kvm -boot order=n,menu=on ... Guest will boot from network first, if it's failed, guest will try to boot from other un-selected devices (floppy, cdrom, disk) one by one. Sometimes, user don't want to boot from some devices. This patch changes And sometimes he want. The patch changes behaviour unconditionally. New behaviour should be user selectable. Something line -boot order=strict on qemu command line. seabios to boot only from selected devices. If user choose first boot device from menu, then seabios will try all the devices, even some of them are not selected. Signed-off-by: Amos Kong ak...@redhat.com --- Resend for CCing seabios maillist. --- src/boot.c | 13 - 1 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/boot.c b/src/boot.c index 3ca7960..ee810ac 100644 --- a/src/boot.c +++ b/src/boot.c @@ -424,6 +424,10 @@ interactive_bootmenu(void) maxmenu++; printf(%d. %s\n, maxmenu , strtcpy(desc, pos-description, ARRAY_SIZE(desc))); +/* If user chooses first boot device from menu, we will treat + all the devices as selected. */ +if (pos-priority == DEFAULT_PRIO) +pos-priority = DEFAULT_PRIO - 1; pos = pos-next; } @@ -490,7 +494,10 @@ boot_prep(void) // Map drives and populate BEV list struct bootentry_s *pos = BootList; -while (pos) { + +/* The priority of un-selected device is not changed, + we only boot from user selected devices. */ +while (pos pos-priority != DEFAULT_PRIO) { switch (pos-type) { case IPL_TYPE_BCV: call_bcv(pos-vector.seg, pos-vector.offset); @@ -513,10 +520,6 @@ boot_prep(void) } pos = pos-next; } - -// If nothing added a floppy/hd boot - add it manually. -add_bev(IPL_TYPE_FLOPPY, 0); -add_bev(IPL_TYPE_HARDDISK, 0); } -- 1.7.1 -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] acpi: fix irq 8 ressource conflict
On Wed, Dec 05, 2012 at 05:28:31PM -0500, Kevin O'Connor wrote: CCing Alex. On Wed, Dec 05, 2012 at 01:18:48PM -0500, Jason Baron wrote: On Wed, Dec 05, 2012 at 10:13:57AM +0100, Gerd Hoffmann wrote: Both hpet and rtc have irq 8 in their ressources, making windows unhappy because of the conflict. Fix this by making rtc check whenever the hpet is present, and claim irq 8 only in case it isn't. So rtc gets irq 8 only in case you start qemu with the -no-hpet switch. Verified working in both linux (check pnp boot messages) and windows (check rtc ressources in device manager) guests. Resolves the Windows 7 RTC resource conflict on Q35 as well. However, Windows XP continues to blue screen on boot as I reported, and Brad also posted (for both piix and q35). Looks like a regression caused by d9f5cdbdf55d61aef9a1a534d9123ef734427478 - I'll revert that patch if no fix is found. If it was copied from real HW can we check that XP runs on this HW? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH v3] DSDT: Fix HPET _CRS Method
On Mon, Nov 19, 2012 at 12:35:47PM +0100, Alexander Graf wrote: On 19.11.2012, at 12:33, Gleb Natapov wrote: On Mon, Nov 19, 2012 at 11:22:48AM +0100, Alexander Graf wrote: On 16.11.2012, at 19:46, Kevin O'Connor wrote: On Fri, Nov 16, 2012 at 01:02:18PM -0500, Gabriel L. Somlo wrote: ping On Thu, Nov 08, 2012 at 12:35:17PM -0500, Gabriel L. Somlo wrote: Updated _CRS method for HPET, bringing it in line with the way it is presented on recent hardware (e.g. Dell Latitude D630, MacPro5,1, etc); Allows it to be detected and utilized from Mac OS X; Also tested OK on Linux (F16 64-bit install DVD) and Windows (Win7 64-bit install DVD). Signed-off-by: Gabriel Somlo so...@cmu.edu I'm okay with the patch. I'm looking to see an Ack from one of the kvm/qemu developers. (I saw Gerd was okay with the last version of the patch.) IIRC Marcelo was the one changing the DSDT code from what this patch produces to what we have today. My initial HPET DSDT entry was also copied from real hardware. Marcelo, any memory on this? :) Keep in mind I might be misremembering - maybe it was someone else after all ;). I think you are misremembering. Git shows that HPET entry was taken from pcbios code and my old pcbios git tree says that the dsdt code was contributed by Beth Kon and it is the same as we have it now in SeaBIOS. Hrm. I'm 100% sure that from my code (which Beth based on) to the code that we have, someone had completely rewritten the DSDT entry. Either way, not complaining as long as we get it sorted out eventually :). Here is Beth's commit in bochs :) http://0x0badc0.de/gitweb?p=bochs/.git;a=commitdiff;h=874040e817e49811c30405d7c5d14b4c42161ca1;hp=1d2bac352688c75e8fdf8010f012b8ec85ba68d1 I added _STA logic in SeaBIOS but otherwise the code is the same. The patch looks OK. Mind to make this a formal acked-by? :) Sure, will do. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v3] DSDT: Fix HPET _CRS Method
On Thu, Nov 08, 2012 at 12:35:17PM -0500, Gabriel L. Somlo wrote: Updated _CRS method for HPET, bringing it in line with the way it is presented on recent hardware (e.g. Dell Latitude D630, MacPro5,1, etc); Allows it to be detected and utilized from Mac OS X; Also tested OK on Linux (F16 64-bit install DVD) and Windows (Win7 64-bit install DVD). Signed-off-by: Gabriel Somlo so...@cmu.edu Acked-by: Gleb Natapov g...@redhat.com --- On Thu, Nov 08, 2012 at 04:12:07PM +0100, Gerd Hoffmann wrote: Looks good to me now. One more thing, the IRQNoFlags line (also present on the real hardware I used as examples) makes it possible to start OS X with multiple virtual CPUs in qemu (e.g. -smp 4,cores=2). Re-ran tests on Fedora16 and Windows7 successfully (also with smp enabled). Thanks, --Gabriel src/acpi-dsdt.dsl | 14 +- 1 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index 284d70d..711302e 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -269,15 +269,11 @@ DefinitionBlock ( Return (0x0F) } Name(_CRS, ResourceTemplate() { -DWordMemory( -ResourceConsumer, PosDecode, MinFixed, MaxFixed, -NonCacheable, ReadWrite, -0x, -0xFED0, -0xFED003FF, -0x, -0x0400 /* 1K memory: FED0 - FED003FF */ -) +IRQNoFlags () {2, 8} +Memory32Fixed (ReadOnly, +0xFED0, // Address Base +0x0400, // Address Length +) }) } } -- 1.7.7.6 ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v2 9/9] seabios: Build the dsdt separately
On Tue, Oct 09, 2012 at 08:04:12PM -0400, Kevin O'Connor wrote: On Mon, Oct 08, 2012 at 11:35:15PM -0400, Jason Baron wrote: From: Jason Baron jba...@redhat.com This builds seabios such that the dsdt tables are no longer built into the seabios binary. They must be passed to the seabios via fw_cfg. This saves space in the seabios binary for unnecessary dsdt tables. I suspect that this will make other users of Seabios, besides qemu unhappy, but I'm not sure. Only qemu/kvm uses the acpi support in seabios. (If one excludes bochs which hasn't really used seabios.) Coreboot and Xen both deploy acpi completely separately from seabios. Instead of moving just the dsdt to qemu, though, can we move all acpi tables into qemu? Moving just the dsdt can lead to conflicts with the generated ssdt code and potentially some of the other acpi tables. The only seabios acpi dependency that pops into mind is the memory addresses of the acpi tables themselves. It should be trivial to have seabios create the rsdt/rsdp while pulling all the remaining acpi tables from qemu via fw_cfg. Almost all the tables in acpi describe hardware and not the firmware. Are there other cases where the firmware needs to have input when generating the contents of an acpi table? There is a BDAT OperationRegion that points to memory allocated by Seabios. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH v2] add a boot parameter to set reboot timeout
On Thu, Sep 20, 2012 at 11:15:26AM +0800, Amos Kong wrote: On 07/09/12 11:11, Amos Kong wrote: Added an option to let qemu transfer a configuration file to bios, etc/boot-fail-wait, which could be specified by command -boot reboot-timeout=T T have a max value of 0x, unit is ms. With this option, guest will wait for a given time if not find bootabled device, then reboot. If reboot-timeout is '-1', guest will not reboot, qemu passes '-1' to bios by default. This feature need the new seabios's support. Seabios pulls the value from the fwcfg file interface, this interface is used because SeaBIOS needs a reliable way of obtaining a name, value size, and value. It in no way requires that there be a real file on the user's host machine. Signed-off-by: Amos Kong ak...@redhat.com --- v2: qemu passes '-1' to bios, guest will not reboot Gleb, Anthony, any comments? Not from me. Looks good. --- hw/fw_cfg.c | 25 + qemu-config.c |3 +++ qemu-options.hx | 12 +--- vl.c|3 ++- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c index 7b3b576..dcde1a9 100644 --- a/hw/fw_cfg.c +++ b/hw/fw_cfg.c @@ -183,6 +183,30 @@ static void fw_cfg_bootsplash(FWCfgState *s) } } +static void fw_cfg_reboot(FWCfgState *s) +{ +int reboot_timeout = -1; +char *p; +const char *temp; + +/* get user configuration */ +QemuOptsList *plist = qemu_find_opts(boot-opts); +QemuOpts *opts = QTAILQ_FIRST(plist-head); +if (opts != NULL) { +temp = qemu_opt_get(opts, reboot-timeout); +if (temp != NULL) { +p = (char *)temp; +reboot_timeout = strtol(p, (char **)p, 10); +} +} +/* validate the input */ +if (reboot_timeout 0x) { +error_report(reboot timeout is larger than 65535, force it to 65535.); +reboot_timeout = 0x; +} +fw_cfg_add_file(s, etc/boot-fail-wait, g_memdup(reboot_timeout, 4), 4); +} + static void fw_cfg_write(FWCfgState *s, uint8_t value) { int arch = !!(s-cur_entry FW_CFG_ARCH_LOCAL); @@ -497,6 +521,7 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port, fw_cfg_add_i16(s, FW_CFG_MAX_CPUS, (uint16_t)max_cpus); fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu); fw_cfg_bootsplash(s); +fw_cfg_reboot(s); s-machine_ready.notify = fw_cfg_machine_ready; qemu_add_machine_init_done_notifier(s-machine_ready); diff --git a/qemu-config.c b/qemu-config.c index c05ffbc..b9f9e0f 100644 --- a/qemu-config.c +++ b/qemu-config.c @@ -638,6 +638,9 @@ QemuOptsList qemu_boot_opts = { }, { .name = splash-time, .type = QEMU_OPT_STRING, +}, { +.name = reboot-timeout, +.type = QEMU_OPT_STRING, }, { /*End of list */ } }, diff --git a/qemu-options.hx b/qemu-options.hx index 3c411c4..0249a60 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -339,13 +339,14 @@ ETEXI DEF(boot, HAS_ARG, QEMU_OPTION_boot, -boot [order=drives][,once=drives][,menu=on|off]\n - [,splash=sp_name][,splash-time=sp_time]\n + [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time]\n 'drives': floppy (a), hard disk (c), CD-ROM (d), network (n)\n 'sp_name': the file's name that would be passed to bios as logo picture, if menu=on\n -'sp_time': the period that splash picture last if menu=on, unit is ms\n, +'sp_time': the period that splash picture last if menu=on, unit is ms\n +'rb_timeout': the timeout before guest reboot when boot failed, unit is ms\n, QEMU_ARCH_ALL) STEXI -@item -boot [order=@var{drives}][,once=@var{drives}][,menu=on|off][,splash=@var{sp_name}][,splash-time=@var{sp_time}] +@item -boot [order=@var{drives}][,once=@var{drives}][,menu=on|off][,splash=@var{sp_name}][,splash-time=@var{sp_time}][,reboot-timeout=@var{rb_timeout}] @findex -boot Specify boot order @var{drives} as a string of drive letters. Valid drive letters depend on the target achitecture. The x86 PC uses: a, b @@ -364,6 +365,11 @@ limitation: The splash file could be a jpeg file or a BMP file in 24 BPP format(true color). The resolution should be supported by the SVGA mode, so the recommended is 320x240, 640x480, 800x640. +A timeout could be passed to bios, guest will pause for @var{rb_timeout} ms +when boot failed, then reboot. If @var{rb_timeout} is '-1', guest will not +reboot, qemu passes '-1' to bios by default. Currently Seabios for X86 +system support it. + @example # try to boot from network first, then from hard disk qemu-system-i386 -boot order=nc diff --git a/vl.c b/vl.c index 7c577fa..1bd9931
Re: [SeaBIOS] [PATCH v2 0/2] Add IPMI SMBIOS/ACPI support
On Sun, Aug 12, 2012 at 08:22:12PM -0500, Corey Minyard wrote: Patch 2 is complex and I don't fully understand what it is doing. A quick scan leads me to believe it is constructing a dynamic SSDT - though it's not clear why a dynamic SSDT is needed and why the existing mechanism (see build_ssdt()) for generating dynamic SSDTs is not used. It is constructing an addition to the DSDT table that is tacked on to the end of that table if IPMI is present. It is complex, but building ACPI namespace data is complex, and the data is not fixed length. You do not need to construct IPMI device dynamically in DSDT. Write it in AML and have _STA method that tells OSPM if device is present or not. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
On Mon, Aug 13, 2012 at 12:37:11PM +0200, Gerd Hoffmann wrote: Hi, Isnt pmtimer ioport usable? 14MHz. Can give it a try. 14 MHz looks wrong though, apci.h says: /* PM Timer ticks per second (HZ) */ #define PM_TIMER_FREQUENCY 3579545 Is this fixed? Or hardware specific? 3.579545 MHz clock required by ACPI spec. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v2 0/2] Add IPMI SMBIOS/ACPI support
On Mon, Aug 13, 2012 at 09:47:50AM -0500, Corey Minyard wrote: On 08/13/2012 01:25 AM, Gleb Natapov wrote: On Sun, Aug 12, 2012 at 08:22:12PM -0500, Corey Minyard wrote: Patch 2 is complex and I don't fully understand what it is doing. A quick scan leads me to believe it is constructing a dynamic SSDT - though it's not clear why a dynamic SSDT is needed and why the existing mechanism (see build_ssdt()) for generating dynamic SSDTs is not used. It is constructing an addition to the DSDT table that is tacked on to the end of that table if IPMI is present. It is complex, but building ACPI namespace data is complex, and the data is not fixed length. You do not need to construct IPMI device dynamically in DSDT. Write it in AML and have _STA method that tells OSPM if device is present or not. There are lots of different options for IPMI devices. There are three different interface types, with two string lengths. They can all appear at arbitrary places in I/O or memory space. They can have an interrupt or not. I would like to be able to represent all off the possibilities so users can simulate any arbitrary machine they want. I considered writing it in AML 8 times and figuring the offsets to set the various values, but that seems rather messy to me. How different are they. Can you give human readable example? If the real desire is to have a single IPMI device type at a single address with a single interrupt always on, we could do that. The BIOS would still need a way to know that the device was present or not, so something will have to be passed. I'm not sure that reading from the standard address to detect the device is reliable enough, but that could be done, too. -corey -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
On Fri, Aug 10, 2012 at 10:18:00AM +0300, Gleb Natapov wrote: can fix the in-kernel PIT issues with GRUB (see Michaels message) while testing. What message exactly? found it. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Compiling SeaBIOS for coreboot has problems with its ACPI code
On Tue, Aug 07, 2012 at 07:34:37PM +, Moore, Robert wrote: This is very interesting. If I understand correctly, you are using a utility plus various directives to generate tables of AML offsets -- presumably in order to dynamically change AML values, correct? Yes. I have to say that I have not seen anything like this, from any BIOS vendor. Physical HW is not as dynamic (if at all) as virtualized one. We do not want to have separate AML table for each possible HW configuration user can create while starting QEMU. By the way, is there interest in adding some of the functionality that we get by parsing the listing to iasl directly? We are always interested in adding features to make the compiler more useful. What would you suggest? The tool depends on the compiler output and thus fragile. It broke with new compiler version. It would be nice if compiler had a way to generate offsets for us. Bob -Original Message- From: Michael S. Tsirkin [mailto:m...@redhat.com] Sent: Sunday, August 05, 2012 1:45 PM To: Moore, Robert Cc: Idwer Vollering; Kevin O'Connor; seabios@seabios.org; Tang, Feng Subject: Re: [SeaBIOS] Compiling SeaBIOS for coreboot has problems with its ACPI code On Sun, Aug 05, 2012 at 11:36:15PM +0300, Michael S. Tsirkin wrote: On Mon, Jul 30, 2012 at 07:42:48PM +, Moore, Robert wrote: Yes, you are correct, the listing no longer includes the comments. Sorry for causing you a problem. What is happening is that the preprocessor parser is stripping the comments during the creation of the .i file. Then, the compiler is invoked on the .i file -- thus, the comments are gone. This is going to take a bit of work to correct, but we will do it. In the meantime, try using the -Pn flag to disable the preprocessor. When this flag is set, the preprocessor is completely bypassed and the compiler should function as it did previously. So we are doing it this way meanwhile. If you change this preprocessor behaviour like you indicated you would, please let us know so e can test. Thanks! By the way, is there interest in adding some of the functionality that we get by parsing the listing to iasl directly? Here's what our tool currently supports: # Process mixed ASL/AML listing (.lst file) produced by iasl -l # Locate and execute ACPI_EXTRACT directives, output offset info # # Documentation of ACPI_EXTRACT_* directive tags: # # These directive tags output offset information from AML for BIOS runtime # table generation. # Each directive is of the form: # ACPI_EXTRACT_TYPE array_name Operator (...) # and causes the extractor to create an array # named array_name with offset, in the generated AML, # of an object of a given type in the following Operator. # # A directive must fit on a single code line. # # Object type in AML is verified, a mismatch causes a build failure. # # Directives and operators currently supported are: # ACPI_EXTRACT_NAME_DWORD_CONST - extract a Dword Const object from Name() # ACPI_EXTRACT_NAME_WORD_CONST - extract a Word Const object from Name() # ACPI_EXTRACT_NAME_BYTE_CONST - extract a Byte Const object from Name() # ACPI_EXTRACT_METHOD_STRING - extract a NameString from Method() # ACPI_EXTRACT_NAME_STRING - extract a NameString from Name() # ACPI_EXTRACT_PROCESSOR_START - start of Processor() block # ACPI_EXTRACT_PROCESSOR_STRING - extract a NameString from Processor() # ACPI_EXTRACT_PROCESSOR_END - offset at last byte of Processor() + 1 # ACPI_EXTRACT_PKG_START - start of Package block # # ACPI_EXTRACT_ALL_CODE - create an array storing the generated AML bytecode # # ACPI_EXTRACT is not allowed anywhere else in code, except in comments. Example: ACPI_EXTRACT_NAME_DWORD_CONST aml_adr_dword Name (_ADR, 0x0001) adds the offset of 0x0001 constant to array aml_adr_dword Example: ACPI_EXTRACT_METHOD_STRING aml_ej0_name Method (_EJ0, 1) { Return(PCEJ(0x0001)) } adds the offset of _EJ0 string to array aml_ej0_name ACPI_EXTRACT_ALL_CODE ssdp_pcihp_aml names the array to include the generated AML code -- MST ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] acpi: LNKS is not needed
On Tue, Aug 07, 2012 at 03:01:29PM +0200, Paolo Bonzini wrote: LNKS is a bit strange in that it reuses the same PIIX register as LNKA, but has a different interrupt. This means that the _CRS it returns will not be one of the possible resources from _PRS. This shows up in the Linux boot logs as ACPI: PCI Interrupt Link [LNKS] (IRQs 9) *0 Instead of that, we can simply use a hardwired interrupt index. Cc: Gleb Natapov gnata...@redhat.com Cc: Laszlo Ersek ler...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com Looks good to me. --- src/acpi-dsdt.dsl | 16 1 file modificato, 4 inserzioni(+), 12 rimozioni(-) diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index 083e2be..66ca853 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -86,7 +86,10 @@ DefinitionBlock ( #define prt_slot3(nr) prt_slot(nr, LNKC, LNKD, LNKA, LNKB) prt_slot0(0x), /* Device 1 is power mgmt device, and can only use irq 9 */ - prt_slot(0x0001, LNKS, LNKB, LNKC, LNKD), + Package() { 0x1, 0,0, 9 }, + Package() { 0x1, 1, LNKB, 0 }, + Package() { 0x1, 2, LNKC, 0 }, + Package() { 0x1, 3, LNKD, 0 } prt_slot2(0x0002), prt_slot3(0x0003), prt_slot0(0x0004), @@ -653,17 +656,6 @@ DefinitionBlock ( Method (_CRS, 0, NotSerialized) { Return (IQCR(PRQ3)) } Method (_SRS, 1, NotSerialized) { SETIRQ(PRQ3, Arg0) } } -Device(LNKS) { -Name(_HID, EISAID(PNP0C0F)) // PCI interrupt link -Name(_UID, 5) -Name(_PRS, ResourceTemplate() { -Interrupt (, Level, ActiveHigh, Shared) -{ 9 } -}) -Method (_STA, 0, NotSerialized) { Return (IQST(PRQ0)) } -Method (_DIS, 0, NotSerialized) { DISIRQ(PRQ0) } -Method (_CRS, 0, NotSerialized) { Return (IQCR(PRQ0)) } -} } / -- 1.7.11.2 -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [SeaBIOS PATCH 0/2] Allow non-contiguous APIC IDs (v3)
On Tue, Jul 31, 2012 at 09:14:13PM -0400, Kevin O'Connor wrote: On Wed, Jul 25, 2012 at 03:45:28PM -0300, Eduardo Habkost wrote: Changes v2 - v3: - Report I/O APIC ID = 0 on MP-table, too Changes v1 - v2: - Patch 1/2: cosmetic whitespace change - Patch 2/2: use size suffixes on asm instructions on smp.c - New patch descriptions Eduardo Habkost (2): report real I/O APIC ID (0) on MADT and MP-table (v3) allow CPUs to have non-contiguous Local APIC IDs (v2) The series looks okay to me. However, I think Gleb and Avi had some comments - has that been resolved now? No, but the comment is very minor and can be resolved by follow up patch if ever. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] Add an IPMI SMBIOS entry
On Mon, Jul 30, 2012 at 09:57:47PM -0500, miny...@acm.org wrote: From: Corey Minyard cminy...@mvista.com An IPMI device is being added to the qemu code, and it has an SMBIOS entry to describe the interface characteristics. So add the SMBIOS entry to the BIOS so it can handle this. Signed-off-by: Corey Minyard cminy...@mvista.com --- Makefile |2 +- src/ipmi.c | 16 src/ipmi.h | 27 +++ src/paravirt.c | 18 ++ src/paravirt.h |9 + src/post.c |2 ++ src/smbios.c | 54 ++ src/smbios.h | 12 src/util.h |5 + 9 files changed, 144 insertions(+), 1 deletions(-) create mode 100644 src/ipmi.c create mode 100644 src/ipmi.h diff --git a/Makefile b/Makefile index dfdec5c..1b6eb96 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ SRCBOTH=misc.c stacks.c pmm.c output.c util.c block.c floppy.c ata.c mouse.c \ pnpbios.c pirtable.c vgahooks.c ramdisk.c pcibios.c blockcmd.c \ usb.c usb-uhci.c usb-ohci.c usb-ehci.c usb-hid.c usb-msc.c \ virtio-ring.c virtio-pci.c virtio-blk.c virtio-scsi.c apm.c ahci.c \ -usb-uas.c lsi-scsi.c +usb-uas.c lsi-scsi.c ipmi.c SRC16=$(SRCBOTH) system.c disk.c font.c SRC32FLAT=$(SRCBOTH) post.c shadow.c memmap.c coreboot.c boot.c \ acpi.c smm.c mptable.c smbios.c pciinit.c optionroms.c mtrr.c \ diff --git a/src/ipmi.c b/src/ipmi.c new file mode 100644 index 000..7521a8b --- /dev/null +++ b/src/ipmi.c @@ -0,0 +1,16 @@ +// IPMI setup information +// +// Copyright (C) 2012 Corey Minyard cminy...@mvista.com +// +// This file may be distributed under the terms of the GNU LGPLv3 license. + +#include ipmi.h +#include paravirt.h + +struct ipmi_info ipmi_info; + +void +ipmi_setup(void) +{ +qemu_cfg_load_ipmi(ipmi_info); +} diff --git a/src/ipmi.h b/src/ipmi.h new file mode 100644 index 000..b0cf61e --- /dev/null +++ b/src/ipmi.h @@ -0,0 +1,27 @@ +// IPMI setup information +// +// Copyright (C) 2012 Corey Minyard cminy...@mvista.com +// +// This file may be distributed under the terms of the GNU LGPLv3 license. + +#ifndef __IPMI_H +#define __IPMI_H + +#include config.h // CONFIG_COREBOOT +#include types.h + +struct ipmi_info { +u64 base_addr; +u8 interface; +u8 reg_space; +u8 reg_spacing; +u8 slave_addr; +u8 irq; +u8 version; +}; + +extern struct ipmi_info ipmi_info; + +void ipmi_setup(void); + +#endif diff --git a/src/paravirt.c b/src/paravirt.c index 2a98d53..4986cfb 100644 --- a/src/paravirt.c +++ b/src/paravirt.c @@ -148,6 +148,24 @@ void* qemu_cfg_e820_load_next(void *addr) return addr; } +void qemu_cfg_load_ipmi(struct ipmi_info *info) +{ +qemu_cfg_read_entry(info-interface, QEMU_CFG_IPMI_INTERFACE, sizeof(u8)); +qemu_cfg_read_entry(info-base_addr, QEMU_CFG_IPMI_BASE_ADDR, sizeof(u64)); + +if ((info-interface == 0) || (info-base_addr == 0)) +return; + +info-base_addr = le64_to_cpu(info-base_addr); +qemu_cfg_read_entry(info-reg_spacing, QEMU_CFG_IPMI_REG_SPACING, +sizeof(u8)); +qemu_cfg_read_entry(info-reg_space, QEMU_CFG_IPMI_REG_SPACE, sizeof(u8)); +qemu_cfg_read_entry(info-version, QEMU_CFG_IPMI_VERSION, sizeof(u8)); +qemu_cfg_read_entry(info-slave_addr, QEMU_CFG_IPMI_SLAVE_ADDR, +sizeof(u8)); +qemu_cfg_read_entry(info-irq, QEMU_CFG_IPMI_IRQ, sizeof(u8)); +} + struct smbios_header { u16 length; u8 type; diff --git a/src/paravirt.h b/src/paravirt.h index a284c41..b14e41b 100644 --- a/src/paravirt.h +++ b/src/paravirt.h @@ -3,6 +3,7 @@ #include config.h // CONFIG_COREBOOT #include util.h +#include ipmi.h // struct ipmi_info /* This CPUID returns the signature 'KVMKVMKVM' in ebx, ecx, and edx. It * should be used to determine that a VM is running under KVM. @@ -35,6 +36,13 @@ static inline int kvm_para_available(void) #define QEMU_CFG_BOOT_MENU 0x0e #define QEMU_CFG_MAX_CPUS 0x0f #define QEMU_CFG_FILE_DIR 0x19 +#define QEMU_CFG_IPMI_INTERFACE 0x30 +#define QEMU_CFG_IPMI_BASE_ADDR 0x31 +#define QEMU_CFG_IPMI_REG_SPACE 0x32 +#define QEMU_CFG_IPMI_REG_SPACING 0x33 +#define QEMU_CFG_IPMI_SLAVE_ADDR0x34 +#define QEMU_CFG_IPMI_IRQ 0x35 +#define QEMU_CFG_IPMI_VERSION 0x36 Better to add _one_ fw_cfg entry that contains all of the info. But I am sure Kevin will want this to be passed through file interface anyway. #define QEMU_CFG_ARCH_LOCAL 0x8000 #define QEMU_CFG_ACPI_TABLES(QEMU_CFG_ARCH_LOCAL + 0) #define QEMU_CFG_SMBIOS_ENTRIES (QEMU_CFG_ARCH_LOCAL + 1) @@ -64,6 +72,7 @@ struct e820_reservation { }; u32
Re: [SeaBIOS] [PATCH] Add an IPMI SMBIOS entry
On Tue, Jul 31, 2012 at 08:33:53AM -0500, Corey Minyard wrote: On 07/31/2012 02:10 AM, Gleb Natapov wrote: /* This CPUID returns the signature 'KVMKVMKVM' in ebx, ecx, and edx. It * should be used to determine that a VM is running under KVM. @@ -35,6 +36,13 @@ static inline int kvm_para_available(void) #define QEMU_CFG_BOOT_MENU 0x0e #define QEMU_CFG_MAX_CPUS 0x0f #define QEMU_CFG_FILE_DIR 0x19 +#define QEMU_CFG_IPMI_INTERFACE 0x30 +#define QEMU_CFG_IPMI_BASE_ADDR 0x31 +#define QEMU_CFG_IPMI_REG_SPACE 0x32 +#define QEMU_CFG_IPMI_REG_SPACING 0x33 +#define QEMU_CFG_IPMI_SLAVE_ADDR0x34 +#define QEMU_CFG_IPMI_IRQ 0x35 +#define QEMU_CFG_IPMI_VERSION 0x36 Better to add _one_ fw_cfg entry that contains all of the info. But I am sure Kevin will want this to be passed through file interface anyway. I considered that, but that seems like a more brittle interface. New fields may need to be added in the future. I'll look at the file interface, but I would think it would be best to have something that could be added to. If you think it will have to be extended version it or include feature bitmap. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [SeaBIOS PATCH 2/2] allow CPUs to have non-contiguous Local APIC IDs (v2)
On Wed, Jul 25, 2012 at 03:42:21PM -0300, Eduardo Habkost wrote: On Mon, Jul 23, 2012 at 03:20:14PM +0300, Gleb Natapov wrote: On Fri, Jul 20, 2012 at 02:04:50PM -0300, Eduardo Habkost wrote: Extract Local APIC IDs directly from the CPUs, and instead of check for i CountCPUs, check if the APIC ID was present on boot, when building ACPI tables and the MP-Table. This keeps ACPI Processor ID == APIC ID, but allows the hardware-SeaBIOS interface be completely APIC-ID based and not depend on any other kind of CPU identifier. This way, SeaBIOS may change the way ACPI Processor IDs are chosen in the future. As currently SeaBIOS supports only xAPIC and not x2APIC, the list of present-on-boot APIC IDs is a 256-bit bitmap. If one day SeaBIOS starts to support x2APIC, the data structure used to enumerate the APIC IDs will have to be changed (but this is an internal implementation detail, not visible to the OS or on any hardware=SeaBIOS interface). For current QEMU versions (that always make the APIC IDs contiguous), the OS-visible behavior and resulting ACPI tables should be exactly the same. This patch will simply allow QEMU to start setting non-contiguous APIC IDs (that is a requirement for some sockets/cores/threads topology settings). Changes v1 - v2: - Use size suffixes on all asm instructions on smp.c - New patch description Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- src/acpi-dsdt.dsl |4 +++- src/acpi.c|9 + src/mptable.c |2 +- src/smp.c | 17 + src/util.h|1 + 5 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index 2060686..72dc7d8 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -676,6 +676,7 @@ DefinitionBlock ( /* Methods called by run-time generated SSDT Processor objects */ Method (CPMA, 1, NotSerialized) { // _MAT method - create an madt apic buffer +// Arg0 = Processor ID = Local APIC ID // Local0 = CPON flag for this cpu Store(DerefOf(Index(CPON, Arg0)), Local0) // Local1 = Buffer (in madt apic form) to return @@ -688,6 +689,7 @@ DefinitionBlock ( } Method (CPST, 1, NotSerialized) { // _STA method - return ON status of cpu +// Arg0 = Processor ID = Local APIC ID // Local0 = CPON flag for this cpu Store(DerefOf(Index(CPON, Arg0)), Local0) If (Local0) { Return(0xF) } Else { Return(0x0) } @@ -708,7 +710,7 @@ DefinitionBlock ( Store (PRS, Local5) // Local2 = last read byte from bitmap Store (Zero, Local2) -// Local0 = cpuid iterator +// Local0 = Processor ID / APIC ID iterator Store (Zero, Local0) While (LLess(Local0, SizeOf(CPON))) { // Local1 = CPON flag for this cpu diff --git a/src/acpi.c b/src/acpi.c index da3bc57..39b7172 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -327,7 +327,7 @@ build_madt(void) apic-length = sizeof(*apic); apic-processor_id = i; apic-local_apic_id = i; -if (i CountCPUs) +if (apic_id_is_present(apic-local_apic_id)) apic-flags = cpu_to_le32(1); else apic-flags = cpu_to_le32(0); @@ -445,6 +445,7 @@ build_ssdt(void) } // build Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...} +// Arg0 = Processor ID = APIC ID *(ssdt_ptr++) = 0x14; // MethodOp ssdt_ptr = encodeLen(ssdt_ptr, 2+5+(12*acpi_cpus), 2); *(ssdt_ptr++) = 'N'; @@ -477,7 +478,7 @@ build_ssdt(void) ssdt_ptr = encodeLen(ssdt_ptr, 2+1+(1*acpi_cpus), 2); *(ssdt_ptr++) = acpi_cpus; for (i=0; iacpi_cpus; i++) -*(ssdt_ptr++) = (i CountCPUs) ? 0x01 : 0x00; +*(ssdt_ptr++) = (apic_id_is_present(i)) ? 0x01 : 0x00; // store pci io windows: start, end, length // this way we don't have to do the math in the dsdt @@ -656,10 +657,10 @@ build_srat(void) core-proximity_lo = curnode; memset(core-proximity_hi, 0, 3); core-local_sapic_eid = 0; -if (i CountCPUs) +if (apic_id_is_present(i)) core-flags = cpu_to_le32(1); else -core-flags = 0; +core-flags = cpu_to_le32(0); core++; } diff --git a/src/mptable.c b/src/mptable.c index 103f462..9406f98 100644 --- a/src/mptable.c +++ b/src/mptable.c @@ -59,7 +59,7 @@ mptable_init(void) cpu-apicid = i; cpu-apicver = apic_version; /* cpu flags: enabled
Re: [SeaBIOS] SeaBIOS, FW_CFG_NUMA, and FW_CFG_MAX_CPUS
On Fri, Jul 20, 2012 at 05:00:25PM -0300, Eduardo Habkost wrote: Hi, While working at the CPU index vs APIC ID changes, I stumbled upon another not-very-well-defined interface between SeaBIOS and QEMU, and I would like to clarify the semantics and constraints of some FW_CFG entries. First, the facts/assumptions: - There's no concept of CPU index or CPU identifier that SeaBIOS and QEMU agree upon, except for the APIC ID. All SeaBIOS can really see are the CPU APIC IDs, on boot or on CPU hotplug. - The APIC ID is already a perfectly good CPU identifier, that is present on bare metal hardware too. - Adding a new kind of CPU identifier in addition to the APIC ID would just make things more complex. - The only problem with APIC IDs is that they may not be contiguous. That said, I would like to clarify the meaning of: - FW_CFG_MAX_CPUS What are the basic semantics and expectations about FW_CFG_MAX_CPUS? Considering that the APIC IDs may not be contiguous, is it supposed to be: a) the maximum number of CPUs that will be ever online, doesn't matter their APIC IDs, or b) a value so that every CPU has APIC ID MAX_CPUS. A practical example: suppose we have a machine with 18 CPUs with the following APIC IDs: 0x00, 0x01, 0x02, 0x04, 0x05, 0x06, 0x08, 0x09, 0x0a, 0x10, 0x11, 0x12, 0x14, 0x15, 0x16, 0x18, 0x19, 0x1a. (That's the expected result for a machine with 2 sockets, 3 cores per socket, 3 threads per core.) In that case, should FW_CFG_MAX_CPUS be: a) 18, or b) 27 (0x1b)? If it should be 18, it will require additional work on SeaBIOS to make: - CPU hotplug work - SRAT/MADT/SSDT tables be built with Processor ID != APIC ID - SRAT/MADT/SSDT tables be kept stable if the system is hibernated and resumed after a CPU is hot-plugged. (Probably in that case I would suggest introducing a FW_CFG_MAX_APIC_ID entry, so that SeaBIOS can still build the ACPI tables more easily). - FW_CFG_NUMA The first problem with FW_CFG_NUMA is that it depends on FW_CFG_MAX_CPUS (so it inherits the same questions above). The second is that FW_CFG_NUMA is a CPU-based table, but there's nothing SeaBIOS can use to know what CPUs FW_CFG_NUMA is refering to, except for the APIC IDs. So, should FW_CFG_NUMA be indexed by APIC IDs? - My proposal: My proposal is to try to keep things simple, and just use the following rule: - Never have a CPU with APIC ID FW_CFG_MAX_CPUS. This way: - The SeaBIOS ACPI code can be kept simple. - The current CPU hotplug interface can work as-is (up to 256 CPUs), based on APIC IDs. - The current FW_CFG_NUMA interface can work as-is, indexed by APIC IDs. - The ACPI tables can be easily kept stable between hibernate and resume, after CPU hotplug. This is the direction I am trying to go, and I am sending this just to make sure nobody is against it, and to not surprise anybody when I send a QEMU patch to make FW_CFG_MAX_CPUS be based on APIC IDs. This shouldn't change the meaning of maxcpus on command line though. Qemu can calculate max ACPI ID needed to support maxcpus by itself. My second proposal would be to introduce a FW_CFG_MAX_APIC_ID entry, so the SeaBIOS ACPI code can be kept simple. My third proposal would be to introduce a FW_CFG CPU Index = APIC ID table, but I really wouldn't like to introduce a new type of CPU identifier to be used between QEMU and SeaBIOS, when the APIC ID is a perfectly good unique CPU identifier that already exists in bare metal hardware. -- Eduardo -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [seabios PATCH 1/2] acpi: set I/O APIC ID to 0 by default
On Fri, Jul 20, 2012 at 01:22:43PM -0300, Eduardo Habkost wrote: On Fri, Jul 20, 2012 at 12:18:59AM +0300, Gleb Natapov wrote: On Thu, Jul 19, 2012 at 05:52:41PM -0300, Eduardo Habkost wrote: When resetting an I/O APIC, its ID is set to 0, so set it to 0 on the MADT table too. Actually BIOS needs to configure ioapic id to a uniqe value. This does not really matter for KVM though. Where does this requirement comes from? I am guessing it matters only when the I/O APIC is directly connected to the APIC bus (according to Intel SDM, that's the case only for old Pentium and P6 CPUs)[1]. http://www.intel.com/design/chipsets/datashts/29056601.pdf says that it should be programmed to unique value. I checked 4 machines on 3 of them this was the case on one (AMD) there are 3 IOAPICs and they all overlap with APICs. Anyway, even if some hardware has this unique-ID requirement, today Seabios does not fulfill it, leaving the I/O APIC ID as 0. The patch at least makes the MADT table match reality. The currant code was written when we could only dream supporting 16 cpus and back than it did correct thing, but now it just broken. QEMU do not care about IOAPIC ID for sure so I am OK with the patch. [1] I have checked 3 different machines, and all machines I have looked have an I/O APIC ID that conflicts with an existing Local APIC ID, on the ACPI MADT table. Some iasl dumps may be found online by googling for: Subtable Type : 01 I/O Apic ID I looked at 5 or 6 matches, and almost every one have an I/O APIC ID conflicting with a Local APIC ID. 1 from 4 for me :) Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- src/acpi.c |2 +- src/config.h |2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/acpi.c b/src/acpi.c index 55e4607..3f55de9 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -335,7 +335,7 @@ build_madt(void) struct madt_io_apic *io_apic = (void*)apic; io_apic-type = APIC_IO; io_apic-length = sizeof(*io_apic); -io_apic-io_apic_id = CountCPUs; +io_apic-io_apic_id = BUILD_IOAPIC_ID; io_apic-address = cpu_to_le32(BUILD_IOAPIC_ADDR); io_apic-interrupt = cpu_to_le32(0); diff --git a/src/config.h b/src/config.h index 3a70867..878c691 100644 --- a/src/config.h +++ b/src/config.h @@ -52,9 +52,11 @@ #define BUILD_PCIMEM64_END0x100ULL #define BUILD_IOAPIC_ADDR 0xfec0 +#define BUILD_IOAPIC_ID 0 #define BUILD_HPET_ADDRESS0xfed0 #define BUILD_APIC_ADDR 0xfee0 + // Important real-mode segments #define SEG_IVT 0x #define SEG_BDA 0x0040 -- 1.7.10.4 -- Gleb. -- Eduardo -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH]: enable USE_PLATFORM_CLOCK bit in FADT flags
On Fri, Jul 20, 2012 at 07:24:27AM -0300, Marcelo Tosatti wrote: On Fri, Jul 20, 2012 at 07:49:03AM +0300, Gleb Natapov wrote: On Thu, Jul 19, 2012 at 08:44:07PM -0400, Kevin O'Connor wrote: On Tue, Jul 17, 2012 at 02:18:00PM -0300, Marcelo Tosatti wrote: Enable bit 15 (USE_PLATFORM_CLOCK) of FADT flags field so that older Windows guests do not make use of the TSC for timestamping. I fixed up and committed this patch. But after I committed it, I realized it reverted Gleb's Drop FIX_RTC flag from FADT patch (c7be281b). Gleb, if you can you confirm c7be281b is still valid I'll put it back in. It also reverts 20fcf9b. 20fcf9b = addition of RTC_S4 (bit 7). The patch i sent to add bit 15 does not touch that: -/* WBINVD + PROC_C1 + SLP_BUTTON + RTC_S4 */ -fadt-flags = cpu_to_le32((1 0) | (1 2) | (1 5) | (1 7)); +/* WBINVD + PROC_C1 + SLP_BUTTON + RTC_S4 + USE_PLATFORM_CLOCK */ +fadt-flags = cpu_to_le32((1 0) | (1 2) | (1 5) | (1 7) | + (1 15)); Actually the patch is: -/* WBINVD + PROC_C1 + SLP_BUTTON + FIX_RTC */ -fadt-flags = cpu_to_le32((1 0) | (1 2) | (1 5) | (1 6)); +/* WBINVD + PROC_C1 + SLP_BUTTON + FIX_RTC + USE_PLATFORM_CLOCK */ +fadt-flags = cpu_to_le32((1 0) | (1 2) | (1 5) | (1 6) | + (1 15)); Somehow (1 7) became (1 6). -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC seabios PATCH] enumerate APIC IDs directly from CPUs
On Tue, Jul 17, 2012 at 06:56:30PM -0300, Eduardo Habkost wrote: This patch is an attempt to fix the non-continguous-APIC-ID problem without the FW_CFG_LAPIC_INFO approach I have sent proposed last week. Basically, this changes Seabios to probe for APIC IDs directly from the CPUs on boot, instead of getting it using fw_cfg, store the found APIC IDs on a bitmap, and use that information whe building the MADT, SRAT, and SSDT ACPI tables. To do this properly, we have to decide the meaning of CPU IDs in the QEMU-Seabios interfaces, too. I see two possible approaches: 1) Have Seabios and QEMU agree on a a CPU identifier, that is independent from the APIC ID. 2) Always use the Initial APIC ID on all communication between QEMU and Seabios. We need to be prepared to support more than 255 cpus. With 255 cpus comes x2apic and x2apic has 32bit apic ids. HW does not have to support all of the bits though, but potentially all the bitmasks can grow prohibitedly large. This patch implements the second approach. That means: - QEMU won't know anything about the ACPI Processor IDs chosen by Seabios; - The NUMA CPU affinity table in QEMU_CFG_NUMA is APIC-ID-based, not Processor-ID-based; - The CPU hotplug bitmaps on I/O port 0xaf00 will be APIC-ID-based, too. Internally, the following changes were made on the ACPI code: - The CPON array is now APIC-ID-based - The NTFY method now gets the CPU APIC ID as parameter - The CPMA method now gets two parameters: the Processor ID and the APIC ID - The CPST method now gets the CPU APIC ID as parameter To try to make things less likely to break on the common, non-hotplug case, this patch makes the Processor IDs be chosen this way: - The CPUs present on boot get contiguous Processor IDs (even if the APIC IDs are not contiguous); - The remaining Processor declarations are going to associated to the remaining APIC IDs (immediately after the last present APIC ID), sequentially. This means that hotplugged CPUs may not get contiguous Processor declarations if their APIC IDs are not contiguous. I am curious what will happen if cpu will be hot plugged, than hibernate and resume is done. After resume hot plugged cpu will have different Processor ID in ACPI. This may or may not be a problem. Some comment below. I would like to know what others think about this approach. I think it is a much simpler approach than relying on some APIC ID = CPU ID translation to make Seabios and QEMU agree. This way, Seabios has absolute freedom to choose the ACPI Processor IDs. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- src/acpi-dsdt.dsl | 11 -- src/acpi.c| 109 - src/smp.c | 17 + src/ssdt-proc.dsl | 12 -- src/util.h|3 ++ 5 files changed, 127 insertions(+), 25 deletions(-) diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index 2060686..51ca037 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -674,20 +674,23 @@ DefinitionBlock ( External(CPON, PkgObj) /* Methods called by run-time generated SSDT Processor objects */ -Method (CPMA, 1, NotSerialized) { +Method (CPMA, 2, NotSerialized) { // _MAT method - create an madt apic buffer +// Arg0 = Processor ID +// Arg1 = Local APIC ID // Local0 = CPON flag for this cpu -Store(DerefOf(Index(CPON, Arg0)), Local0) +Store(DerefOf(Index(CPON, Arg1)), Local0) // Local1 = Buffer (in madt apic form) to return Store(Buffer(8) {0x00, 0x08, 0x00, 0x00, 0x00, 0, 0, 0}, Local1) // Update the processor id, lapic id, and enable/disable status Store(Arg0, Index(Local1, 2)) -Store(Arg0, Index(Local1, 3)) +Store(Arg1, Index(Local1, 3)) Store(Local0, Index(Local1, 4)) Return (Local1) } Method (CPST, 1, NotSerialized) { // _STA method - return ON status of cpu +// Arg0 = Local APIC ID // Local0 = CPON flag for this cpu Store(DerefOf(Index(CPON, Arg0)), Local0) If (Local0) { Return(0xF) } Else { Return(0x0) } @@ -708,7 +711,7 @@ DefinitionBlock ( Store (PRS, Local5) // Local2 = last read byte from bitmap Store (Zero, Local2) -// Local0 = cpuid iterator +// Local0 = APIC ID iterator Store (Zero, Local0) While (LLess(Local0, SizeOf(CPON))) { // Local1 = CPON flag for this cpu diff --git a/src/acpi.c b/src/acpi.c index 55e4607..149ff42 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -302,6 +302,63 @@ build_fadt(struct pci_device *pci) return fadt; } + +/* APIC IDs of each Processor indexed by ACPI Processor ID. + * Used to
Re: [SeaBIOS] [seabios PATCH 1/2] acpi: set I/O APIC ID to 0 by default
On Thu, Jul 19, 2012 at 05:52:41PM -0300, Eduardo Habkost wrote: When resetting an I/O APIC, its ID is set to 0, so set it to 0 on the MADT table too. Actually BIOS needs to configure ioapic id to a uniqe value. This does not really matter for KVM though. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- src/acpi.c |2 +- src/config.h |2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/acpi.c b/src/acpi.c index 55e4607..3f55de9 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -335,7 +335,7 @@ build_madt(void) struct madt_io_apic *io_apic = (void*)apic; io_apic-type = APIC_IO; io_apic-length = sizeof(*io_apic); -io_apic-io_apic_id = CountCPUs; +io_apic-io_apic_id = BUILD_IOAPIC_ID; io_apic-address = cpu_to_le32(BUILD_IOAPIC_ADDR); io_apic-interrupt = cpu_to_le32(0); diff --git a/src/config.h b/src/config.h index 3a70867..878c691 100644 --- a/src/config.h +++ b/src/config.h @@ -52,9 +52,11 @@ #define BUILD_PCIMEM64_END0x100ULL #define BUILD_IOAPIC_ADDR 0xfec0 +#define BUILD_IOAPIC_ID 0 #define BUILD_HPET_ADDRESS0xfed0 #define BUILD_APIC_ADDR 0xfee0 + // Important real-mode segments #define SEG_IVT 0x #define SEG_BDA 0x0040 -- 1.7.10.4 -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH]: enable USE_PLATFORM_CLOCK bit in FADT flags
On Thu, Jul 19, 2012 at 08:44:07PM -0400, Kevin O'Connor wrote: On Tue, Jul 17, 2012 at 02:18:00PM -0300, Marcelo Tosatti wrote: Enable bit 15 (USE_PLATFORM_CLOCK) of FADT flags field so that older Windows guests do not make use of the TSC for timestamping. I fixed up and committed this patch. But after I committed it, I realized it reverted Gleb's Drop FIX_RTC flag from FADT patch (c7be281b). Gleb, if you can you confirm c7be281b is still valid I'll put it back in. It also reverts 20fcf9b. They are both still valid. Marcelo what source tree the patch was against? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Bootorder Failover (Patch)
On Tue, Jul 17, 2012 at 06:43:01AM -0600, Steve Goodrich wrote: From: Kevin O'Connor [mailto:ke...@koconnor.net] If checking for an MBR is sufficient to determine if the drive is bootable then I'd suggest adding an option to read and validate the MBR in the POST phase prior to assigning a drive id to it. I had looked into this, but could not trace out the functions/methods that would allow me to read a single sector from the device. Can you give me a pointer on this? [...] Why is it not safe to alter the IDMap in the boot phase? Is it unsafe to allow it to change before the device has actually booted, or is it only unsafe to change once the device has transferred execution to the OS? The BIOS specs call for making the f-segment read-only after the POST phase ends. This is implemented under QEMU (and only on QEMU). The IDMap variable (along with all variables marked with VAR16) is stored in the f-segment. What BIOS specs are you referring to? I don't recall seeing those. Also, since this is currently targeted at a production coreboot/SeaBIOS environment, would it be sufficient to make the feature dependent upon the BIOS being built for coreboot? If it makes sense for QEMU why add the limitation? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [QEMU PATCH 2/7] hw/apic.c: rename bit functions to not conflict with bitops.h
On Sat, Jul 14, 2012 at 09:09:23AM +, Blue Swirl wrote: On Fri, Jul 13, 2012 at 6:07 PM, Eduardo Habkost ehabk...@redhat.com wrote: On Thu, Jul 12, 2012 at 07:24:35PM +, Blue Swirl wrote: On Tue, Jul 10, 2012 at 8:22 PM, Eduardo Habkost ehabk...@redhat.com wrote: Signed-off-by: Eduardo Habkost ehabk...@redhat.com Maybe the bitops functions should be renamed instead, for example prefixed by 'qemu_'. That may be safer if one day the kernel find their way to system headers too. Well, if there's any risk the kernel functions will conflict with the QEMU function names, that would be an additional reason to rename the apic.c functions too, so they don't conflict with the kernel functions either. Yes, that could be the case too. Than it would be Linux headers problem and will be fixed there. Personally, I would never sent a patch to rename the bitops.h functions, as the current names work perfectly to me. --- hw/apic.c | 34 +- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/hw/apic.c b/hw/apic.c index 60552df..d322fe3 100644 --- a/hw/apic.c +++ b/hw/apic.c @@ -50,7 +50,7 @@ static int ffs_bit(uint32_t value) return ctz32(value); } -static inline void set_bit(uint32_t *tab, int index) +static inline void apic_set_bit(uint32_t *tab, int index) { int i, mask; i = index 5; @@ -58,7 +58,7 @@ static inline void set_bit(uint32_t *tab, int index) tab[i] |= mask; } -static inline void reset_bit(uint32_t *tab, int index) +static inline void apic_reset_bit(uint32_t *tab, int index) { int i, mask; i = index 5; @@ -66,7 +66,7 @@ static inline void reset_bit(uint32_t *tab, int index) tab[i] = ~mask; } -static inline int get_bit(uint32_t *tab, int index) +static inline int apic_get_bit(uint32_t *tab, int index) { int i, mask; i = index 5; @@ -183,7 +183,7 @@ void apic_deliver_pic_intr(DeviceState *d, int level) case APIC_DM_FIXED: if (!(lvt APIC_LVT_LEVEL_TRIGGER)) break; -reset_bit(s-irr, lvt 0xff); +apic_reset_bit(s-irr, lvt 0xff); /* fall through */ case APIC_DM_EXTINT: cpu_reset_interrupt(s-cpu_env, CPU_INTERRUPT_HARD); @@ -379,13 +379,13 @@ void apic_poll_irq(DeviceState *d) static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode) { -apic_report_irq_delivered(!get_bit(s-irr, vector_num)); +apic_report_irq_delivered(!apic_get_bit(s-irr, vector_num)); -set_bit(s-irr, vector_num); +apic_set_bit(s-irr, vector_num); if (trigger_mode) -set_bit(s-tmr, vector_num); +apic_set_bit(s-tmr, vector_num); else -reset_bit(s-tmr, vector_num); +apic_reset_bit(s-tmr, vector_num); if (s-vapic_paddr) { apic_sync_vapic(s, SYNC_ISR_IRR_TO_VAPIC); /* @@ -405,8 +405,8 @@ static void apic_eoi(APICCommonState *s) isrv = get_highest_priority_int(s-isr); if (isrv 0) return; -reset_bit(s-isr, isrv); -if (!(s-spurious_vec APIC_SV_DIRECTED_IO) get_bit(s-tmr, isrv)) { +apic_reset_bit(s-isr, isrv); +if (!(s-spurious_vec APIC_SV_DIRECTED_IO) apic_get_bit(s-tmr, isrv)) { ioapic_eoi_broadcast(isrv); } apic_sync_vapic(s, SYNC_FROM_VAPIC | SYNC_TO_VAPIC); @@ -445,7 +445,7 @@ static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask, int idx = apic_find_dest(dest); memset(deliver_bitmask, 0x00, MAX_APIC_WORDS * sizeof(uint32_t)); if (idx = 0) -set_bit(deliver_bitmask, idx); +apic_set_bit(deliver_bitmask, idx); } } else { /* XXX: cluster mode */ @@ -455,11 +455,11 @@ static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask, if (apic_iter) { if (apic_iter-dest_mode == 0xf) { if (dest apic_iter-log_dest) -set_bit(deliver_bitmask, i); +apic_set_bit(deliver_bitmask, i); } else if (apic_iter-dest_mode == 0x0) { if ((dest 0xf0) == (apic_iter-log_dest 0xf0) (dest apic_iter-log_dest 0x0f)) { -set_bit(deliver_bitmask, i); +apic_set_bit(deliver_bitmask, i); } } } else { @@ -502,14 +502,14 @@ static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode, break; case 1: memset(deliver_bitmask, 0x00, sizeof(deliver_bitmask)); -
Re: [SeaBIOS] [Qemu-devel] [RFC 0/7+1] QEMU APIC ID + topology bug + CPU hotplug
On Thu, Jul 12, 2012 at 03:51:48PM +0200, Igor Mammedov wrote: On 07/10/2012 10:22 PM, Eduardo Habkost wrote: The hotplug case is a bit more complex: we need to either: - have a mechanism to let the ACPI SSDT code know what's the APIC ID of hotplugged CPUs; or - make Seabios run some code in the hotplugged CPU (I am assuming that this is simply not possible). If SMM is supported by qemu/kvm than it will be possible to trigger SMI from APCI method and SMI handler, that are supposed to run on all CPUs (including hot-plugged one) , could fixup APIC ID of added CPU in some memory region used by ACPI. SMM is not supported (arguably is should not be supported on real HW too), but even if it was default SMM address is 0x38000 IIRC and it is a middle of a memory and can be used by a guest OS. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH 1/3] Fix aml_name_string() to recognize block name modifiers.
On Tue, May 22, 2012 at 09:23:03PM -0400, Kevin O'Connor wrote: On Sun, May 20, 2012 at 12:03:38PM +0300, Gleb Natapov wrote: Signed-off-by: Gleb Natapov g...@redhat.com The patch series looks okay to me. Let me know when the corresponding qemu patches are committed. It is committed now: 459ae5ea5ad682c2b3220beb244d4102c1a4e332 -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] Problem accessing a small virtio disk
I noticed that SeaBIOS fails to boot from virtio disk smaller than 512K. The attempt to access a disk fails at basic_access(). (cylinder = nlc || head = nlh || sector nlspt) is true because nlc is zero. The problem seams to be in how SeaBIOS calculates lchs from pchs in get_translation(). Both QEMU and SeaBIOS fake disk geometry from real disk size, but it seams that they do it differently and when they disagree part of the disk may not be readable. What would be the best way to fix that? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] GPT disks in a BIOS world
On Tue, Jun 05, 2012 at 10:58:43AM +0200, Fred . wrote: On Tue, Jun 5, 2012 at 5:21 AM, Kevin O'Connor ke...@koconnor.net wrote: On Mon, Jun 04, 2012 at 07:11:30PM -0700, Ralf A. Quint wrote: At 03:36 PM 6/4/2012, Kevin O'Connor wrote: On Mon, Jun 04, 2012 at 03:33:05PM +0200, Fred . wrote: http://mjg59.dreamwidth.org/8035.html Does SeaBIOS support GPT disk labels? Does SeaBIOS understand GPT disk labels? Is it aware of GPT? This is a common misunderstanding of the BIOS. The BIOS doesn't do anything with partition tables at all (at least according to the available specs). Thus, the BIOS doesn't care if it's a legacy partition table or a GPT partition table. Excuse me, but isn't it the BIOS that after POST is initiating the boot process from the active partition? Not quite. The BIOS loads the first sector of the hard drive (ie, the MBR) into memory and runs the code found there. It cares nothing about the partition table. It's quite common for the executable code in that first sector to analyze the partition table, load yet other code, and then jump to that code - but that activity is outside the BIOS and is easily upgradable. But does GPT disks even have a MBR? Do you know this new site called google.com? You can try to ask it a couple of question :) http://www.petri.co.il/gpt-vs-mbr-based-disks.htm Isn't the GPT a replacement for MBR? If the disk doesn't have any MBR, does the BIOS load the first sector of GPT? At least, that's what SeaBIOS does and what the available BIOS specs call for (and what every boot loader I've seen expects the bios to do). But, who knows what crazy things various comercial BIOSes do. -Kevin ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
QEMU may want to disable guest's S3/S4 support and it wants to distinguish between regular powerdown and S4 powerdown. To support that new fw_cfg option was added that passes supported system states and what value should guest use to enter each state. States are passed in 6 byte array. Each byte represents one system state. If byte at offset X has its MSB set it means that system state X is supported and to enter it guest should use the value from lowest 7 bits. Patch also detects old QEMU and uses values that work in backwards compatible way there. Signed-off-by: Gleb Natapov g...@redhat.com --- src/acpi-dsdt.dsl | 32 - src/acpi-dsdt.hex | 42 +--- src/acpi.c | 15 src/ssdt-pcihp.dsl | 36 ++ src/ssdt-pcihp.hex | 185 +--- 5 files changed, 172 insertions(+), 138 deletions(-) diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl index 4bdc268..37899fc 100644 --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -604,38 +604,6 @@ DefinitionBlock ( } } - -/ - * Suspend - / - -/* - * S3 (suspend-to-ram), S4 (suspend-to-disk) and S5 (power-off) type codes: - * must match piix4 emulation. - */ -Name (\_S3, Package (0x04) -{ -0x01, /* PM1a_CNT.SLP_TYP */ -0x01, /* PM1b_CNT.SLP_TYP */ -Zero, /* reserved */ -Zero /* reserved */ -}) -Name (\_S4, Package (0x04) -{ -Zero, /* PM1a_CNT.SLP_TYP */ -Zero, /* PM1b_CNT.SLP_TYP */ -Zero, /* reserved */ -Zero /* reserved */ -}) -Name (\_S5, Package (0x04) -{ -Zero, /* PM1a_CNT.SLP_TYP */ -Zero, /* PM1b_CNT.SLP_TYP */ -Zero, /* reserved */ -Zero /* reserved */ -}) - - / * CPU hotplug / diff --git a/src/acpi-dsdt.hex b/src/acpi-dsdt.hex index a4af597..8678fbf 100644 --- a/src/acpi-dsdt.hex +++ b/src/acpi-dsdt.hex @@ -3,12 +3,12 @@ static unsigned char AmlCode[] = { 0x53, 0x44, 0x54, -0x21, -0x11, +0xfd, +0x10, 0x0, 0x0, 0x1, -0xe8, +0x4a, 0x42, 0x58, 0x50, @@ -3925,42 +3925,6 @@ static unsigned char AmlCode[] = { 0x52, 0x51, 0x30, -0x8, -0x5f, -0x53, -0x33, -0x5f, -0x12, -0x6, -0x4, -0x1, -0x1, -0x0, -0x0, -0x8, -0x5f, -0x53, -0x34, -0x5f, -0x12, -0x6, -0x4, -0x0, -0x0, -0x0, -0x0, -0x8, -0x5f, -0x53, -0x35, -0x5f, -0x12, -0x6, -0x4, -0x0, -0x0, -0x0, -0x0, 0x10, 0x49, 0xe, diff --git a/src/acpi.c b/src/acpi.c index 30888b9..06ffe0a 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -492,6 +492,8 @@ extern void link_time_assertion(void); static void* build_pcihp(void) { +char *sys_states; +int sys_state_size; u32 rmvc_pcrm; int i; @@ -523,6 +525,19 @@ static void* build_pcihp(void) } } +sys_states = romfile_loadfile(etc/system-states, sys_state_size); +if (!sys_states || sys_state_size != 6) +sys_states = (char[]){128, 0, 0, 129, 128, 128}; + +if (!(sys_states[3] 128)) +ssdt[acpi_s3_name[0]] = 'X'; +if (!(sys_states[4] 128)) +ssdt[acpi_s4_name[0]] = 'X'; +else +ssdt[acpi_s4_pkg[0] + 1] = ssdt[acpi_s4_pkg[0] + 3] = sys_states[4] 127; +((struct acpi_table_header*)ssdt)-checksum = 0; +((struct acpi_table_header*)ssdt)-checksum -= checksum(ssdt, sizeof(ssdp_pcihp_aml)); + return ssdt; } diff --git a/src/ssdt-pcihp.dsl b/src/ssdt-pcihp.dsl index 4b435b8..12555e2 100644 --- a/src/ssdt-pcihp.dsl +++ b/src/ssdt-pcihp.dsl @@ -95,4 +95,40 @@ DefinitionBlock (ssdt-pcihp.aml, SSDT, 0x01, BXPC, BXSSDTPCIHP, 0x1) gen_pci_hotplug(1f) } } + +Scope(\) { +/ + * Suspend + / + +/* + * S3 (suspend-to-ram), S4 (suspend-to-disk) and S5 (power-off) type codes: + * must match piix4 emulation. + */ + +ACPI_EXTRACT_NAME_STRING acpi_s3_name +Name (_S3, Package (0x04) +{ +One, /* PM1a_CNT.SLP_TYP */ +One, /* PM1b_CNT.SLP_TYP */ +Zero, /* reserved */ +Zero /* reserved */ +}) +ACPI_EXTRACT_NAME_STRING acpi_s4_name +ACPI_EXTRACT_PKG_START acpi_s4_pkg +Name (_S4, Package (0x04) +{ +0x2, /* PM1a_CNT.SLP_TYP */ +0x2, /* PM1b_CNT.SLP_TYP */ +Zero, /* reserved */ +Zero /* reserved */ +}) +Name (_S5, Package (0x04) +{ +Zero, /* PM1a_CNT.SLP_TYP */ +Zero, /* PM1b_CNT.SLP_TYP */ +Zero, /* reserved */ +Zero /* reserved */ +}) +} } diff --git
[SeaBIOS] [PATCH 1/3] Fix aml_name_string() to recognize block name modifiers.
Signed-off-by: Gleb Natapov g...@redhat.com --- tools/acpi_extract.py |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/tools/acpi_extract.py b/tools/acpi_extract.py index 5f613e4..8038269 100755 --- a/tools/acpi_extract.py +++ b/tools/acpi_extract.py @@ -121,7 +121,11 @@ def aml_name_string(offset): if (aml[offset] != 0x08): die( Name offset 0x%x: expected 0x08 actual 0x%x % (offset, aml[offset])); -return offset + 1; +offset += 1 +# Block Name Modifier. Skip it. +if (aml[offset] == 0x5c or aml[offset] == 0x5e): +offset += 1 +return offset; # Given data offset, find dword const offset def aml_data_dword_const(offset): -- 1.7.7.3 ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 03:47:02PM +0300, Avi Kivity wrote: On 05/20/2012 03:36 PM, Gleb Natapov wrote: On Sun, May 20, 2012 at 03:30:50PM +0300, Avi Kivity wrote: On 05/20/2012 03:15 PM, Gleb Natapov wrote: On Sun, May 20, 2012 at 02:44:51PM +0300, Avi Kivity wrote: On 05/20/2012 12:03 PM, Gleb Natapov wrote: QEMU may want to disable guest's S3/S4 support and it wants to distinguish between regular powerdown and S4 powerdown. To support that new fw_cfg option was added that passes supported system states and what value should guest use to enter each state. States are passed in 6 byte array. Each byte represents one system state. If byte at offset X has its MSB set it means that system state X is supported and to enter it guest should use the value from lowest 7 bits. Patch also detects old QEMU and uses values that work in backwards compatible way there. Do we actually have to patch the DSDT? Or can _S3 etc be made into functions instead? (and talk to the bios, or even to fwcfg directly?) We better not talk to fwcfg after OSPM is started since this is firmware confing interface. Why not? The OS isn't going to talk to it, so we can have a driver in ACPI. The OS is going to talk to it since the OS is the one who interprets AML. I meant, not directly. So the driver in ACPI has exclusive access. What's the difference? We may want to disable fwcfg after OS bootup at all in the feature. Who knows what kind of sensitive information we may want to pass by it in the feature? May be something TPM related? fwcfg is for passing information to the guest. If you want to hide something from the guest, just don't put it in fwcfg. Where to put it if we want to pass it to a firmware, but not an OS. That was the point of fwcfg. If you want to pass something to a guest OS use virtio-serial. And I do not see any advantage of using fwcfg from AML. It's an alternative to patching AML. Sure it takes some effort to write the driver, but afterwards we can modify the guest behaviour more easily. One possible client is -M old, so you can revert to previous behaviour depending on fwcfg data. -M old is easy to support with the current patch. You just set new properties to compatibility values. The code is written with this in mind. And this is not an alternative to patching AML as I am trying to explain to you below. You can eliminate patching of s4 value, but that's it, you still need to patch out _S3/_S4 names. (we don't need a driver in AML to avoid patching, we can have AML talk to the bios and the bios drive fwcfg; but I think we'll find uses for a driver). I am not sure what you mean. AML can't talk to the bios. It can read values that bios put somewhere. I do not see advantage of this method and it requires patching still. Regardless, presence of _S3 name or method is all that needed for OS enabling S3 option. If _S3 is defined as a method it has to return Package() otherwise iasl refuses to compile it. Can't we Return (Package (...) { ... }) or equivalent? We can, how does it help? The contents of the package can be determined at runtime. And? _S3 name should not exists at all in order to disable S3, not return something different. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 05:46:46PM +0300, Avi Kivity wrote: On 05/20/2012 05:43 PM, Gleb Natapov wrote: Or it can be a fixed address in low memory, or a scratch register in hardware. Both will work (fixed addresses are better be avoided and who needs another PV device), but I do not see how either of them is better then patching. What is your concern? Patching is harder to maintain. Unfortunately it's unavoidable. Here we in agreement, and I was against patching till it was unavoidable, but than pci hotplug started using it, and afterwards processor definitions, so no point in avoiding it now by using inferior methods. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.
On Sun, May 20, 2012 at 12:39:13PM -0400, Kevin O'Connor wrote: On Sun, May 20, 2012 at 07:25:40PM +0300, Avi Kivity wrote: On 05/20/2012 07:16 PM, Kevin O'Connor wrote: Here we in agreement, and I was against patching till it was unavoidable, but than pci hotplug started using it, and afterwards processor definitions, so no point in avoiding it now by using inferior methods. I agree as well. What's the background to needing to have dynamic S3/S4 definitions? (Why will some qemu instances be able to sleep and not others?) Backwards compatibility. qemu has a -M machine-type option that expose an old qemu's guest-visible attributes. If an old qemu didn't support S3, then -M old shouldn't either. The DSDT has claimed S3, S4, and S5 support since SeaBIOS has supported ACPI. What's the background to the requirement to stop claiming support for it? Not all guests have working S3/S4 implementation. We want management to be able to disable S3/S4 for such guests. S4 value changes since we want to distinguish between S4 and S5 in qemu. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH 2/2] Get system state configuration from QEMU and patcth DSDT with it.
On Wed, May 16, 2012 at 05:50:31PM +0200, Paolo Bonzini wrote: Il 16/05/2012 15:46, Gleb Natapov ha scritto: I saw that, but I don't get why doing it this way instead of defining the object in AML and patching it? I can define Name(S4VL, 0x2) and path 0x2 to whatever QEMU wants me to use, or I can patch Package directly like I did. Can we build an SSDT that includes the contents of fw_cfg (e.g. FW_CFG_SIGNATURE at offset 0, FW_CFG_UUID at offset 4, FW_CFG_NOGRAPHIC at offset 16... the entry - offset mapping and the defaults would be part of SeaBIOS), and then read that data from normal DSDT methods? Kevin does not want to use offsets any more :) He wants to use files, so this will not work for new entries. That would be similar to Gerd's patch, but without letting the OSPM use the real fw_cfg device. Latest Gerd's patch does not use fw_cfg device. It creates AML code manually, I am saying we can do the same by patching, but the end result will be the same. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH 2/2] Get system state configuration from QEMU and patcth DSDT with it.
On Mon, May 14, 2012 at 09:43:19PM -0400, Kevin O'Connor wrote: On Mon, May 14, 2012 at 03:35:23PM +0300, Gleb Natapov wrote: QEMU may want to disable guest's S3/S4 support and it wants to distinguish between regular powerdown and S4 powerdown. To support that new fw_cfg option was added that passes supported system states and what value should guest use to enter each state. States are passed in 6 byte array. Each byte represents one system state. If byte at offset X has its MSB set it means that system state X is supported and to enter it guest should use the value from lowest 7 bits. Patch also detects old QEMU and uses values that work in backwards compatible way there. A couple of comments - see below. [...] --- a/src/acpi-dsdt.dsl +++ b/src/acpi-dsdt.dsl @@ -613,6 +613,7 @@ DefinitionBlock ( * S3 (suspend-to-ram), S4 (suspend-to-disk) and S5 (power-off) type codes: * must match piix4 emulation. */ +ACPI_EXTRACT_NAME_STRING acpi_s3_name Name (\_S3, Package (0x04) { 0x01, /* PM1a_CNT.SLP_TYP */ @@ -620,10 +621,12 @@ DefinitionBlock ( Zero, /* reserved */ Zero /* reserved */ }) +ACPI_EXTRACT_NAME_STRING acpi_s4_name +ACPI_EXTRACT_PKG_START acpi_s4_pkg The DSDT is quite complex and has a diverse usage. I'd feel more comfortable leaving it as static and doing any dynamic work in an SSDT. In this particular case, can't the objects be turned into methods which calculate the associated values and return the correct results? Checked with WindowsXP and Linux and they work if I make _S3 to be a method that returns package, so we can drop ACPI_EXTRACT_PKG_START and do runtime calculation, but what this calculation will be based on? We will have to pass QEMU S4 value to AML somehow and this will involve patching of something eventually. And of course we will still have to patch out _S3/_S4 names in case qemu want to disable them. I do not see how we can disable them in any other way. I think the use of patching will only increase now after we let that genie out of the bottle, so moving each part that we want to patch in separate SSDT will not scale. Here the patching is minimal, moving only _Sx to a separate SSDT feels unnecessary. Of course we can do it later if thing will become more complex. We are not creating any ABIs here that we cannot redo, just small implementation detail. [...] --- a/src/paravirt.c +++ b/src/paravirt.c @@ -92,6 +92,22 @@ int qemu_cfg_irq0_override(void) return v; } +int qemu_cfg_system_states(char *states) +{ I'd prefer to see any new fw_cfg entries use the QEMU_CFG_FILE_DIR mechanism so that seabios can use romfile_loadfile (or similar). The number of files you can pass over fw_cfg interface is limited due to implementation details. I think we should continue using regular fw_cfg entries for code that is QEMU specific and files for code that is shared with coreboot. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [coreboot] seabios feature request : reboot VM or retry boot devices when no valid boot disk is found
On Sat, May 12, 2012 at 10:01:49AM -0400, Kevin O'Connor wrote: On Sat, May 12, 2012 at 03:04:52AM +0200, Peter Stuge wrote: Kevin O'Connor wrote: +// Unable to find bootable device - warn user and eventually retry. +static void +boot_fail(void) +{ +printf(No bootable device.\n); +// Wait for 60 seconds and then reboot. +u32 end = calc_future_timer(60*1000); I'd suggest printf(No bootable device found. Retrying in 5 seconds...\n); and lowering the timeout. I'd only do this if the timeout was large - indeed I think 60 seconds is a little on the low side. (Not being able to boot is almost always an error - it should be fixed not covered up.) Agree. For VM use it's better to notify management about it. That said, making it a config option is trivial. The patch below does not allow to disable the behaviour (short of setting ridiculously long timeout). Lets make 0 timeout to mean never timeout and make it a default. -Kevin diff --git a/src/boot.c b/src/boot.c index 8cc94b0..b5b3360 100644 --- a/src/boot.c +++ b/src/boot.c @@ -617,6 +617,26 @@ boot_rom(u32 vector) call_boot_entry(so, 0); } +// Unable to find bootable device - warn user and eventually retry. +static void +boot_fail(void) +{ +u32 retrytime = romfile_loadint(etc/boot-fail-wait, 60*1000); +printf(No bootable device. Retrying in %d seconds.\n, retrytime/1000); +// Wait for 'retrytime' milliseconds and then reboot. +u32 end = calc_future_timer(retrytime); +for (;;) { +if (check_timer(end)) +break; +wait_irq(); +} +printf(Rebooting.\n); +struct bregs br; +memset(br, 0, sizeof(br)); +br.code = SEGOFF(SEG_BIOS, (u32)reset_vector); +call16big(br); +} + // Determine next boot method and attempt a boot using it. static void do_boot(u16 seq_nr) @@ -625,10 +645,7 @@ do_boot(u16 seq_nr) panic(Boot support not compiled in.\n); if (seq_nr = BEVCount) { -printf(No bootable device.\n); -// Loop with irqs enabled - this allows ctrl+alt+delete to work. -for (;;) -wait_irq(); +boot_fail(); } // Boot the given BEV type. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [coreboot] seabios feature request : reboot VM or retry boot devices when no valid boot disk is found
On Sat, May 12, 2012 at 03:04:52AM +0200, Peter Stuge wrote: Kevin O'Connor wrote: +// Unable to find bootable device - warn user and eventually retry. +static void +boot_fail(void) +{ +printf(No bootable device.\n); +// Wait for 60 seconds and then reboot. +u32 end = calc_future_timer(60*1000); I'd suggest printf(No bootable device found. Retrying in 5 seconds...\n); and lowering the timeout. On host running hundreds of VMs reboot each 5 seconds may generate a lot of unneeded load. Better make it configurable with option to disable the behaviour. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RESEND PATCH v3] hotplug: add device per func in ACPI DSDT tables
On Fri, May 11, 2012 at 01:46:17AM +0800, Jiang Liu wrote: On 05/11/2012 01:42 AM, Michael S. Tsirkin wrote: On Fri, May 11, 2012 at 01:17:38AM +0800, Jiang Liu wrote: On 05/09/2012 03:24 PM, Amos Kong wrote: --- src/ssdt-pcihp.dsl | 17 src/ssdt-pcihp.hex | 8869 +++- 2 files changed, 8781 insertions(+), 105 deletions(-) diff --git a/src/ssdt-pcihp.dsl b/src/ssdt-pcihp.dsl index 4b435b8..2a3c326 100644 --- a/src/ssdt-pcihp.dsl +++ b/src/ssdt-pcihp.dsl @@ -17,14 +17,23 @@ DefinitionBlock (ssdt-pcihp.aml, SSDT, 0x01, BXPC, BXSSDTPCIHP, 0x1) // at runtime, if the slot is detected to not support hotplug. // Extract the offset of the address dword and the // _EJ0 name to allow this patching. -#define hotplug_slot(slot) \ -Device (S##slot) { \ +#define hotplug_func(slot, fn) \ +Device (S##slot##fn) { \ ACPI_EXTRACT_NAME_DWORD_CONST aml_adr_dword \ - Name (_ADR, 0x##slot##) \ + Name (_ADR, 0x##slot##000##fn) \ ACPI_EXTRACT_METHOD_STRING aml_ej0_name \ Method (_EJ0, 1) { Return(PCEJ(0x##slot)) } \ Name (_SUN, 0x##slot)\ } It would be perfect if the Device object could also support _PS0 and _STA methods. It needs qemu support, and some backward compatibility hack. Why? Could we re-add the slot back after hot-removing it from the guest OS with this ACPI implementation? Say execute following scripts from guest OS. echo 0 /sys/bus/pci/slot/xx/power echo 1 /sys/bus/pci/slot/xx/power No because qemu removes device after eject. Do you have a need for this functionality? What is it? I'm not familiar with qemu:( On native OS, admin could trigger PCI device hotplug operations through /sys/bus/pci/slot/xx/power. Not sure whether that's needed for guest OS too. Why is it needed on physical HW? May be it is needed in a VM for the same reason? -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RESEND PATCH v3] hotplug: add device per func in ACPI DSDT tables
On Fri, May 11, 2012 at 11:44:02PM +0800, Jiang Liu wrote: On 05/11/2012 06:14 PM, Gleb Natapov wrote: I'm not familiar with qemu:( On native OS, admin could trigger PCI device hotplug operations through /sys/bus/pci/slot/xx/power. Not sure whether that's needed for guest OS too. Why is it needed on physical HW? May be it is needed in a VM for the same reason? As Amos has mentioned, it's used power on/off a PCI device instead of physical hotplug. Not sure whether it's needed in guest OS. Probably for assigned devices. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] SeaBIOS vs other BIOS?
On Sun, May 06, 2012 at 10:26:31PM +0200, Fred . wrote: And in terms of standards compliance? I know proprietary BIOS have advantage when it comes to SMBIOS due to the implementation in SeaBIOS lagging behind several versions. On Sun, May 6, 2012 at 8:00 PM, Peter Stuge pe...@stuge.se wrote: Fred . wrote: How is SeaBIOS working towards the non-technical goals of the project? This is not so clear. I'm not even sure that there are non-technical goals for the project. Competing with commercial BIOS products would require a company to put a SeaBIOS-based PC firmware to market, quite likely in concert with coreboot. I know of one company which offers among other things coreboot services, Sage Engineering, who are quite active in the coreboot community. http://www.se-eng.com/coreboot.html But even so you can see that the business model is different from commercial BIOS products, and already this small difference presents a non-technical challenge. SeaBIOS lacks documentation. It lacks communication. The website is not updated with news about the development. There is no mention of what's new, whats planned, etc. Now I am curious what other BIOSes give you all that? In all that points Seabios in not different from most other open source project. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] SeaBIOS vs other BIOS?
On Mon, May 07, 2012 at 10:44:21AM +0200, Fred . wrote: On Mon, May 7, 2012 at 9:40 AM, Gleb Natapov g...@redhat.com wrote: On Sun, May 06, 2012 at 10:26:31PM +0200, Fred . wrote: And in terms of standards compliance? I know proprietary BIOS have advantage when it comes to SMBIOS due to the implementation in SeaBIOS lagging behind several versions. On Sun, May 6, 2012 at 8:00 PM, Peter Stuge pe...@stuge.se wrote: Fred . wrote: How is SeaBIOS working towards the non-technical goals of the project? This is not so clear. I'm not even sure that there are non-technical goals for the project. Competing with commercial BIOS products would require a company to put a SeaBIOS-based PC firmware to market, quite likely in concert with coreboot. I know of one company which offers among other things coreboot services, Sage Engineering, who are quite active in the coreboot community. http://www.se-eng.com/coreboot.html But even so you can see that the business model is different from commercial BIOS products, and already this small difference presents a non-technical challenge. SeaBIOS lacks documentation. It lacks communication. The website is not updated with news about the development. There is no mention of what's new, whats planned, etc. Now I am curious what other BIOSes give you all that? In all that points Seabios in not different from most other open source project. -- Gleb. Perhaps other BIOS have private channels to make such communications directly to their customers. SeaBIOS customers pretty much define SeaBIOS roadmap, so perhaps such communication is unnecessary. Or perhaps they don't need to due to having been in the BIOS game pretty much since it started. Or perhaps they do not care because most of the BIOS customers do not aware of its existence or that they have an alternative? I think information, documentation, roadmaps, public announcements, timely communication would be good. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] synching GPE0_BLK between OVMF and qemu
On Fri, Apr 27, 2012 at 07:24:48PM +0200, Laszlo Ersek wrote: On 04/27/12 17:12, Jordan Justen wrote: On Fri, Apr 27, 2012 at 07:31, Laszlo Ersek ler...@redhat.com wrote: edk2's OvmfPkg/AcpiTables/Platform.h specifies GPE0_BLK at 0x40C, while qemu's hw/acpi_piix4.c expects the guest to access it at 0xAFE0. Which macro should be modified to get them in sync? Do they need to be in sync? It appears to me so: https://bugzilla.redhat.com/show_bug.cgi?id=653382#c22 We set PBMA to 0x400 in OvmfPkg/Library/AcpiTimerLib/AcpiTimerLib.c, so isn't 0x40c correct? Considering OVMF in isolation, I presume it's self-consistent. However, is it necessary (a) to group these ACPI registers closely together, (b) to base the group at 0x400? From 5.2.9 Fixed ACPI Description Table (FADT) in the ACPI spec (v5.0) it would appear OVMF can freely choose where to put GPE0_BLK, in both senses (ie. port address considered alone, and also in relation to the other ACPI registers). OVMF can't freely choose where to put GPE0_BLK. It should describe to OSPM where GPE0_BLK is in HW. If it provides incorrect value this is OVMF bug. Considering SeaBIOS again (build_fadt()): - PORT_ACPI_PM_BASE is 0xb000, - PM1a_EVT_BLK, PM1a_CNT_BLK and PM_TMR_BLK are located consecutively from this base, - but GPE0_BLK is placed at 0xafe0 (build_fadt() -- pci_init_device(fadt_init_tbl) -- piix4_fadt_init()) That is because those are two totally different things. One is PM1 register another is GPE0 register. The very obvious hint that they are unrelated is that they described by two different fields in FADT. But I'm likely missing something ^W everything... Thanks! Laszlo -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] synching GPE0_BLK between OVMF and qemu
On Fri, Apr 27, 2012 at 08:47:00PM +0200, Laszlo Ersek wrote: On 04/27/12 20:09, Gleb Natapov wrote: On Fri, Apr 27, 2012 at 07:24:48PM +0200, Laszlo Ersek wrote: From 5.2.9 Fixed ACPI Description Table (FADT) in the ACPI spec (v5.0) it would appear OVMF can freely choose where to put GPE0_BLK, in both senses (ie. port address considered alone, and also in relation to the other ACPI registers). OVMF can't freely choose where to put GPE0_BLK. It should describe to OSPM where GPE0_BLK is in HW. If it provides incorrect value this is OVMF bug. By freely I didn't mean freedom from correctly setting up the FADT. I meant OVMF coders can reasonably freely choose what port to use -- because the ACPI spec, or their own codebase, or the other registers don't restrict them -- and then they have to describe the chosen port in FADT. As a special case, they even have the freedom to choose GPE0_BLK so that it matches the hardware (qemu) and things start working; there's no external obstacle in their way. My only point was you're free to set up GPE0_BLK correctly, nothing ties your hands. Considering SeaBIOS again (build_fadt()): - PORT_ACPI_PM_BASE is 0xb000, - PM1a_EVT_BLK, PM1a_CNT_BLK and PM_TMR_BLK are located consecutively from this base, - but GPE0_BLK is placed at 0xafe0 (build_fadt() -- pci_init_device(fadt_init_tbl) -- piix4_fadt_init()) That is because those are two totally different things. One is PM1 register another is GPE0 register. The very obvious hint that they are unrelated is that they described by two different fields in FADT. Of course. That was my exact point. They are independent, so OVMF isn't forced either to squeeze them in the same port range from 0x400. OVMF is free to locate GPE0 so that it matches qemu, independently from other ACPI registers. This was how I interpreted our discussion with Jordan: L: Shouldn't qemu OVMF agree on GPE0? J: Why? Anyway, OVMF should be correct, because all ACPI registers are in one tight bunch, starting from 0x400. L: None of those two characteristics (1: 0x400, 2: one tight bunch) are required by the spec. For proof, see what SeaBIOS does: 1: it doesn't use 0x400 as base, 2: GPE0 is not even above the base. To quote a mutual friend, we're in violent agreement. Oh yes, we are :) Anyway, back to my original question: currently OVMF and qemu disagree wrt. GPE0. Which one should I try to modify so that they agree? OVMF of course if you ask me. We are not going to recall millions of QEMU evaluations boards! -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH 0/9] ACPI memory hotplug
On Mon, Apr 23, 2012 at 04:31:04PM +0300, Avi Kivity wrote: On 04/22/2012 05:20 PM, Gleb Natapov wrote: On Sun, Apr 22, 2012 at 05:13:27PM +0300, Avi Kivity wrote: On 04/22/2012 05:09 PM, Gleb Natapov wrote: On Sun, Apr 22, 2012 at 05:06:43PM +0300, Avi Kivity wrote: On 04/22/2012 04:56 PM, Gleb Natapov wrote: start. We will need it for migration anyway. hotplug-able memory slots i.e. initial system memory is not modeled with memslots. The concept could be generalized to include all memory though, or it could more closely follow kvm-memory slots. OK, I hope final version will allow for memory 4G to be hot-pluggable. Why is that important? Because my feeling is that people that want to use this kind of feature what to start using it with VMs smaller than 4G. Of course not all memory have to be hot unpluggable. Making first 1M or event first 128M not unpluggable make perfect sense. Can't you achieve this with -m 1G, -device dimm,size=1G,populated=true -device dimm,size=1G,populated=false? From this: (for hw/pc.c PCI hole is currently [below_4g_mem_size, 4G), so hotplugged memory should start from max(4G, above_4g_mem_size). I understand that hotpluggable memory can start from above 4G only. With the config above we will have memory hole from 1G to PCI memory hole. May be not a big problem, but I do not see technical reason for the constrain. (I don't think hotplugging below 512MB is needed, but I don't have any real data on this). 512MB looks like a reasonable limitation too, but again if there is not technical reason for having the limitation why have it? I was thinking about not having tons of 128MB slots, so we don't have a configuration that is far from reality. But maybe this thinking is too conservative. I think it is good interface to make memory that is specified with -m to be one big unpluggable slot, but slots defined with -device should start just above what -m specifies (after proper alignment). Memory hot-plug granularity is controlled by slot's size parameter. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH 0/9] ACPI memory hotplug
On Tue, Apr 24, 2012 at 10:24:51AM +0200, Vasilis Liaskovitis wrote: Hi, On Tue, Apr 24, 2012 at 10:52:24AM +0300, Gleb Natapov wrote: On Mon, Apr 23, 2012 at 02:31:15PM +0200, Vasilis Liaskovitis wrote: The 440fx spec mentions: The address range from the top of main DRAM to 4 Gbytes (top of physical memory space supported by the 440FX PCIset) is normally mapped to PCI. The PMC forwards all accesses within this address range to PCI. What we probably want is that the initial memory map creation takes into account all dimms specified (both populated/unpopulated) Yes. So -m 1G, -device dimm,size=1G,populated=true -device dimm,size=1G,populated=false would create a system map with top of memory and start of PCI-hole at 2G. What -m 1G means on this command line? Isn't it redundant? yes, this was redundant with the original concept. May be we should make -m create non unplaggable, populated slot starting at address 0. Ten you config above will specify 3G memory with 2G populated (first of which is not removable) and 1G unpopulated. PCI hole starts above 3G. I agree -m should mean one big unpluggable slot. So in the new proposal,-device dimm populated=true means a hot-removable dimm that has already been hotplugged. Yes. A question here is when exactly should the initial hot-add event for this dimm be played? If the relevant OSPM has not yet been initialized (e.g. acpi_memhotplug module in a linux guest needs to be loaded), the guest may not see the event. This is a general issue of course, but with initially populated hot-removable dimms it may be a bigger issue. Can ospm acpi initialization be detected? Or maybe you are suggesting populated=true is part of initial memory (i.e. not hot-added, but still hot-removable). Though in that case guestOS may use it for bootmem allocations, making hot-remove more likely to fail at the memory offlining stage. If memory slot is populated even before OSPM is started BIOS will detect that by reading mem_sts and will create e820 map appropriately. OSPM will detect it by evaluating _STA during boot. This is not unique for memory hot-plug. Any hotpluggable device have the same issue. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH 8/9] pc: adjust e820 map on hot-add and hot-remove
On Mon, Apr 23, 2012 at 01:27:40PM +0200, Vasilis Liaskovitis wrote: On Sun, Apr 22, 2012 at 04:58:47PM +0300, Gleb Natapov wrote: On Thu, Apr 19, 2012 at 04:08:46PM +0200, Vasilis Liaskovitis wrote: Hotplugged memory is not persistent in the e820 memory maps. After hotplugging a memslot and rebooting the VM, the hotplugged device is not present. A possible solution is to add an e820 for the new memslot in the acpi_piix4 hot-add handler. On a reset, Seabios (see next patch in series) will enable all memory devices for which it finds an e820 entry that covers the devices's address range. On hot-remove, the acpi_piix4 handler will try to remove the e820 entry corresponding to the device. This will work when no VM reboots happen between hot-add and hot-remove, but it is not a sufficient solution in general: Seabios and GuestOS merge adjacent e820 entries on machine reboot, so the sequence hot-add/ rebootVM / hot-remove will fail to remove a corresponding e820 entry at the hot-remove phase. Why do you need this path and the next one? Bios can restore the state of memslots and build e820 map by reading mems_sts. i see, that is a simpler solution. Since qemu currently creates most ram e820map Quite the contrary. Qemu creates only one entry that Seabios can't figure by itself. entries and passes them to seabios, I tried to follow the same approach. But your suggestion makes things easier and we don't have to worry about merged e820 entries on hot-remove. I 'll rework it. thanks, Vasilis -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH 0/9] ACPI memory hotplug
On Thu, Apr 19, 2012 at 04:08:38PM +0200, Vasilis Liaskovitis wrote: This is a prototype for ACPI memory hotplug on x86_64 target. Based on some earlier work and comments from Gleb. Memslot devices are modeled with a new qemu command line -memslot id=name,start=start_addr,size=sz,node=pxm user is responsible for defining memslots with meaningful start/size values, e.g. not defining a memory slot over a PCI-hole. Alternatively, the start size could also be handled/assigned automatically from the specific emulated hardware (for hw/pc.c PCI hole is currently [below_4g_mem_size, 4G), so hotplugged memory should start from max(4G, above_4g_mem_size). Node is defining numa proximity for this memslot. When not defined it defaults to zero. e.g. -memslot id=hot1,start=4294967296,size=536870912,node=0 will define a 512M memory slot starting at physical address 4G, belonging to numa node 0. Memory slots are added or removed with a new hmp command memslot: Hot-add syntax: memslot id add Hot-remove syntax: memslot id delete - All memslots are initially unpopulated. Memslots are currently modeling only We can add ,populated=true to -memslot to make slot populated from the start. We will need it for migration anyway. hotplug-able memory slots i.e. initial system memory is not modeled with memslots. The concept could be generalized to include all memory though, or it could more closely follow kvm-memory slots. OK, I hope final version will allow for memory 4G to be hot-pluggable. - Memslots are abstracted as qdevices attached to the main system bus. However, memory hotplugging has its own side channel ignoring main_system_bus's hotplug incapability. A cleaner integration would be needed. What's the preferred way of modeling memory devices in qom? Would it be better to attach memory devices as children-links of an acpi-capable device (in the pc case acpi_piix4) instead of the system bus? - Refcounting memory slots has been discussed (but is not included in this series yet). Depopulating a memory region happens on a guestOS _EJ callback, which means the guestOS will not be using the region anymore. However, guest addresses from the depopulated region need to also be unmapped from the qemu address space using cpu_physical_memory_unmap(). Does memory_region_del_subregion() or some other memory API call guarantee that a memoryregion has been unmapped from qemu's address space? - What is the expected behaviour of hotplugged memory after a reboot? Is it supposed to be persistent after reboots? The last 2 patches in the series try to make hotplugged memslots persistent after reboot by creating and consulting e820 map entries. A better solution is needed for hot-remove after a reboot, because e820 entries can be merged. series is based on uq/master for qemu-kvm, and master for seabios. Can be found also at: Vasilis Liaskovitis (9): Seabios: Add SSDT memory device support Seabios, acpi: Implement acpi-dsdt functions for memory hotplug. Seabios, acpi: generate hotplug memory devices. Implement memslot device abstraction acpi_piix4: Implement memory device hotplug registers and handlers. pc: pass paravirt info for hotplug memory slots to BIOS Implement memslot command-line option and memslot hmp monitor command pc: adjust e820 map on hot-add and hot-remove Seabios, acpi: enable memory devices if e820 entry is present Makefile.objs |2 +- hmp-commands.hx | 15 hw/acpi_piix4.c | 103 +++- hw/memslot.c| 201 +++ hw/memslot.h| 44 hw/pc.c | 87 ++-- hw/pc.h |1 + monitor.c |8 ++ monitor.h |1 + qemu-config.c | 25 +++ qemu-options.hx |8 ++ sysemu.h|1 + vl.c| 44 - 13 files changed, 528 insertions(+), 12 deletions(-) create mode 100644 hw/memslot.c create mode 100644 hw/memslot.h create mode 100644 src/ssdt-mem.dsl src/acpi-dsdt.dsl | 68 ++- src/acpi.c| 155 +++-- src/memmap.c | 15 + src/ssdt-mem.dsl | 66 ++ 4 files changed, 298 insertions(+), 6 deletions(-) create mode 100644 src/ssdt-mem.dsl -- 1.7.9 -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH 8/9] pc: adjust e820 map on hot-add and hot-remove
On Thu, Apr 19, 2012 at 04:08:46PM +0200, Vasilis Liaskovitis wrote: Hotplugged memory is not persistent in the e820 memory maps. After hotplugging a memslot and rebooting the VM, the hotplugged device is not present. A possible solution is to add an e820 for the new memslot in the acpi_piix4 hot-add handler. On a reset, Seabios (see next patch in series) will enable all memory devices for which it finds an e820 entry that covers the devices's address range. On hot-remove, the acpi_piix4 handler will try to remove the e820 entry corresponding to the device. This will work when no VM reboots happen between hot-add and hot-remove, but it is not a sufficient solution in general: Seabios and GuestOS merge adjacent e820 entries on machine reboot, so the sequence hot-add/ rebootVM / hot-remove will fail to remove a corresponding e820 entry at the hot-remove phase. Why do you need this path and the next one? Bios can restore the state of memslots and build e820 map by reading mems_sts. Signed-off-by: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com --- hw/acpi_piix4.c |6 ++ hw/pc.c | 28 hw/pc.h |1 + 3 files changed, 35 insertions(+), 0 deletions(-) diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c index 2921d18..2b5fd04 100644 --- a/hw/acpi_piix4.c +++ b/hw/acpi_piix4.c @@ -619,6 +619,9 @@ static void piix4_memslot_eject(uint32_t addr, uint32_t val) s = memslot_find_from_idx(start + idx); assert(s != NULL); memslot_depopulate(s); +if (e820_del_entry(s-start, s-size, E820_RAM) == -EBUSY) +PIIX4_DPRINTF(failed to remove e820 entry for memslot %u\n, + s-idx); } val = val 1; idx++; @@ -634,6 +637,9 @@ static int piix4_memslot_hotplug(DeviceState *qdev, SysBusDevice *dev, int if (add) { enable_mem_device(s, slot-idx); +if (e820_add_entry(slot-start, slot-size, E820_RAM) == -EBUSY) +PIIX4_DPRINTF(failed to add e820 entry for memslot %u\n, +slot-idx); } else { disable_mem_device(s, slot-idx); diff --git a/hw/pc.c b/hw/pc.c index f1f550a..04d243f 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -593,6 +593,34 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type) return index; } +int e820_del_entry(uint64_t address, uint64_t length, uint32_t type) +{ +int index = le32_to_cpu(e820_table.count); +int search; +struct e820_entry *entry; + +if (index == 0) +return -EBUSY; +search = index - 1; +entry = e820_table.entry[search]; +while (search = 0) { +if ((entry-address == cpu_to_le64(address)) +(entry-length == cpu_to_le64(length)) +(entry-type == cpu_to_le32(type))){ +if (search != index - 1) { +memcpy(e820_table.entry[search], e820_table.entry[search + 1], +sizeof(struct e820_entry) * (index - search)); +} +index--; +e820_table.count = cpu_to_le32(index); +return 1; +} +search--; +entry = e820_table.entry[search]; +} +return -EBUSY; +} + static void bochs_bios_setup_hp_memslots(uint64_t *fw_cfg_slots); static void *bochs_bios_init(void) diff --git a/hw/pc.h b/hw/pc.h index 74d3369..4925e8c 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -226,5 +226,6 @@ void pc_system_firmware_init(MemoryRegion *rom_memory); #define E820_UNUSABLE 5 int e820_add_entry(uint64_t, uint64_t, uint32_t); +int e820_del_entry(uint64_t, uint64_t, uint32_t); #endif -- 1.7.9 -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] BEV and bootorder
On Sat, Apr 14, 2012 at 11:19:31PM -0400, Kevin O'Connor wrote: On Thu, Apr 12, 2012 at 01:30:36PM -0600, Steve Goodrich wrote: I'm working towards a goal of having specific devices be bootable, and *only* those devices. For example, if my bootorder file specifies SATA drive 3, I do not want it to try SATA drives 0, 1, and 2, nor any other HDD or floppy that it finds. My first question is: how do I do this? There is no current way to do this. I suppose one could code support for a stop boot option to the boot order file - so that if it was listed in the file the boot would stop after trying all options prior to it. I thought to add skipboot file. If device is in skipboot file it is not considered for booting from. If that can't be answered, can someone explain to me the relationship between the bootorder file and the BEV (Boot Execution Vector) configured in boot.c? All possible boot options (both BEV and BCV) are assembled in a sorted list pointed to by boot.c:BootList. The bootorder file alters the default sort order of that list. During the latter parts of the POST phase, the BCVs are executed and only BEVs remain. The list of BEVs is generated from the BootList. So, in a nutshell, the bootorder file determines the order of the BEVs that SeaBIOS will attempt to boot from. -Kevin ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios