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 <[email protected]>
[...]
> 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
