Re: [PATCH v6 1/3] xen/vpci: Move ecam access functions to common code

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Oct 15, 2021 at 07:53:38AM +0000, Bertrand Marquis wrote:
> > On 15 Oct 2021, at 08:44, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
> > On Thu, Oct 14, 2021 at 03:49:49PM +0100, Bertrand Marquis wrote:
> >> @@ -434,25 +420,8 @@ static int vpci_mmcfg_read(struct vcpu *v, unsigned 
> >> long addr,
> >>     reg = vpci_mmcfg_decode_addr(mmcfg, addr, &sbdf);
> >>     read_unlock(&d->arch.hvm.mmcfg_lock);
> >> 
> >> -    if ( !vpci_access_allowed(reg, len) ||
> >> -         (reg + len) > PCI_CFG_SPACE_EXP_SIZE )
> >> -        return X86EMUL_OKAY;
> >> -
> >> -    /*
> >> -     * According to the PCIe 3.1A specification:
> >> -     *  - Configuration Reads and Writes must usually be DWORD or smaller
> >> -     *    in size.
> >> -     *  - Because Root Complex implementations are not required to support
> >> -     *    accesses to a RCRB that cross DW boundaries [...] software
> >> -     *    should take care not to cause the generation of such accesses
> >> -     *    when accessing a RCRB unless the Root Complex will support the
> >> -     *    access.
> >> -     *  Xen however supports 8byte accesses by splitting them into two
> >> -     *  4byte accesses.
> >> -     */
> >> -    *data = vpci_read(sbdf, reg, min(4u, len));
> >> -    if ( len == 8 )
> >> -        *data |= (uint64_t)vpci_read(sbdf, reg + 4, 4) << 32;
> >> +    /* Ignore return code */
> >> +    vpci_ecam_mmio_read(sbdf, reg, len, data);
> > 
> > I think it would be better for vpci_ecam_mmio_read to just return the
> > read value, or ~0 in case of error, at least that interface would be
> > simpler and suitable for x86.
> 
> I am not quite sure on this as on absolute to read ~0 is possible so the
> caller cannot distinguish between properly reading ~0 or an access allowed 
> error.

How do you report an access allowed error on Arm for the PCI config
space?

At least on x86 I don't think we currently have a way to propagate
such errors, neither a plan to do so that I'm aware.

Thanks, Roger.