Re: [Xen-devel] [PATCH v2 26/30] xen/x86: add PCIe emulation

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.09.16 at 17:57, <roger.pau@xxxxxxxxxx> wrote:
> Add a new MMIO handler that traps accesses to PCIe regions, as discovered by
> Xen from the MCFG ACPI table. The handler used is the same as the one used
> for accesses to the IO PCI configuration space.

Both in the title and in the code you use PCIe when you really mean
MCFG / MMCFG. Please don't misguide people like this: PCI-X uses
extended config space too afaik.

> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -46,6 +46,8 @@
>  #include <xen/iocap.h>
>  #include <public/hvm/ioreq.h>
>  
> +#include "../x86_64/mmconfig.h"

Please don't. If declaration there are needed here, move them into
xen/include/asm-x86/.

> +static struct acpi_mcfg_allocation *pcie_find_mmcfg(unsigned long addr)
> +{
> +    int i;

unsigned int

> +static void pcie_decode_addr(unsigned long addr, unsigned int *bus,

By the time you gets here what gets passed in is not an address
anymore, so please don't name the parameter this way.

> +                             unsigned int *slot, unsigned int *func,
> +                             unsigned int *reg)
> +{
> +
> +    *bus = (addr >> 20) & 0xff;
> +    *slot = (addr >> 15) & 0x1f;
> +    *func = (addr >> 12) & 0x7;

Any reason you can't use pci_mmcfg_decode() here instead of
these various open coded numbers, or at least some derived
logic?

> +static int pcie_range(struct vcpu *v, unsigned long addr)
> +{
> +
> +    return pcie_find_mmcfg(addr) != NULL ? 1 : 0;

No need for the conditional expression.

Jan


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