From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55014) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwqij-0003Ri-Uh for qemu-devel@nongnu.org; Wed, 19 Oct 2016 09:15:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bwqii-0002Xq-T4 for qemu-devel@nongnu.org; Wed, 19 Oct 2016 09:15:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48224) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bwqii-0002Xe-Lw for qemu-devel@nongnu.org; Wed, 19 Oct 2016 09:15:48 -0400 Date: Wed, 19 Oct 2016 11:15:46 -0200 From: Eduardo Habkost Message-ID: <20161019131546.GL5057@thinpad.lan.raisama.net> References: <1476878743-144953-1-git-send-email-imammedo@redhat.com> <1476878743-144953-12-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1476878743-144953-12-git-send-email-imammedo@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 11/13] pc: add 'etc/boot-cpus' fw_cfg file for machine with more than 255 CPUs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org, kraxel@redhat.com, liuxiaojian6@huawei.com, mst@redhat.com, rkrcmar@redhat.com, peterx@redhat.com, kevin@koconnor.net, pbonzini@redhat.com, lersek@redhat.com, chao.gao@intel.com On Wed, Oct 19, 2016 at 02:05:41PM +0200, Igor Mammedov wrote: > Currently firmware uses 1 byte at 0x5F offset in RTC CMOS > to get number of CPUs present at boot. However 1 byte is > not enough to handle more than 255 CPUs. So add a new > fw_cfg file that would allow QEMU to tell it. > For compat reasons add file only for machine types that > support more than 255 CPUs. > > Signed-off-by: Igor Mammedov [...] > static X86CPU *pc_new_cpu(const char *typename, int64_t apic_id, > Error **errp) > { > @@ -1232,6 +1221,11 @@ static void pc_build_feature_control_file(PCMachineState *pcms) > fw_cfg_add_file(pcms->fw_cfg, "etc/msr_feature_control", val, sizeof(*val)); > } > > +static void rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count) > +{ > + rtc_set_memory(rtc, 0x5f, cpus_count - 1); If we have more than 255 CPUs, shouldn't we at least tell the old BIOS that we have 255, instead of silently truncating bits? > +} > + > static > void pc_machine_done(Notifier *notifier, void *data) > { > @@ -1240,7 +1234,7 @@ void pc_machine_done(Notifier *notifier, void *data) > PCIBus *bus = pcms->bus; > > /* set the number of CPUs */ > - rtc_set_memory(pcms->rtc, 0x5f, pc_present_cpus_count(pcms) - 1); > + rtc_set_cpus_count(pcms->rtc, le16_to_cpu(pcms->boot_cpus_le)); > > if (bus) { > int extra_hosts = 0; > @@ -1261,8 +1255,15 @@ void pc_machine_done(Notifier *notifier, void *data) > > acpi_setup(); > if (pcms->fw_cfg) { > + MachineClass *mc = MACHINE_GET_CLASS(pcms); > + > pc_build_smbios(pcms->fw_cfg); > pc_build_feature_control_file(pcms); > + > + if (mc->max_cpus > 255) { > + fw_cfg_add_file(pcms->fw_cfg, "etc/boot-cpus", &pcms->boot_cpus_le, > + sizeof(pcms->boot_cpus_le)); > + } > } > } > > @@ -1786,9 +1787,11 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev, > } > } > > + /* increment the number of CPUs */ > + pcms->boot_cpus_le = cpu_to_le16(le16_to_cpu(pcms->boot_cpus_le) + 1); Is this really safe? What if the guest is in the middle of a etc/boot-cpus read? > if (dev->hotplugged) { > - /* increment the number of CPUs */ > - rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1); > + /* Update the number of CPUs in CMOS */ > + rtc_set_cpus_count(pcms->rtc, le16_to_cpu(pcms->boot_cpus_le)); > } > > found_cpu = pc_find_cpu_slot(pcms, CPU(dev), NULL); > @@ -1842,7 +1845,10 @@ static void pc_cpu_unplug_cb(HotplugHandler *hotplug_dev, > found_cpu->cpu = NULL; > object_unparent(OBJECT(dev)); > > - rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) - 1); > + /* decrement the number of CPUs */ > + pcms->boot_cpus_le = cpu_to_le16(le16_to_cpu(pcms->boot_cpus_le) - 1); > + /* Update the number of CPUs in CMOS */ > + rtc_set_cpus_count(pcms->rtc, le16_to_cpu(pcms->boot_cpus_le)); > out: > error_propagate(errp, local_err); > } > -- > 2.7.4 > -- Eduardo