* [PATCH 0/2] x86: hvmloader improvements
@ 2016-06-16 9:33 Jan Beulich
2016-06-16 9:40 ` [PATCH 1/2] hvmloader: limit CPUs exposed to guests Jan Beulich
2016-06-16 9:40 ` [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters Jan Beulich
0 siblings, 2 replies; 15+ messages in thread
From: Jan Beulich @ 2016-06-16 9:33 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
1: limit CPUs exposed to guests
2: don't hard-code IO-APIC parameters
Signed-off-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] hvmloader: limit CPUs exposed to guests
2016-06-16 9:33 [PATCH 0/2] x86: hvmloader improvements Jan Beulich
@ 2016-06-16 9:40 ` Jan Beulich
2016-06-16 15:09 ` Boris Ostrovsky
2016-06-17 9:40 ` Andrew Cooper
2016-06-16 9:40 ` [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters Jan Beulich
1 sibling, 2 replies; 15+ messages in thread
From: Jan Beulich @ 2016-06-16 9:40 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 5168 bytes --]
Various Linux versions allocate (partial) per-CPU data for all of them,
as there is no indication in MADT whether they're hotpluggable. That's
a little wasteful in terms of resource consumption especially for
- guests with not overly much memory assigned,
- 32-bit guests not having overly much address space available.
Therefore limit what we put into MADT to the "maxvcpus" value, and make
sure AML doesn't touch memory addresses corresponding to CPUs beyond
that value (we can't reasonably make the respective processor objects
disappear).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -48,6 +48,7 @@ struct acpi_info {
uint8_t com2_present:1; /* 0[1] - System has COM2? */
uint8_t lpt1_present:1; /* 0[2] - System has LPT1? */
uint8_t hpet_present:1; /* 0[3] - System has HPET? */
+ uint16_t nr_cpus; /* 2 - Number of CPUs */
uint32_t pci_min, pci_len; /* 4, 8 - PCI I/O hole boundaries */
uint32_t madt_csum_addr; /* 12 - Address of MADT checksum */
uint32_t madt_lapic0_addr; /* 16 - Address of first MADT LAPIC struct */
@@ -55,9 +56,6 @@ struct acpi_info {
uint64_t pci_hi_min, pci_hi_len; /* 24, 32 - PCI I/O hole boundaries */
};
-/* Number of processor objects in the chosen DSDT. */
-static unsigned int nr_processor_objects;
-
static void set_checksum(
void *table, uint32_t checksum_offset, uint32_t length)
{
@@ -89,7 +87,7 @@ static struct acpi_20_madt *construct_ma
sz = sizeof(struct acpi_20_madt);
sz += sizeof(struct acpi_20_madt_intsrcovr) * 16;
sz += sizeof(struct acpi_20_madt_ioapic);
- sz += sizeof(struct acpi_20_madt_lapic) * nr_processor_objects;
+ sz += sizeof(struct acpi_20_madt_lapic) * hvm_info->nr_vcpus;
madt = mem_alloc(sz, 16);
if (!madt) return NULL;
@@ -142,8 +140,9 @@ static struct acpi_20_madt *construct_ma
io_apic->ioapic_addr = IOAPIC_BASE_ADDRESS;
lapic = (struct acpi_20_madt_lapic *)(io_apic + 1);
+ info->nr_cpus = hvm_info->nr_vcpus;
info->madt_lapic0_addr = (uint32_t)lapic;
- for ( i = 0; i < nr_processor_objects; i++ )
+ for ( i = 0; i < hvm_info->nr_vcpus; i++ )
{
memset(lapic, 0, sizeof(*lapic));
lapic->type = ACPI_PROCESSOR_LOCAL_APIC;
@@ -151,8 +150,7 @@ static struct acpi_20_madt *construct_ma
/* Processor ID must match processor-object IDs in the DSDT. */
lapic->acpi_processor_id = i;
lapic->apic_id = LAPIC_ID(i);
- lapic->flags = ((i < hvm_info->nr_vcpus) &&
- test_bit(i, hvm_info->vcpu_online)
+ lapic->flags = (test_bit(i, hvm_info->vcpu_online)
? ACPI_LOCAL_APIC_ENABLED : 0);
lapic++;
}
@@ -534,14 +532,12 @@ void acpi_build_tables(struct acpi_confi
dsdt = mem_alloc(config->dsdt_15cpu_len, 16);
if (!dsdt) goto oom;
memcpy(dsdt, config->dsdt_15cpu, config->dsdt_15cpu_len);
- nr_processor_objects = 15;
}
else
{
dsdt = mem_alloc(config->dsdt_anycpu_len, 16);
if (!dsdt) goto oom;
memcpy(dsdt, config->dsdt_anycpu, config->dsdt_anycpu_len);
- nr_processor_objects = HVM_MAX_VCPUS;
}
/*
--- a/tools/firmware/hvmloader/acpi/dsdt.asl
+++ b/tools/firmware/hvmloader/acpi/dsdt.asl
@@ -50,7 +50,8 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2,
UAR2, 1,
LTP1, 1,
HPET, 1,
- Offset(4),
+ Offset(2),
+ NCPU, 16,
PMIN, 32,
PLEN, 32,
MSUA, 32, /* MADT checksum address */
--- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
+++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
@@ -150,6 +150,14 @@ int main(int argc, char **argv)
indent(); printf("MSU, 8\n");
pop_block();
+ /* Processor object helpers. */
+ push_block("Method", "PMAT, 2");
+ push_block("If", "LLess(Arg0, NCPU)");
+ stmt("Return", "ToBuffer(Arg1)");
+ pop_block();
+ stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
+ pop_block();
+
/* Define processor objects and control methods. */
for ( cpu = 0; cpu < max_cpus; cpu++)
{
@@ -171,17 +179,22 @@ int main(int argc, char **argv)
pop_block();
push_block("Method", "_MAT, 0");
- stmt("Return", "ToBuffer(MAT)");
+ if ( cpu )
+ stmt("Return", "PMAT (%d, MAT)", cpu);
+ else
+ stmt("Return", "ToBuffer(MAT)");
pop_block();
push_block("Method", "_STA");
+ if ( cpu )
+ push_block("If", "LLess(%d, \\_SB.NCPU)", cpu);
push_block("If", "FLG");
stmt("Return", "0xF");
pop_block();
- push_block("Else", NULL);
+ if ( cpu )
+ pop_block();
stmt("Return", "0x0");
pop_block();
- pop_block();
push_block("Method", "_EJ0, 1, NotSerialized");
stmt("Sleep", "0xC8");
[-- Attachment #2: hvmloader-limit-CPUs.patch --]
[-- Type: text/plain, Size: 5207 bytes --]
hvmloader: limit CPUs exposed to guests
Various Linux versions allocate (partial) per-CPU data for all of them,
as there is no indication in MADT whether they're hotpluggable. That's
a little wasteful in terms of resource consumption especially for
- guests with not overly much memory assigned,
- 32-bit guests not having overly much address space available.
Therefore limit what we put into MADT to the "maxvcpus" value, and make
sure AML doesn't touch memory addresses corresponding to CPUs beyond
that value (we can't reasonably make the respective processor objects
disappear).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -48,6 +48,7 @@ struct acpi_info {
uint8_t com2_present:1; /* 0[1] - System has COM2? */
uint8_t lpt1_present:1; /* 0[2] - System has LPT1? */
uint8_t hpet_present:1; /* 0[3] - System has HPET? */
+ uint16_t nr_cpus; /* 2 - Number of CPUs */
uint32_t pci_min, pci_len; /* 4, 8 - PCI I/O hole boundaries */
uint32_t madt_csum_addr; /* 12 - Address of MADT checksum */
uint32_t madt_lapic0_addr; /* 16 - Address of first MADT LAPIC struct */
@@ -55,9 +56,6 @@ struct acpi_info {
uint64_t pci_hi_min, pci_hi_len; /* 24, 32 - PCI I/O hole boundaries */
};
-/* Number of processor objects in the chosen DSDT. */
-static unsigned int nr_processor_objects;
-
static void set_checksum(
void *table, uint32_t checksum_offset, uint32_t length)
{
@@ -89,7 +87,7 @@ static struct acpi_20_madt *construct_ma
sz = sizeof(struct acpi_20_madt);
sz += sizeof(struct acpi_20_madt_intsrcovr) * 16;
sz += sizeof(struct acpi_20_madt_ioapic);
- sz += sizeof(struct acpi_20_madt_lapic) * nr_processor_objects;
+ sz += sizeof(struct acpi_20_madt_lapic) * hvm_info->nr_vcpus;
madt = mem_alloc(sz, 16);
if (!madt) return NULL;
@@ -142,8 +140,9 @@ static struct acpi_20_madt *construct_ma
io_apic->ioapic_addr = IOAPIC_BASE_ADDRESS;
lapic = (struct acpi_20_madt_lapic *)(io_apic + 1);
+ info->nr_cpus = hvm_info->nr_vcpus;
info->madt_lapic0_addr = (uint32_t)lapic;
- for ( i = 0; i < nr_processor_objects; i++ )
+ for ( i = 0; i < hvm_info->nr_vcpus; i++ )
{
memset(lapic, 0, sizeof(*lapic));
lapic->type = ACPI_PROCESSOR_LOCAL_APIC;
@@ -151,8 +150,7 @@ static struct acpi_20_madt *construct_ma
/* Processor ID must match processor-object IDs in the DSDT. */
lapic->acpi_processor_id = i;
lapic->apic_id = LAPIC_ID(i);
- lapic->flags = ((i < hvm_info->nr_vcpus) &&
- test_bit(i, hvm_info->vcpu_online)
+ lapic->flags = (test_bit(i, hvm_info->vcpu_online)
? ACPI_LOCAL_APIC_ENABLED : 0);
lapic++;
}
@@ -534,14 +532,12 @@ void acpi_build_tables(struct acpi_confi
dsdt = mem_alloc(config->dsdt_15cpu_len, 16);
if (!dsdt) goto oom;
memcpy(dsdt, config->dsdt_15cpu, config->dsdt_15cpu_len);
- nr_processor_objects = 15;
}
else
{
dsdt = mem_alloc(config->dsdt_anycpu_len, 16);
if (!dsdt) goto oom;
memcpy(dsdt, config->dsdt_anycpu, config->dsdt_anycpu_len);
- nr_processor_objects = HVM_MAX_VCPUS;
}
/*
--- a/tools/firmware/hvmloader/acpi/dsdt.asl
+++ b/tools/firmware/hvmloader/acpi/dsdt.asl
@@ -50,7 +50,8 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2,
UAR2, 1,
LTP1, 1,
HPET, 1,
- Offset(4),
+ Offset(2),
+ NCPU, 16,
PMIN, 32,
PLEN, 32,
MSUA, 32, /* MADT checksum address */
--- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
+++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
@@ -150,6 +150,14 @@ int main(int argc, char **argv)
indent(); printf("MSU, 8\n");
pop_block();
+ /* Processor object helpers. */
+ push_block("Method", "PMAT, 2");
+ push_block("If", "LLess(Arg0, NCPU)");
+ stmt("Return", "ToBuffer(Arg1)");
+ pop_block();
+ stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
+ pop_block();
+
/* Define processor objects and control methods. */
for ( cpu = 0; cpu < max_cpus; cpu++)
{
@@ -171,17 +179,22 @@ int main(int argc, char **argv)
pop_block();
push_block("Method", "_MAT, 0");
- stmt("Return", "ToBuffer(MAT)");
+ if ( cpu )
+ stmt("Return", "PMAT (%d, MAT)", cpu);
+ else
+ stmt("Return", "ToBuffer(MAT)");
pop_block();
push_block("Method", "_STA");
+ if ( cpu )
+ push_block("If", "LLess(%d, \\_SB.NCPU)", cpu);
push_block("If", "FLG");
stmt("Return", "0xF");
pop_block();
- push_block("Else", NULL);
+ if ( cpu )
+ pop_block();
stmt("Return", "0x0");
pop_block();
- pop_block();
push_block("Method", "_EJ0, 1, NotSerialized");
stmt("Sleep", "0xC8");
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters
2016-06-16 9:33 [PATCH 0/2] x86: hvmloader improvements Jan Beulich
2016-06-16 9:40 ` [PATCH 1/2] hvmloader: limit CPUs exposed to guests Jan Beulich
@ 2016-06-16 9:40 ` Jan Beulich
2016-06-16 16:49 ` Boris Ostrovsky
2016-06-17 10:05 ` Andrew Cooper
1 sibling, 2 replies; 15+ messages in thread
From: Jan Beulich @ 2016-06-16 9:40 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 3222 bytes --]
The IO-APIC address has variable bits determined by the PCI-to-ISA
bridge, and the IO-APIC version should be read from the IO-APIC. (Note
that there's still implicit rather than explicit agreement on the
IO-APIC base address between qemu and the hypervisor.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -138,7 +138,7 @@ static struct acpi_20_madt *construct_ma
io_apic->type = ACPI_IO_APIC;
io_apic->length = sizeof(*io_apic);
io_apic->ioapic_id = IOAPIC_ID;
- io_apic->ioapic_addr = IOAPIC_BASE_ADDRESS;
+ io_apic->ioapic_addr = ioapic_base_address;
lapic = (struct acpi_20_madt_lapic *)(io_apic + 1);
info->nr_cpus = hvm_info->nr_vcpus;
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -42,9 +42,10 @@ extern struct bios_config ovmf_config;
#define PAGE_SHIFT 12
#define PAGE_SIZE (1ul << PAGE_SHIFT)
-#define IOAPIC_BASE_ADDRESS 0xfec00000
+extern uint32_t ioapic_base_address;
+extern uint8_t ioapic_version;
+
#define IOAPIC_ID 0x01
-#define IOAPIC_VERSION 0x11
#define LAPIC_BASE_ADDRESS 0xfee00000
#define LAPIC_ID(vcpu_id) ((vcpu_id) * 2)
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -108,6 +108,9 @@ asm (
unsigned long scratch_start = SCRATCH_PHYSICAL_ADDRESS;
+uint32_t ioapic_base_address = 0xfec00000;
+uint8_t ioapic_version;
+
static void init_hypercalls(void)
{
uint32_t eax, ebx, ecx, edx;
@@ -185,6 +188,9 @@ static void init_vm86_tss(void)
static void apic_setup(void)
{
+ ioapic_base_address |= (pci_readb(PCI_ISA_DEVFN, 0x80) & 0x3f) << 10;
+ ioapic_version = ioapic_read(0x01) & 0xff;
+
/* Set the IOAPIC ID to the static value used in the MP/ACPI tables. */
ioapic_write(0x00, IOAPIC_ID);
--- a/tools/firmware/hvmloader/mp_tables.c
+++ b/tools/firmware/hvmloader/mp_tables.c
@@ -227,9 +227,9 @@ static void fill_mp_ioapic_entry(struct
{
mpie->type = ENTRY_TYPE_IOAPIC;
mpie->ioapic_id = IOAPIC_ID;
- mpie->ioapic_version = IOAPIC_VERSION;
+ mpie->ioapic_version = ioapic_version;
mpie->ioapic_flags = 1; /* enabled */
- mpie->ioapic_addr = IOAPIC_BASE_ADDRESS;
+ mpie->ioapic_addr = ioapic_base_address;
}
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -490,14 +490,14 @@ void *scratch_alloc(uint32_t size, uint3
uint32_t ioapic_read(uint32_t reg)
{
- *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x00) = reg;
- return *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x10);
+ *(volatile uint32_t *)(ioapic_base_address + 0x00) = reg;
+ return *(volatile uint32_t *)(ioapic_base_address + 0x10);
}
void ioapic_write(uint32_t reg, uint32_t val)
{
- *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x00) = reg;
- *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x10) = val;
+ *(volatile uint32_t *)(ioapic_base_address + 0x00) = reg;
+ *(volatile uint32_t *)(ioapic_base_address + 0x10) = val;
}
uint32_t lapic_read(uint32_t reg)
[-- Attachment #2: hvmloader-IOAPIC-settings.patch --]
[-- Type: text/plain, Size: 3265 bytes --]
hvmloader: don't hard-code IO-APIC parameters
The IO-APIC address has variable bits determined by the PCI-to-ISA
bridge, and the IO-APIC version should be read from the IO-APIC. (Note
that there's still implicit rather than explicit agreement on the
IO-APIC base address between qemu and the hypervisor.)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -138,7 +138,7 @@ static struct acpi_20_madt *construct_ma
io_apic->type = ACPI_IO_APIC;
io_apic->length = sizeof(*io_apic);
io_apic->ioapic_id = IOAPIC_ID;
- io_apic->ioapic_addr = IOAPIC_BASE_ADDRESS;
+ io_apic->ioapic_addr = ioapic_base_address;
lapic = (struct acpi_20_madt_lapic *)(io_apic + 1);
info->nr_cpus = hvm_info->nr_vcpus;
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -42,9 +42,10 @@ extern struct bios_config ovmf_config;
#define PAGE_SHIFT 12
#define PAGE_SIZE (1ul << PAGE_SHIFT)
-#define IOAPIC_BASE_ADDRESS 0xfec00000
+extern uint32_t ioapic_base_address;
+extern uint8_t ioapic_version;
+
#define IOAPIC_ID 0x01
-#define IOAPIC_VERSION 0x11
#define LAPIC_BASE_ADDRESS 0xfee00000
#define LAPIC_ID(vcpu_id) ((vcpu_id) * 2)
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -108,6 +108,9 @@ asm (
unsigned long scratch_start = SCRATCH_PHYSICAL_ADDRESS;
+uint32_t ioapic_base_address = 0xfec00000;
+uint8_t ioapic_version;
+
static void init_hypercalls(void)
{
uint32_t eax, ebx, ecx, edx;
@@ -185,6 +188,9 @@ static void init_vm86_tss(void)
static void apic_setup(void)
{
+ ioapic_base_address |= (pci_readb(PCI_ISA_DEVFN, 0x80) & 0x3f) << 10;
+ ioapic_version = ioapic_read(0x01) & 0xff;
+
/* Set the IOAPIC ID to the static value used in the MP/ACPI tables. */
ioapic_write(0x00, IOAPIC_ID);
--- a/tools/firmware/hvmloader/mp_tables.c
+++ b/tools/firmware/hvmloader/mp_tables.c
@@ -227,9 +227,9 @@ static void fill_mp_ioapic_entry(struct
{
mpie->type = ENTRY_TYPE_IOAPIC;
mpie->ioapic_id = IOAPIC_ID;
- mpie->ioapic_version = IOAPIC_VERSION;
+ mpie->ioapic_version = ioapic_version;
mpie->ioapic_flags = 1; /* enabled */
- mpie->ioapic_addr = IOAPIC_BASE_ADDRESS;
+ mpie->ioapic_addr = ioapic_base_address;
}
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -490,14 +490,14 @@ void *scratch_alloc(uint32_t size, uint3
uint32_t ioapic_read(uint32_t reg)
{
- *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x00) = reg;
- return *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x10);
+ *(volatile uint32_t *)(ioapic_base_address + 0x00) = reg;
+ return *(volatile uint32_t *)(ioapic_base_address + 0x10);
}
void ioapic_write(uint32_t reg, uint32_t val)
{
- *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x00) = reg;
- *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x10) = val;
+ *(volatile uint32_t *)(ioapic_base_address + 0x00) = reg;
+ *(volatile uint32_t *)(ioapic_base_address + 0x10) = val;
}
uint32_t lapic_read(uint32_t reg)
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] hvmloader: limit CPUs exposed to guests
2016-06-16 9:40 ` [PATCH 1/2] hvmloader: limit CPUs exposed to guests Jan Beulich
@ 2016-06-16 15:09 ` Boris Ostrovsky
2016-06-16 15:25 ` Jan Beulich
2016-06-17 9:40 ` Andrew Cooper
1 sibling, 1 reply; 15+ messages in thread
From: Boris Ostrovsky @ 2016-06-16 15:09 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Andrew Cooper
On 06/16/2016 05:40 AM, Jan Beulich wrote:
--- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
+++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
@@ -150,6 +150,14 @@ int main(int argc, char **argv)
indent(); printf("MSU, 8\n");
pop_block();
+ /* Processor object helpers. */
+ push_block("Method", "PMAT, 2");
+ push_block("If", "LLess(Arg0, NCPU)");
+ stmt("Return", "ToBuffer(Arg1)");
+ pop_block();
+ stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
Could you explain what this is? (I suspect this is related to MAT
object and I don't think I understand what it is).
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] hvmloader: limit CPUs exposed to guests
2016-06-16 15:09 ` Boris Ostrovsky
@ 2016-06-16 15:25 ` Jan Beulich
2016-06-16 15:37 ` Boris Ostrovsky
0 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2016-06-16 15:25 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: Andrew Cooper, xen-devel
>>> On 16.06.16 at 17:09, <boris.ostrovsky@oracle.com> wrote:
> On 06/16/2016 05:40 AM, Jan Beulich wrote:
>
> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
> @@ -150,6 +150,14 @@ int main(int argc, char **argv)
> indent(); printf("MSU, 8\n");
> pop_block();
>
> + /* Processor object helpers. */
> + push_block("Method", "PMAT, 2");
> + push_block("If", "LLess(Arg0, NCPU)");
> + stmt("Return", "ToBuffer(Arg1)");
> + pop_block();
> + stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
>
>
> Could you explain what this is? (I suspect this is related to MAT
> object and I don't think I understand what it is).
This is a helper routine for _MAT(), helping greatly to reduce
overall size of the DSDT. It checks whether the CPU is within the
range of available ones (online or offline), and if it isn't returns a
static buffer instead of data read from MADT (as it's the purpose
of this patch to remove these MADT entries for not present CPUs).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] hvmloader: limit CPUs exposed to guests
2016-06-16 15:25 ` Jan Beulich
@ 2016-06-16 15:37 ` Boris Ostrovsky
2016-06-16 16:04 ` Jan Beulich
0 siblings, 1 reply; 15+ messages in thread
From: Boris Ostrovsky @ 2016-06-16 15:37 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, xen-devel
On 06/16/2016 11:25 AM, Jan Beulich wrote:
>>>> On 16.06.16 at 17:09, <boris.ostrovsky@oracle.com> wrote:
>> On 06/16/2016 05:40 AM, Jan Beulich wrote:
>>
>> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
>> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
>> @@ -150,6 +150,14 @@ int main(int argc, char **argv)
>> indent(); printf("MSU, 8\n");
>> pop_block();
>>
>> + /* Processor object helpers. */
>> + push_block("Method", "PMAT, 2");
>> + push_block("If", "LLess(Arg0, NCPU)");
>> + stmt("Return", "ToBuffer(Arg1)");
>> + pop_block();
>> + stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
>>
>>
>> Could you explain what this is? (I suspect this is related to MAT
>> object and I don't think I understand what it is).
> This is a helper routine for _MAT(), helping greatly to reduce
> overall size of the DSDT. It checks whether the CPU is within the
> range of available ones (online or offline), and if it isn't returns a
> static buffer instead of data read from MADT (as it's the purpose
> of this patch to remove these MADT entries for not present CPUs).
I meant just the last line (I understand what the routine is --- I am
not clear where MAT format is defined).
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] hvmloader: limit CPUs exposed to guests
2016-06-16 15:37 ` Boris Ostrovsky
@ 2016-06-16 16:04 ` Jan Beulich
2016-06-16 16:38 ` Boris Ostrovsky
0 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2016-06-16 16:04 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: Andrew Cooper, xen-devel
>>> On 16.06.16 at 17:37, <boris.ostrovsky@oracle.com> wrote:
> On 06/16/2016 11:25 AM, Jan Beulich wrote:
>>>>> On 16.06.16 at 17:09, <boris.ostrovsky@oracle.com> wrote:
>>> On 06/16/2016 05:40 AM, Jan Beulich wrote:
>>>
>>> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
>>> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
>>> @@ -150,6 +150,14 @@ int main(int argc, char **argv)
>>> indent(); printf("MSU, 8\n");
>>> pop_block();
>>>
>>> + /* Processor object helpers. */
>>> + push_block("Method", "PMAT, 2");
>>> + push_block("If", "LLess(Arg0, NCPU)");
>>> + stmt("Return", "ToBuffer(Arg1)");
>>> + pop_block();
>>> + stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
>>>
>>>
>>> Could you explain what this is? (I suspect this is related to MAT
>>> object and I don't think I understand what it is).
>> This is a helper routine for _MAT(), helping greatly to reduce
>> overall size of the DSDT. It checks whether the CPU is within the
>> range of available ones (online or offline), and if it isn't returns a
>> static buffer instead of data read from MADT (as it's the purpose
>> of this patch to remove these MADT entries for not present CPUs).
>
> I meant just the last line (I understand what the routine is --- I am
> not clear where MAT format is defined).
That's a MADT entry of type ACPI_MADT_TYPE_LOCAL_APIC. Basically
an equivalent of the ToBuffer(MAT) used previously (and now still used
for CPU0).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] hvmloader: limit CPUs exposed to guests
2016-06-16 16:04 ` Jan Beulich
@ 2016-06-16 16:38 ` Boris Ostrovsky
0 siblings, 0 replies; 15+ messages in thread
From: Boris Ostrovsky @ 2016-06-16 16:38 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, xen-devel
On 06/16/2016 12:04 PM, Jan Beulich wrote:
>>>> On 16.06.16 at 17:37, <boris.ostrovsky@oracle.com> wrote:
>> On 06/16/2016 11:25 AM, Jan Beulich wrote:
>>>>>> On 16.06.16 at 17:09, <boris.ostrovsky@oracle.com> wrote:
>>>> On 06/16/2016 05:40 AM, Jan Beulich wrote:
>>>>
>>>> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
>>>> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
>>>> @@ -150,6 +150,14 @@ int main(int argc, char **argv)
>>>> indent(); printf("MSU, 8\n");
>>>> pop_block();
>>>>
>>>> + /* Processor object helpers. */
>>>> + push_block("Method", "PMAT, 2");
>>>> + push_block("If", "LLess(Arg0, NCPU)");
>>>> + stmt("Return", "ToBuffer(Arg1)");
>>>> + pop_block();
>>>> + stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
>>>>
>>>>
>>>> Could you explain what this is? (I suspect this is related to MAT
>>>> object and I don't think I understand what it is).
>>> This is a helper routine for _MAT(), helping greatly to reduce
>>> overall size of the DSDT. It checks whether the CPU is within the
>>> range of available ones (online or offline), and if it isn't returns a
>>> static buffer instead of data read from MADT (as it's the purpose
>>> of this patch to remove these MADT entries for not present CPUs).
>> I meant just the last line (I understand what the routine is --- I am
>> not clear where MAT format is defined).
> That's a MADT entry of type ACPI_MADT_TYPE_LOCAL_APIC. Basically
> an equivalent of the ToBuffer(MAT) used previously (and now still used
> for CPU0).
Ah, ok -- makes sense now.
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters
2016-06-16 9:40 ` [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters Jan Beulich
@ 2016-06-16 16:49 ` Boris Ostrovsky
2016-06-17 6:00 ` Jan Beulich
2016-06-17 10:05 ` Andrew Cooper
1 sibling, 1 reply; 15+ messages in thread
From: Boris Ostrovsky @ 2016-06-16 16:49 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Andrew Cooper
On 06/16/2016 05:40 AM, Jan Beulich wrote:
> The IO-APIC address has variable bits determined by the PCI-to-ISA
> bridge, and the IO-APIC version should be read from the IO-APIC. (Note
> that there's still implicit rather than explicit agreement on the
> IO-APIC base address between qemu and the hypervisor.)
It probably doesn't matter now for PVHv2 since we are not going to
initially emulate IOAPIC but when/if we do, how do we query for base
address and version? Should we just rely on the same implicit agreement?
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters
2016-06-16 16:49 ` Boris Ostrovsky
@ 2016-06-17 6:00 ` Jan Beulich
2016-06-17 11:51 ` Boris Ostrovsky
0 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2016-06-17 6:00 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: Andrew Cooper, xen-devel
>>> On 16.06.16 at 18:49, <boris.ostrovsky@oracle.com> wrote:
> On 06/16/2016 05:40 AM, Jan Beulich wrote:
>> The IO-APIC address has variable bits determined by the PCI-to-ISA
>> bridge, and the IO-APIC version should be read from the IO-APIC. (Note
>> that there's still implicit rather than explicit agreement on the
>> IO-APIC base address between qemu and the hypervisor.)
>
> It probably doesn't matter now for PVHv2 since we are not going to
> initially emulate IOAPIC but when/if we do, how do we query for base
> address and version? Should we just rely on the same implicit agreement?
If this refers to the sentence in parentheses, then this was really
meant to indicate a work item. I.e. qemu should negotiate with
Xen. For PVHv2 this would then probably mean for hvmloader to
query Xen (via new hypercall).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] hvmloader: limit CPUs exposed to guests
2016-06-16 9:40 ` [PATCH 1/2] hvmloader: limit CPUs exposed to guests Jan Beulich
2016-06-16 15:09 ` Boris Ostrovsky
@ 2016-06-17 9:40 ` Andrew Cooper
1 sibling, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2016-06-17 9:40 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 16/06/16 10:40, Jan Beulich wrote:
> Various Linux versions allocate (partial) per-CPU data for all of them,
> as there is no indication in MADT whether they're hotpluggable. That's
> a little wasteful in terms of resource consumption especially for
> - guests with not overly much memory assigned,
> - 32-bit guests not having overly much address space available.
> Therefore limit what we put into MADT to the "maxvcpus" value, and make
> sure AML doesn't touch memory addresses corresponding to CPUs beyond
> that value (we can't reasonably make the respective processor objects
> disappear).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
I don't feel that I know AML well enough to review, but the C looks ok
and this seems like a sensible change.
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters
2016-06-16 9:40 ` [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters Jan Beulich
2016-06-16 16:49 ` Boris Ostrovsky
@ 2016-06-17 10:05 ` Andrew Cooper
2016-06-17 10:23 ` Jan Beulich
1 sibling, 1 reply; 15+ messages in thread
From: Andrew Cooper @ 2016-06-17 10:05 UTC (permalink / raw)
To: Jan Beulich, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 711 bytes --]
On 16/06/16 10:40, Jan Beulich wrote:
> The IO-APIC address has variable bits determined by the PCI-to-ISA
> bridge, and the IO-APIC version should be read from the IO-APIC. (Note
> that there's still implicit rather than explicit agreement on the
> IO-APIC base address between qemu and the hypervisor.)
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
The status quo is not great, and I can see why you want to improve it.
However, I think that this is not the way to do that. It ties HVMLoader
to the PIIX4 board in Qemu, and will break attempts to use Q35 or
something else. (In Q35, the IO-APIC decode address comes from Chipset
Configuration Register, rather than ISA device config space).
~Andrew
[-- Attachment #1.2: Type: text/html, Size: 1206 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters
2016-06-17 10:05 ` Andrew Cooper
@ 2016-06-17 10:23 ` Jan Beulich
0 siblings, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2016-06-17 10:23 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
>>> On 17.06.16 at 12:05, <andrew.cooper3@citrix.com> wrote:
> On 16/06/16 10:40, Jan Beulich wrote:
>> The IO-APIC address has variable bits determined by the PCI-to-ISA
>> bridge, and the IO-APIC version should be read from the IO-APIC. (Note
>> that there's still implicit rather than explicit agreement on the
>> IO-APIC base address between qemu and the hypervisor.)
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> The status quo is not great, and I can see why you want to improve it.
>
> However, I think that this is not the way to do that. It ties HVMLoader
> to the PIIX4 board in Qemu, and will break attempts to use Q35 or
> something else. (In Q35, the IO-APIC decode address comes from Chipset
> Configuration Register, rather than ISA device config space).
hvmloader is already tied to PIIX4, and enabling Q35 will, among
other things, require hvmloader to become aware of that chipset.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters
2016-06-17 6:00 ` Jan Beulich
@ 2016-06-17 11:51 ` Boris Ostrovsky
2016-06-17 14:26 ` Jan Beulich
0 siblings, 1 reply; 15+ messages in thread
From: Boris Ostrovsky @ 2016-06-17 11:51 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, xen-devel
On 06/17/2016 02:00 AM, Jan Beulich wrote:
>>>> On 16.06.16 at 18:49, <boris.ostrovsky@oracle.com> wrote:
>> On 06/16/2016 05:40 AM, Jan Beulich wrote:
>>> The IO-APIC address has variable bits determined by the PCI-to-ISA
>>> bridge, and the IO-APIC version should be read from the IO-APIC. (Note
>>> that there's still implicit rather than explicit agreement on the
>>> IO-APIC base address between qemu and the hypervisor.)
>> It probably doesn't matter now for PVHv2 since we are not going to
>> initially emulate IOAPIC but when/if we do, how do we query for base
>> address and version? Should we just rely on the same implicit agreement?
> If this refers to the sentence in parentheses, then this was really
> meant to indicate a work item. I.e. qemu should negotiate with
> Xen. For PVHv2 this would then probably mean for hvmloader to
> query Xen (via new hypercall).
We are not using hvmloader for PVH so I am not sure how that would work.
You mean the toolstack?
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters
2016-06-17 11:51 ` Boris Ostrovsky
@ 2016-06-17 14:26 ` Jan Beulich
0 siblings, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2016-06-17 14:26 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: Andrew Cooper, xen-devel
>>> On 17.06.16 at 13:51, <boris.ostrovsky@oracle.com> wrote:
> On 06/17/2016 02:00 AM, Jan Beulich wrote:
>>>>> On 16.06.16 at 18:49, <boris.ostrovsky@oracle.com> wrote:
>>> On 06/16/2016 05:40 AM, Jan Beulich wrote:
>>>> The IO-APIC address has variable bits determined by the PCI-to-ISA
>>>> bridge, and the IO-APIC version should be read from the IO-APIC. (Note
>>>> that there's still implicit rather than explicit agreement on the
>>>> IO-APIC base address between qemu and the hypervisor.)
>>> It probably doesn't matter now for PVHv2 since we are not going to
>>> initially emulate IOAPIC but when/if we do, how do we query for base
>>> address and version? Should we just rely on the same implicit agreement?
>> If this refers to the sentence in parentheses, then this was really
>> meant to indicate a work item. I.e. qemu should negotiate with
>> Xen. For PVHv2 this would then probably mean for hvmloader to
>> query Xen (via new hypercall).
>
> We are not using hvmloader for PVH so I am not sure how that would work.
> You mean the toolstack?
Oh, right. Whoever puts the ACPI tables in place.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-06-17 14:26 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-16 9:33 [PATCH 0/2] x86: hvmloader improvements Jan Beulich
2016-06-16 9:40 ` [PATCH 1/2] hvmloader: limit CPUs exposed to guests Jan Beulich
2016-06-16 15:09 ` Boris Ostrovsky
2016-06-16 15:25 ` Jan Beulich
2016-06-16 15:37 ` Boris Ostrovsky
2016-06-16 16:04 ` Jan Beulich
2016-06-16 16:38 ` Boris Ostrovsky
2016-06-17 9:40 ` Andrew Cooper
2016-06-16 9:40 ` [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters Jan Beulich
2016-06-16 16:49 ` Boris Ostrovsky
2016-06-17 6:00 ` Jan Beulich
2016-06-17 11:51 ` Boris Ostrovsky
2016-06-17 14:26 ` Jan Beulich
2016-06-17 10:05 ` Andrew Cooper
2016-06-17 10:23 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).