[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.1 2/2] i386/acpi-build: support hotplug of
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH for-2.1 2/2] i386/acpi-build: support hotplug of VCPU with APIC ID 0xFF |
Date: |
Mon, 17 Mar 2014 11:08:46 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
On 03/16/14 13:31, Michael S. Tsirkin wrote:
> On Fri, Mar 14, 2014 at 11:22:52PM +0100, Laszlo Ersek wrote:
>> Building on the previous patch, raise the maximal count of processor
>> objects / NTFY branches / CPON elements from 255 to 256. This allows the
>> VCPU with APIC ID 0xFF to be hotplugged.
>>
>> Signed-off-by: Laszlo Ersek <address@hidden>
>> ---
>> hw/i386/acpi-build.c | 16 +++++++++++-----
>> 1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 2bbefb5..51162fc 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -999,11 +999,15 @@ build_ssdt(GArray *table_data, GArray *linker,
>> AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
>> PcPciInfo *pci, PcGuestInfo *guest_info)
>> {
>> - int acpi_cpus = MIN(0xff, guest_info->apic_id_limit);
>
> Maybe just make this line
> int acpi_cpus = guest_info->apic_id_limit;
> and then the patch will be smaller.
I did think of that, but I didn't like the unchecked unsigned --> int
conversion. The limit checks below cover that too.
Also, the patch renders the function similar to the other acpi builder
functions; there are a handful of loops that directly name
"guest_info->apic_id_limit" in their controlling expressions.
Anyway, would you be OK with a patch that did the assignment you suggest
(rather than modifying the loops), and kept the BUILD_BUG and the
g_assert below?
Thanks
Laszlo
>
>> int ssdt_start = table_data->len;
>> uint8_t *ssdt_ptr;
>> int i;
>>
>> + /* The current AML generator can cover the APIC ID range [0..255],
>> + * inclusive, for VCPU hotplug. */
>> + QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
>> + g_assert(guest_info->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
>> +
>> /* Copy header and patch values in the S3_ / S4_ / S5_ packages */
>> ssdt_ptr = acpi_data_push(table_data, sizeof(ssdp_misc_aml));
>> memcpy(ssdt_ptr, ssdp_misc_aml, sizeof(ssdp_misc_aml));
>> @@ -1029,7 +1033,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>> build_append_nameseg(sb_scope, "_SB_");
>>
>> /* build Processor object for each processor */
>> - for (i = 0; i < acpi_cpus; i++) {
>> + for (i = 0; i < guest_info->apic_id_limit; i++) {
>> uint8_t *proc = acpi_data_push(sb_scope, ACPI_PROC_SIZEOF);
>> memcpy(proc, ACPI_PROC_AML, ACPI_PROC_SIZEOF);
>> proc[ACPI_PROC_OFFSET_CPUHEX] = acpi_get_hex(i >> 4);
>> @@ -1042,7 +1046,8 @@ build_ssdt(GArray *table_data, GArray *linker,
>> * Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)}
>> ...}
>> */
>> /* Arg0 = Processor ID = APIC ID */
>> - build_append_notify_method(sb_scope, "NTFY", "CP%0.02X", acpi_cpus);
>> + build_append_notify_method(sb_scope, "NTFY", "CP%0.02X",
>> + guest_info->apic_id_limit);
>>
>> /* build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })"
>> */
>> build_append_byte(sb_scope, 0x08); /* NameOp */
>> @@ -1052,8 +1057,9 @@ build_ssdt(GArray *table_data, GArray *linker,
>> GArray *package = build_alloc_array();
>> uint8_t op = 0x13; /* VarPackageOp */
>>
>> - build_append_int(package, acpi_cpus); /* VarNumElements */
>> - for (i = 0; i < acpi_cpus; i++) {
>> + build_append_int(package,
>> + guest_info->apic_id_limit); /* VarNumElements
>> */
>> + for (i = 0; i < guest_info->apic_id_limit; i++) {
>> uint8_t b = test_bit(i, cpu->found_cpus) ? 0x01 : 0x00;
>> build_append_byte(package, b);
>> }
>> --
>> 1.8.3.1