[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command
From: |
Igor Mammedov |
Subject: |
Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command |
Date: |
Wed, 23 Oct 2019 16:59:50 +0200 |
On Tue, 22 Oct 2019 17:49:06 +0200
Laszlo Ersek <address@hidden> wrote:
> On 10/22/19 16:42, Igor Mammedov wrote:
> > On Tue, 22 Oct 2019 14:39:24 +0200
> > Laszlo Ersek <address@hidden> wrote:
> >
> >> On 10/21/19 15:06, Laszlo Ersek wrote:
> >>> On 10/18/19 18:18, Igor Mammedov wrote:
>
> >>>> Considering firmware runs the first, it should enable modern interface
> >>>> on its own
> >>>> 1. Store 0x0 to selector register (actually it's store into bitmap to
> >>>> attempt switch).
> >>>> and to check if interface is present
> >>>> 2. Store 0x0 to selector register (to ensure valid selector value
> >>>> (otherwise command is ignored))
> >>>> 3. Store 0x0 to command register (to be able to read back selector
> >>>> from command data)
> >>>> 4. Store 0x0 to selector register (because #3 can select the a cpu
> >>>> with events if any)
> >>>> be aware libvirt may start QEMU in paused mode (hotplug context)
> >>>> and hotplugs extra CPUs
> >>>> with device_add and then let guest run. So firmware may see
> >>>> present CPUs with events
> >>>> at boot time.
> >>>> 5. Read 'command data' register.
> >>>> 6. If value read is 0, the interface is available.
>
> >> When we read the command data register in the last step, that is at
> >> offset 0x8 in the register block. Considering the legacy "CPU present
> >> bitmap", if no CPU is present in that range, then the firmware could
> >> read a zero value. I got confused because I thought we were reading at
> >> offset 0, which would always have bit0 set (for CPU#0).
> >>
> >> Can we detect the modern interface like this:
> >>
> >> 1. store 0x0 to selector register (attempt to switch)
> >> 2. read one byte at offset 0 in the register block
> >> 3. if bit#0 is set, the modern interface is unavailable;
> >> otherwise (= bit#0 clear), the modern interface is available
> >>
> >> Here's why:
> >>
> >> - if even the legacy interface is missing, then step 2 is an unassigned
> >> read, hence the value read is all-bits-one; bit#0 is set
> >>
> >> - if only the legacy interface is available, then bit#0 stands for
> >> CPU#0, it will be set
> >>
> >> - if the switch-over in step 1 is successful, then offset 0 is reserved,
> >> hence it returns all-bits-zero.
> >>
> >> With this, if we ever assigned offset 0 for reading, then we'd have to
> >> define it with bit#0 constantly clear.
> >
> > There is no need to reserve bit#0 if in step #5 we use s/'command
> > data'/'Command data 2'/
>
> Good idea. We can drop step 4 too:
>
> [0x0] Command data 2: (DWORD access, little endian)
> If the "CPU selector" value last stored by the guest refers to
> an impossible CPU, then 0.
>
> This is skipped by step 2.
>
> Otherwise, if the "Command field" value last stored by the
> guest differs from 3, then 0.
>
> This is triggered by step 3.
>
> So step 4 does not look necessary. (As long as the guest is OK with the
> selector ending up with a changed value.)
sounds good,
I'll respin patches taking this into account.
> Otherwise, the most significant 32 bits of the selected CPU's
> architecture specific ID.
>
> Not relevant for this use case.
>
> > Alternatively we can reserve bit#0 and sequentially read upper half from
> > 'Command data'
> > (one a new flag to show that there is more data to read).
>
> I like the "Command data 2" register more. The "temporal domain" is
> always a complication in register definitions.
>
> > (Upper half currently is not necessary, it's there for future ARM's MPIDR).
> >
> > One more thing, this behavior is based on artifacts of x86 machine and
> > AllOnes fallback.
> > Obviously it won't work with arm/virt, do we care about AVMF at this point?
> >
>
> No, in the firmware, all this is strictly x86 code. The ArmVirtQemu
> guest firmware has no support for multiprocessing at this time, to my
> understanding.
>
> (Nonetheless, if the register block got placed at an MMIO base address
> on arm/virt, I think "unassigned_mem_ops" would apply there just the same.)
>
> Thanks!
> Laszlo
>
>
- Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command, (continued)
- Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command, Michael S. Tsirkin, 2019/10/10
- Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command, Igor Mammedov, 2019/10/10
- Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command, Laszlo Ersek, 2019/10/10
- Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command, Eduardo Habkost, 2019/10/10
- Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command, Laszlo Ersek, 2019/10/11
- Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command, Igor Mammedov, 2019/10/18
- Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command, Laszlo Ersek, 2019/10/21
- Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command, Laszlo Ersek, 2019/10/22
- Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command, Igor Mammedov, 2019/10/22
- Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command, Laszlo Ersek, 2019/10/22
- Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command,
Igor Mammedov <=
Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command, Philippe Mathieu-Daudé, 2019/10/24
Re: [RFC 0/3] acpi: cphp: add CPHP_GET_CPU_ID_CMD command to cpu hotplug MMIO interface, Michael S. Tsirkin, 2019/10/10