On Sat, Jan 30, 2010 at 12:52:54AM +0800, Liu, Jinsong wrote:
> Gleb Natapov wrote:
> > On Fri, Jan 29, 2010 at 11:34:24PM +0800, Liu, Jinsong wrote:
> >> Gleb Natapov wrote:
> >>> On Thu, Jan 28, 2010 at 10:54:48PM +0800, Liu, Jinsong wrote:
> >>>> Connor and Gleb,
> >>>>
> >>>> I updated my patch according to our discussion:
> >>>> 1. simplify scan loop according to 'maxvcpus';
> >>>> 2. remove unecessary global variables;
> >>>> 3. change hardcode address to bios use only area '0x514';
> >>>> 4. remove simple \_PR scope from ssdt;
> >>>>
> >>> I still don't see that ACPI NVS issue is addressed.
> >>
> >> Gleb,
> >>
> >> I'm not quite sure Win7 BSOD issue you mentioned last email, anyway
> >> I update my patch, define ACPI NVS per my understanding.
> > You need checked build of Windows 7. This is not regular Win7 this is
> > debug version that you can download from MSDN. Here is the bug report
> > about the crash:
> > http://sourceforge.net/tracker/?func=detail&aid=2902983&group_id=180599&ati…
> >
> > Anyway according to ACPI spec ACPI DATA area can be reused by an OS
> > after reading ACPI tables so it is wrong to access this memory after
> > an OS boot.
> >
>
> Yes, it's safer to define ACPI NVS, although it's bios use only area.
>
APCI DATA is explicitly not defined by ACPI spec as "bios use only
area".
--
Gleb.
On Fri, Jan 29, 2010 at 11:34:24PM +0800, Liu, Jinsong wrote:
> Gleb Natapov wrote:
> > On Thu, Jan 28, 2010 at 10:54:48PM +0800, Liu, Jinsong wrote:
> >> Connor and Gleb,
> >>
> >> I updated my patch according to our discussion:
> >> 1. simplify scan loop according to 'maxvcpus';
> >> 2. remove unecessary global variables;
> >> 3. change hardcode address to bios use only area '0x514';
> >> 4. remove simple \_PR scope from ssdt;
> >>
> > I still don't see that ACPI NVS issue is addressed.
>
> Gleb,
>
> I'm not quite sure Win7 BSOD issue you mentioned last email, anyway I update my patch, define ACPI NVS per my understanding.
You need checked build of Windows 7. This is not regular Win7 this is
debug version that you can download from MSDN. Here is the bug report
about the crash:
http://sourceforge.net/tracker/?func=detail&aid=2902983&group_id=180599&ati…
Anyway according to ACPI spec ACPI DATA area can be reused by an OS after
reading ACPI tables so it is wrong to access this memory after an OS boot.
> After test, Win7 guest works fine based on it.
> If anything misunderstanding, you can update my patch per your experience.
>
> Attached are the patches,
> vcpu_hotplug_1.pach is same as my last email sent yesterday;
> vcpu_hotplug_2.patch is to define ACPI NVS;
>
>
> Thanks,
> Jinsong
--
Gleb.
On Thu, Jan 28, 2010 at 10:54:48PM +0800, Liu, Jinsong wrote:
> Connor and Gleb,
>
> I updated my patch according to our discussion:
> 1. simplify scan loop according to 'maxvcpus';
> 2. remove unecessary global variables;
> 3. change hardcode address to bios use only area '0x514';
> 4. remove simple \_PR scope from ssdt;
>
I still don't see that ACPI NVS issue is addressed.
--
Gleb.
On Thu, Jan 28, 2010 at 11:16:17AM +0800, Liu, Jinsong wrote:
> Connor and Gleb,
>
> Thanks for comments and suggestions!
>
> According to the discussion, I will update my patch as:
> 1. simplify method 'PRSC' definiation, move 'maxvcpus' to method 'NTFY' so that code is more direct and easy to understand;
> 2. remove unnecessary global variable 'madt_csum_addr, madt_lapic0_addr, max_cpus_byte, max_cpus_bit';
> 3. use VAR16FIXED() macro for a hardcode address, change hardcode address from '0xEA000' to '0x514' (same as old BOCHS);
VAR16FIXED() is for addresses in the f-segment - for addresses outside
of the bios code, one can just assign a value to the address. I don't
know if 0x514 is used for anything else, but I suppose it's okay if
BOCHS got away with it.
-Kevin
On Thu, Jan 28, 2010 at 11:16:17AM +0800, Liu, Jinsong wrote:
> Connor and Gleb,
>
> Thanks for comments and suggestions!
>
> According to the discussion, I will update my patch as:
> 1. simplify method 'PRSC' definiation, move 'maxvcpus' to method 'NTFY' so that code is more direct and easy to understand;
> 2. remove unnecessary global variable 'madt_csum_addr, madt_lapic0_addr, max_cpus_byte, max_cpus_bit';
> 3. use VAR16FIXED() macro for a hardcode address, change hardcode address from '0xEA000' to '0x514' (same as old BOCHS);
>
> Is it OK?
>
The memory that is accessed from DSDT should be of type ACPI NVS in e820
map. Otherwise Windows7 checked build BSODs.
> Thanks,
> Jinsong
>
> Gleb Natapov wrote:
> > On Sat, Jan 23, 2010 at 06:46:18PM -0500, Kevin O'Connor wrote:
> >> On Fri, Jan 22, 2010 at 09:35:55AM +0800, Liu, Jinsong wrote:
> >>> From cb997030cba02e7e74a29b3d942aeba9808ed293 Mon Sep 17 00:00:00
> >>> 2001
> >>> From: Liu, Jinsong <jinsong.liu(a)intel.com>
> >>> Date: Fri, 22 Jan 2010 03:18:46 +0800
> >>> Subject: [PATCH] Setup vcpu add/remove infrastructure,
> >>> including madt bios_info and dsdt.
> >>>
> >>> 1. setup madt bios_info structure, so that static dsdt get
> >>> run-time madt info like checksum address, lapic address,
> >>> max cpu numbers, with least hardcode magic number
> >>> (realmode address of bios_info).
> >>> 2. setup vcpu add/remove dsdt infrastructure, including
> >>> processor related acpi objects and control methods. vcpu
> >>> add/remove will trigger SCI and then control method
> >>> _L02. By matching madt, vcpu number and add/remove
> >>> action were found, then by notify control method, it
> >>> will notify OS acpi driver.
> >>>
> >>> Signed-off-by: Liu, Jinsong <jinsong.liu(a)intel.com>
> >>
> >> Thanks.
> >>
> >> I'd like to see Gleb's comments on the ACPI changes.
> >>
> > I already commented on KVM list. Here is the link
> > http://www.mail-archive.com/kvm@vger.kernel.org/msg27562.html
> >
> >> [...]
> >>> diff --git a/src/acpi.c b/src/acpi.c
> >>> index f613b03..037da46 100644
> >>> --- a/src/acpi.c
> >>> +++ b/src/acpi.c
> >>> @@ -344,6 +344,9 @@ build_fadt(int bdf)
> >>> return fadt;
> >>> }
> >>>
> >>> +u32 madt_csum_addr, madt_lapic0_addr;
> >>> +u32 max_cpus_byte, max_cpus_bit;
> >>> +
> >>> static void*
> >>> build_madt(void)
> >>> {
> >>> @@ -360,6 +363,10 @@ build_madt(void)
> >>> madt->local_apic_address = cpu_to_le32(BUILD_APIC_ADDR);
> >>> madt->flags = cpu_to_le32(1);
> >>> struct madt_processor_apic *apic = (void*)&madt[1];
> >>> + madt_csum_addr = (u32)&madt->checksum;
> >>> + madt_lapic0_addr = (u32)apic;
> >>> + max_cpus_byte = MaxCountCPUs / 8;
> >>> + max_cpus_bit = MaxCountCPUs % 8;
> >>
> >> I'm confused at why global variables are being created, exported, and
> >> set, just to be copied to another location later on.
> >>
> >>> +/*
> >>> ***************************************************************** +
> >>> * use bios_info to enable dsdt get run-time madt and cpu info, + *
> >>> and to avoid more hardcode magic number. +
> >>> *******************************************************************/
> >>> +#define BIOS_INFO_PHYSICAL_ADDRESS 0x000EA000
> >>
> >> That wont work - nothing stops 0xea000 from being used by an option
> >> rom or the bios itself. If a physical address is needed, the
> >> VAR16FIXED() macro needs to be used. However, I'm very reluctant to
> >> add new hardcoded addresses - there has to be a very compelling
> >> reason to do that.
> >>
> >> -Kevin
> >>
> >> _______________________________________________
> >> SeaBIOS mailing list
> >> SeaBIOS(a)seabios.org
> >> http://www.seabios.org/mailman/listinfo/seabios
--
Gleb.
On Fri, Jan 22, 2010 at 09:35:55AM +0800, Liu, Jinsong wrote:
> From cb997030cba02e7e74a29b3d942aeba9808ed293 Mon Sep 17 00:00:00 2001
> From: Liu, Jinsong <jinsong.liu(a)intel.com>
> Date: Fri, 22 Jan 2010 03:18:46 +0800
> Subject: [PATCH] Setup vcpu add/remove infrastructure, including madt bios_info and dsdt.
>
> 1. setup madt bios_info structure, so that static dsdt get
> run-time madt info like checksum address, lapic address,
> max cpu numbers, with least hardcode magic number (realmode
> address of bios_info).
> 2. setup vcpu add/remove dsdt infrastructure, including processor
> related acpi objects and control methods. vcpu add/remove will
> trigger SCI and then control method _L02. By matching madt, vcpu
> number and add/remove action were found, then by notify control
> method, it will notify OS acpi driver.
>
> Signed-off-by: Liu, Jinsong <jinsong.liu(a)intel.com>
Thanks.
I'd like to see Gleb's comments on the ACPI changes.
[...]
> diff --git a/src/acpi.c b/src/acpi.c
> index f613b03..037da46 100644
> --- a/src/acpi.c
> +++ b/src/acpi.c
> @@ -344,6 +344,9 @@ build_fadt(int bdf)
> return fadt;
> }
>
> +u32 madt_csum_addr, madt_lapic0_addr;
> +u32 max_cpus_byte, max_cpus_bit;
> +
> static void*
> build_madt(void)
> {
> @@ -360,6 +363,10 @@ build_madt(void)
> madt->local_apic_address = cpu_to_le32(BUILD_APIC_ADDR);
> madt->flags = cpu_to_le32(1);
> struct madt_processor_apic *apic = (void*)&madt[1];
> + madt_csum_addr = (u32)&madt->checksum;
> + madt_lapic0_addr = (u32)apic;
> + max_cpus_byte = MaxCountCPUs / 8;
> + max_cpus_bit = MaxCountCPUs % 8;
I'm confused at why global variables are being created, exported, and
set, just to be copied to another location later on.
> +/* *****************************************************************
> + * use bios_info to enable dsdt get run-time madt and cpu info,
> + * and to avoid more hardcode magic number.
> + *******************************************************************/
> +#define BIOS_INFO_PHYSICAL_ADDRESS 0x000EA000
That wont work - nothing stops 0xea000 from being used by an option
rom or the bios itself. If a physical address is needed, the
VAR16FIXED() macro needs to be used. However, I'm very reluctant to
add new hardcoded addresses - there has to be a very compelling reason
to do that.
-Kevin
On Sun, Jan 24, 2010 at 05:24:24PM +0100, congedete(a)voila.fr wrote:
> Hello,
>
> It seems Seabios is still running after jumping to the boot sector.
> Why is the application not finalized in call_boot_entry for example
> ? Disk operations are handled until linux or windows boots because
> Seabios is still running which is unnecessary. It slows Grub2 or
> Windows Boot Manager I think.
After jumping to the OS, SeaBIOS only runs when the OS calls it, or
one of its hardware irq handlers runs. This is a requirement for
proper operation.
Why do you think this slows down your machine?
-Kevin
>
> Are you working on such an improvement or is it a known problem ?
>
> Hope this helps.
>
> ____________________________________________________
>
> Je m’évite la cohue dans les magasins et je profite des petits prix sur http://shopping.voila.fr
>
>
>