Re: [Xen-devel] [PATCH v3 2/9] x86/ecam: add handlers for the PVH Dom0 MMCFG areas

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Tue, Jun 20, 2017 at 07:14:07AM -0600, Jan Beulich wrote:
> >>> On 20.06.17 at 13:56, <roger.pau@xxxxxxxxxx> wrote:
> > On Fri, May 19, 2017 at 07:25:22AM -0600, Jan Beulich wrote:
> >> >>> On 27.04.17 at 16:35, <roger.pau@xxxxxxxxxx> wrote:
> >> > +{
> >> > +    unsigned int i;
> >> > +    int rc;
> >> > +
> >> > +    for ( i = 0; i < pci_mmcfg_config_num; i++ )
> >> > +    {
> >> > +        rc = register_vpci_ecam_handler(d, pci_mmcfg_config[i].address,
> >> > +                                        
> >> > pci_mmcfg_config[i].start_bus_number,
> >> > +                                        
> >> > pci_mmcfg_config[i].end_bus_number,
> >> > +                                        
> >> > pci_mmcfg_config[i].pci_segment);
> >> > +        if ( rc )
> >> > +            return rc;
> >> > +    }
> >> > +
> >> > +    return 0;
> >> > +}
> >> 
> >> What about regions becoming available only post-boot?
> > 
> > This is not yet supported. It needs to be implemented using the
> > PHYSDEVOP_pci_mmcfg_reserved hypercall.
> 
> But then the patch here is incomplete.

OK, I don't think it's going to be a lot of code, it's just
registering extra MMCFG regions.

> >> > +                          unsigned int len, unsigned long *data)
> >> > +{
> >> > +    struct domain *d = v->domain;
> >> > +    struct hvm_ecam *ecam;
> >> > +    unsigned int bus, devfn, reg;
> >> > +    uint32_t data32;
> >> > +    int rc;
> >> > +
> >> > +    vpci_lock(d);
> >> > +    ecam = vpci_ecam_find(d, addr);
> >> > +    if ( !ecam )
> >> > +    {
> >> > +        vpci_unlock(d);
> >> > +        return X86EMUL_UNHANDLEABLE;
> >> > +    }
> >> > +
> >> > +    vpci_ecam_decode_addr(ecam, addr, &bus, &devfn, &reg);
> >> > +
> >> > +    if ( vpci_access_check(reg, len) || reg >= 0xfff )
> >> 
> >> So this function iirc allows only 1-, 2-, and 4-byte accesses. Other
> >> than with port I/O, MMCFG allows wider ones, and once again I
> >> don't think hardware would raise any kind of fault in such a case.
> >> The general expectation is for the fabric to split such accesses.
> > 
> > Hm, the PCIe spec is not authoritative in this regard, is states that
> > supporting 8B accesses is not mandatory. Xen/Linux/FreeBSD will never
> > attempt any access > 4B, hence I haven't coded this case.
> > 
> > Would you be fine with leaving this for later, or would you rather
> > have it implemented as part of this series?
> 
> Since it shouldn't meaningfully much more code, I'd prefer if it was
> done right away. Otherwise I'd have to ask for a "fixme" comment,
> and I'd rather avoid such considering the PVHv1 history.

NP, I've just added it. I have however implemented it by splitting the
access into two 4 byte accesses, and performing two calls to
vpci_{read/write}.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel