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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

>> > +                          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.

Jan


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