On Tue, 22 Oct 2019 17:49:06 +0200 Laszlo Ersek <ler...@redhat.com> wrote:
> On 10/22/19 16:42, Igor Mammedov wrote: > > On Tue, 22 Oct 2019 14:39:24 +0200 > > Laszlo Ersek <ler...@redhat.com> 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 > >
