Re: [RFC PATCH 07/12] hvmloader: allocate MMCONFIG area in the MMIO hole + minor code refactoring

[Alexey G <x1917x@gmail.com>]

On Thu, 22 Mar 2018 09:57:16 +0000
Roger Pau Monné <roger.pau@citrix.com> wrote:
[...]
>> Yes, and it is still needed as we have two distinct (and not equal)
>> interfaces to PCI conf space. Apart from 0..FFh range overlapping
>> they can be considered very different interfaces. And whether it is
>> a real system or emulated -- we can use either one of these two
>> interfaces or both.  
>
>The legacy PCI config space accesses and the MCFG config space access
>are just different methods of accessing the PCI configuration space,
>but the data _must_ be exactly the same. I don't see how a device
>would care about where the access to the config space originated.

If they were different methods of accessing the same thing, they
could've been used interchangeably. When we've got a PCI conf ioreq
which has offset>100h we know we cannot just pass it to emulated
CF8/CFC but have to emulate this specifically.

>> For QEMU zero changes are needed to support MMCONFIG MMIO accesses if
>> they come as MMIO ioreqs. It's just what its MMCONFIG emulation code
>> expects.  
>
>As I said many times in this thread, you seem to be focused around
>what's best for QEMU only, and this is wrong. The IOREQ interface is
>used by QEMU, but it's also used by other device emulators.
>
>I get the feeling that you assume that the correct solution is the one
>that involves less changes to Xen and QEMU. This is simply not true.
>
>> Anyway, for (kind of vague) users of the multiple ioreq servers
>> capability we can enable MMIO translation to PCI conf ioreqs. Note
>> that actually this is an extra step, not forwarding trapped MMCONFIG
>> MMIO accesses to the selected device model as is.
>>  
>> >Getting both IOREQ_TYPE_PCI_CONFIG and IOREQ_TYPE_COPY for PCI
>> >config space access is misleading.  
>> 
>> These are very different accesses, both in transport and
>> capabilities. 
>> >In both cases Xen would have to do the MCFG access decoding in order
>> >to figure out which IOREQ server will handle the request. At which
>> >point the only step that you avoid is the reconstruction of the
>> >memory access from the IOREQ_TYPE_PCI_CONFIG which is trivial.  
>> 
>> The "reconstruction of the memory access" you mentioned won't be easy
>> actually. The thing is, address_space_read/write is not all what we
>> need.
>> 
>> In order to translate PCI conf ioreqs back to emulated MMIO ops, we
>> need to be an involved party, mainly to know where MMCONFIG area is
>> located so we can construct the address within its range from BDF.
>> This piece of information is destroyed in the process of MMIO ioreq
>> translation to PCI conf type.  
>
>QEMU certainly knows the position of the MCFG area (because it's the
>one that tells Xen about it), so I don't understand your concerns
>above.
>> The code which parse PCI conf ioreqs in xen-hvm.c doesn't know
>> anything about the current emulated MMCONFIG state. The correct way
>> to have this info is to participate in its emulation. As we don't
>> participate, we have no other way than trying to gain backdoor
>> access to PCIHost fields via things like object_resolve_*(). This
>> solution is cumbersome and ugly but will work... and may break
>> anytime due to changes in QEMU.   
>
>OK, so you don't want to reconstruct the access, fine.
>
>Then just inject it using pcie_mmcfg_data_{read/write} or some similar
>wrapper. My suggestion was just to try to use the easier way to get
>this injected into QEMU.

QEMU knows its position, the problem it that xen-hvm.c (ioreq
processor) is rather isolated from MMCONFIG emulation.

If you check the pcie_mmcfg_data_read/write MMCONFIG handlers in QEMU,
you can see this:

static uint64_t pcie_mmcfg_data_read(void *opaque, <...>
{
    PCIExpressHost *e = opaque;
...

We know this 'opaque' when we do MMIO-style MMCONFIG handling as
pcie_mmcfg_data_read/write are actual handlers.

But xen-hvm.c needs to gain access to PCIExpressHost out of nowhere,
which is possible but considered a hack by QEMU. We can also insert
some code to MMCONFIG emulation which will store info we need to some
global variables to be used across wildly different and unrelated
modules. It will work, but anyone who see it will have bad thoughts on
his mind.

>> QEMU maintainers will grin while looking at all this I'm afraid --
>> trapped MMIO accesses which are translated to PCI conf accesses which
>> in turn translated back to emulated MMIO accesses upon receiving,
>> along with tedious attempts to gain access to MMCONFIG-related info
>> as we're not invited to the MMCONFIG emulation party.
>>
>> The more I think about it, the more I like the existing
>> map_io_range_to_ioreq_server() approach. :( It works without doing
>> anything, no hacks, no new interfaces, both MMCONFIG and CF8/CFC are
>> working as expected. There is a problem to make it compatible with
>> the specific multiple ioreq servers feature, but providing a new
>> dmop/hypercall (which you suggest is a must have thing to trap
>> MMCONFIG MMIO to give QEMU only the freedom to tell where it is
>> located) allows to solve this problem in any possible way, either
>> MMIO -> PCI conf translation or anything else.  
>
>I'm sorry, but I'm getting lost.
>
>You complain that using IOREQ_TYPE_PCI_CONFIG is not a good approach
>because QEMU needs to know the position of the MCFG area if we want to
>reconstruct and forward the MMIO access. And then you are proposing to
>use IOREQ_TYPE_COPY which _requires_ QEMU to know the position of the
>MCFG area in order to do the decoding of the PCI config space access.
>> >> We can still route either ioreq
>> >> type to multiple device emulators accordingly.    
>> >
>> >It's exactly the same that's done for IO space PCI config space
>> >addresses. QEMU gets an IOREQ_TYPE_PCI_CONFIG and it replays the IO
>> >space access using do_outp and cpu_ioreq_pio.  
>> 
>> ...And it is completely limited to basic PCI conf space. I don't know
>> the context of this line in xen-hvm.c:
>> 
>> val = (1u << 31) | ((req->addr & 0x0f00) << 16) | ((sbdf & 0xffff)
>> << 8) | (req->addr & 0xfc);
>> 
>> but seems like current QEMU versions do not expect anything similar
>> to AMD ECS-style accesses for 0CF8h. It is limited to basic PCI conf
>> only. 
>> >If you think using IOREQ_TYPE_COPY for MCFG accesses is such a
>> >benefit for QEMU, why not just translate the IOREQ_TYPE_PCI_CONFIG
>> >into IOREQ_TYPE_COPY in handle_ioreq and dispatch it using
>> >cpu_ioreq_move?  
>> 
>> Answered above, we need to somehow have access to the info which
>> don't belong to us for this step.  
>
>Why not? QEMU tells Xen the position of the MCFG area but then you
>complain that QEMU doesn't know the position of the MCFG area?

Answered above.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel