From: Gleb Natapov <gleb@redhat.com>
To: "Liu, Jinsong" <jinsong.liu@intel.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>, Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH] Setup vcpu add/remove infrastructure, including madt bios_info and dsdt.
Date: Thu, 21 Jan 2010 14:44:26 +0200 [thread overview]
Message-ID: <20100121124426.GT5238@redhat.com> (raw)
In-Reply-To: <EB8593BCECAB3D40A8248BE0B6400A3835909015@shzsmsx502.ccr.corp.intel.com>
On Thu, Jan 21, 2010 at 07:48:23PM +0800, Liu, Jinsong wrote:
> >From cb997030cba02e7e74a29b3d942aeba9808ed293 Mon Sep 17 00:00:00 2001
> From: Liu, Jinsong <jinsong.liu@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@intel.com>
It looks like AML code is a port of what we had in BOCHS bios with minor
changes. Can you detail what is changed and why for easy review please?
And this still doesn't work with Windows I assume.
> ---
> src/acpi-dsdt.dsl | 131 ++++++++++++++++-
> src/acpi-dsdt.hex | 441 ++++++++++++++++++++++++++++++++++++++++++++++++++---
> src/acpi.c | 7 +
> src/biosvar.h | 14 ++
> src/post.c | 13 ++
> 5 files changed, 582 insertions(+), 24 deletions(-)
>
> diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
> index cc31112..ed78489 100644
> --- a/src/acpi-dsdt.dsl
> +++ b/src/acpi-dsdt.dsl
> @@ -700,8 +700,11 @@ DefinitionBlock (
> Return (0x01)
>
> }
> + /*
> + * _L02 method for CPU notification
> + */
> Method(_L02) {
> - Return(0x01)
> + Return(\_PR.PRSC())
> }
> Method(_L03) {
> Return(0x01)
> @@ -744,4 +747,130 @@ DefinitionBlock (
> }
> }
>
> +
> + Scope (\_PR)
> + {
> + /* BIOS_INFO_PHYSICAL_ADDRESS == 0xEA000 */
> + OperationRegion(BIOS, SystemMemory, 0xEA000, 16)
> + Field(BIOS, DwordAcc, NoLock, Preserve)
> + {
> + MSUA, 32, /* MADT checksum address */
> + MAPA, 32, /* MADT LAPIC0 address */
> + PBYT, 32, /* bytes of max vcpus bitmap */
> + PBIT, 32 /* bits of last byte of max vcpus bitmap */
Why do you need PBYT/PBIT? Adds complexity for no apparent reason.
> + }
> +
> + OperationRegion(MSUM, SystemMemory, MSUA, 1)
> + Field(MSUM, ByteAcc, NoLock, Preserve)
> + {
> + MSU, 8 /* MADT checksum */
> + }
> +
> + #define gen_processor(nr, name) \
> + Processor (C##name, nr, 0x0000b010, 0x06) { \
> + Name (_HID, "ACPI0007") \
> + OperationRegion(MATR, SystemMemory, Add(MAPA, Multiply(nr,8)), 8) \
> + Field (MATR, ByteAcc, NoLock, Preserve) \
> + { \
> + MAT, 64 \
> + } \
> + Field (MATR, ByteAcc, NoLock, Preserve) \
> + { \
> + Offset(4), \
> + FLG, 1 \
> + } \
> + Method(_MAT, 0) { \
> + Return(ToBuffer(MAT)) \
> + } \
> + Method (_STA) { \
> + If (FLG) { Return(0xF) } Else { Return(0x9) } \
> + } \
> + Method (_EJ0, 1, NotSerialized) { \
> + Sleep (0xC8) \
> + } \
Why _EJ0 is needed?
> + } \
> +
> + gen_processor(0, 0)
> + gen_processor(1, 1)
> + gen_processor(2, 2)
> + gen_processor(3, 3)
> + gen_processor(4, 4)
> + gen_processor(5, 5)
> + gen_processor(6, 6)
> + gen_processor(7, 7)
> + gen_processor(8, 8)
> + gen_processor(9, 9)
> + gen_processor(10, A)
> + gen_processor(11, B)
> + gen_processor(12, C)
> + gen_processor(13, D)
> + gen_processor(14, E)
> +
> +
> + Method (NTFY, 2) {
> + #define gen_ntfy(nr) \
> + If (LEqual(Arg0, 0x##nr)) { \
> + If (LNotEqual(Arg1, \_PR.C##nr.FLG)) { \
> + Store (Arg1, \_PR.C##nr.FLG) \
> + If (LEqual(Arg1, 1)) { \
> + Notify(C##nr, 1) \
> + Subtract(\_PR.MSU, 1, \_PR.MSU) \
> + } Else { \
> + Notify(C##nr, 3) \
> + Add(\_PR.MSU, 1, \_PR.MSU) \
> + } \
> + } \
> + }
> +
> + gen_ntfy(0)
> + gen_ntfy(1)
> + gen_ntfy(2)
> + gen_ntfy(3)
> + gen_ntfy(4)
> + gen_ntfy(5)
> + gen_ntfy(6)
> + gen_ntfy(7)
> + gen_ntfy(8)
> + gen_ntfy(9)
> + gen_ntfy(A)
> + gen_ntfy(B)
> + gen_ntfy(C)
> + gen_ntfy(D)
> + gen_ntfy(E)
> +
> + Return(One)
> + }
> +
> +
> + OperationRegion(PRST, SystemIO, 0xaf00, 32)
> + Field (PRST, ByteAcc, NoLock, Preserve)
> + {
> + PRS, 256
> + }
> +
> + Method(PRSC, 0) {
> + Store(PRS, Local3)
> + Store(Zero, Local0)
> + While(LLess(Local0, \_PR.PBYT)) {
> + Store(Zero, Local1)
> + Store(DerefOf(Index(Local3, Local0)), Local2)
> + While(LLess(Local1, 8)) {
> + NTFY(Add(Multiply(Local0, 8), Local1), And(Local2, 1))
> + ShiftRight(Local2, 1, Local2)
> + Increment(Local1)
> + }
> + Increment(Local0)
> + }
> +
> + Store(Zero, Local1)
> + Store(DerefOf(Index(Local3, Local0)), Local2)
> + While(LLess(Local1, \_PR.PBIT)) {
> + NTFY(Add(Multiply(Local0, 8), Local1), And(Local2, 1))
> + ShiftRight(Local2, 1, Local2)
> + Increment(Local1)
> + }
> +
> + Return(One)
> + }
> + }
> }
> 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;
The memory MADT resides in will be used from AML code and should be
marked in e820 map as ACPI NVS since ACPI data area can be reused by the
OS after it reads ACPI table.
> + max_cpus_byte = MaxCountCPUs / 8;
> + max_cpus_bit = MaxCountCPUs % 8;
> int i;
> for (i=0; i<MaxCountCPUs; i++) {
> apic->type = APIC_PROCESSOR;
> diff --git a/src/biosvar.h b/src/biosvar.h
> index b6e061b..57dd86b 100644
> --- a/src/biosvar.h
> +++ b/src/biosvar.h
> @@ -11,6 +11,20 @@
> #include "config.h" // CONFIG_*
> #include "disk.h" // struct chs_s
>
> +/* *****************************************************************
> + * 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
How this number is chosen? How do you now the memory is free for use?
And it should be marked as ACPI NVS in e820 as well.
> +struct bios_info {
> + u32 madt_csum_addr; /* address of MADT checksum */
> + u32 madt_lapic0_addr; /* address of first MADT LAPIC struct */
> + u32 max_cpus_byte; /* max cpus bitmap bytes */
> + u32 max_cpus_bit; /* max cpus bitmap bits of last byte */
> +};
> +
> +extern u32 madt_csum_addr, madt_lapic0_addr;
> +extern u32 max_cpus_byte, max_cpus_bit;
>
> /****************************************************************
> * Interupt vector table
> diff --git a/src/post.c b/src/post.c
> index fb3b37f..8e30a57 100644
> --- a/src/post.c
> +++ b/src/post.c
> @@ -161,6 +161,18 @@ init_bios_tables(void)
> acpi_bios_init();
> }
>
> +static void init_bios_info(void)
> +{
> + struct bios_info *bios_info;
> +
> + bios_info = (struct bios_info *)BIOS_INFO_PHYSICAL_ADDRESS;
> + memset(bios_info, 0, sizeof(*bios_info));
> + bios_info->madt_csum_addr = madt_csum_addr;
> + bios_info->madt_lapic0_addr = madt_lapic0_addr;
> + bios_info->max_cpus_byte = max_cpus_byte;
> + bios_info->max_cpus_bit = max_cpus_bit;
> +}
> +
> // Main setup code.
> static void
> post(void)
> @@ -195,6 +207,7 @@ post(void)
> kbd_setup();
> mouse_setup();
> init_bios_tables();
> + init_bios_info();
>
> // Run vga option rom (if running synchronously)
> if (!CONFIG_THREADS || !CONFIG_THREAD_OPTIONROMS)
> --
> 1.6.5.6
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Gleb.
next prev parent reply other threads:[~2010-01-21 12:44 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-21 11:48 [PATCH] Setup vcpu add/remove infrastructure, including madt bios_info and dsdt Liu, Jinsong
2010-01-21 12:44 ` Gleb Natapov [this message]
2010-01-22 2:15 ` Liu, Jinsong
2010-01-22 5:45 ` Gleb Natapov
2010-01-22 9:08 ` Liu, Jinsong
2010-01-22 10:29 ` Gleb Natapov
2010-01-22 10:54 ` Liu, Jinsong
2010-01-22 11:19 ` Gleb Natapov
2010-01-22 14:52 ` Liu, Jinsong
2010-01-22 15:25 ` Gleb Natapov
2010-01-22 15:56 ` Liu, Jinsong
2010-01-22 16:02 ` Gleb Natapov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100121124426.GT5238@redhat.com \
--to=gleb@redhat.com \
--cc=avi@redhat.com \
--cc=jinsong.liu@intel.com \
--cc=kvm@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox