Re: [kvm-unit-tests PATCH v5 04/12] pci: Rework pci_bar_addr()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 19, 2016 at 02:53:01PM +0200, Alexander Gordeev wrote:
> This update makes pci_bar_addr() interface 64 bit BARs aware and
> introduces a concept of PCI address translation.
> 
> An architecutre should implement pci_translate_addr() interface
> in order to provide mapping between PCI bus address and CPU
> physical address.
> 
> Cc: Thomas Huth <thuth@xxxxxxxxxx>
> Cc: Andrew Jones <drjones@xxxxxxxxxx>
> Signed-off-by: Alexander Gordeev <agordeev@xxxxxxxxxx>
> ---
>  lib/pci.c         | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  lib/pci.h         | 16 +++++++++++-
>  lib/x86/asm/pci.h |  6 +++++
>  3 files changed, 91 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/pci.c b/lib/pci.c
> index b05ecfa3f3da..e3e8dfda6c87 100644
> --- a/lib/pci.c
> +++ b/lib/pci.c
> @@ -21,14 +21,67 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
>  	return PCIDEVADDR_INVALID;
>  }
>  
> -unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num)
> +static uint32_t pci_bar_mask(uint32_t bar)
>  {
> -	uint32_t bar = pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
> +	return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
> +		PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
> +}
>  
> -	if (bar & PCI_BASE_ADDRESS_SPACE_IO)
> -		return bar & PCI_BASE_ADDRESS_IO_MASK;
> -	else
> -		return bar & PCI_BASE_ADDRESS_MEM_MASK;
> +phys_addr_t pci_bar_addr(pcidevaddr_t dev, int bar_num)
> +{
> +	int off = PCI_BASE_ADDRESS_0 + bar_num * 4;
> +	uint32_t bar = pci_config_readl(dev, off);
> +	uint32_t mask = pci_bar_mask(bar);
> +	uint64_t addr = bar & mask;
> +
> +	if (pci_bar_is64(dev, bar_num))
> +		addr |= (uint64_t)pci_config_readl(dev, off + 4) << 32;
> +
> +	return pci_translate_addr(dev, addr);
> +}
> +
> +/*
> + * To determine the amount of address space needed by a PCI device,
> + * one must save the original value of the BAR, write a value of
> + * all 1's to the register, then read it back. The amount of memory
                              ^and
> + * can then be then determined by masking the information bits,
extra 'then'
> + * performing a bitwise NOT and incrementing the value by 1.
                              ^,
> + *
> + * The following pci_bar_size_helper() and pci_bar_size() functions do
> + * the described algorithm.
/do the described/implement the/
> + */
> +static uint32_t pci_bar_size_helper(pcidevaddr_t dev, int bar_num)
> +{
> +	int off = PCI_BASE_ADDRESS_0 + bar_num * 4;
> +	uint32_t bar, size;
> +
> +	bar = pci_config_readl(dev, off);
> +	pci_config_writel(dev, off, ~0u);
> +	size = pci_config_readl(dev, off);
> +	pci_config_writel(dev, off, bar);
> +
> +	return size;
> +}
> +
> +phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num)
> +{
> +	uint32_t bar, size;
> +
> +	size = pci_bar_size_helper(dev, bar_num);
> +	if (!size)
> +		return 0;
> +
> +	bar = pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
> +	size &= pci_bar_mask(bar);
> +
> +	if (pci_bar_is64(dev, bar_num)) {
> +		phys_addr_t size64 = pci_bar_size_helper(dev, bar_num + 1);
> +		size64 = (size64 << 32) | size;
> +
> +		return ~size64 + 1;
> +	} else {
> +		return ~size + 1;
> +	}
>  }
>  
>  bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num)
> @@ -42,3 +95,14 @@ bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num)
>  {
>  	return pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
>  }
> +
> +bool pci_bar_is64(pcidevaddr_t dev, int bar_num)
> +{
> +	uint32_t bar = pci_config_readl(dev, PCI_BASE_ADDRESS_0 + bar_num * 4);
> +
> +	if (bar & PCI_BASE_ADDRESS_SPACE_IO)
> +		return false;
> +
> +	return (bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK) ==
> +		      PCI_BASE_ADDRESS_MEM_TYPE_64;
> +}
> diff --git a/lib/pci.h b/lib/pci.h
> index 54fbf22d634a..4d3e8d385eb1 100644
> --- a/lib/pci.h
> +++ b/lib/pci.h
> @@ -16,7 +16,21 @@ enum {
>  };
>  
>  pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
> -unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num);
> +
> +/*
> + * BAR number in all BAR access functions below is a number of 32-bit
s/BAR number/@bar_num/
s/a number/the index/
s/32-bit/the 32-bit/
> + * register starting from PCI_BASE_ADDRESS_0 offset.
s/PCI_BASE_ADDRESS_0/the PCI_BASE_ADDRESS_0/
> + *
> + * In cases BAR size is 64-bit a caller should still provide BAR number
s/BAR size/where the BAR size/   ^,
s/BAR number/@bar_num/
> + * in terms of 32-bit words. For example, if a device has 64-bit BAR#0
                                                            ^a
> + * and 32-bit BAR#1 the caller should provide 2 to address BAR#1, not 1.
         ^a           ^, then
> + *
> + * It is expected the caller is aware of the device BAR layout and never
> + * tries to address in the middle of a 64-bit register.
remove 'in'
> + */
> +phys_addr_t pci_bar_addr(pcidevaddr_t dev, int bar_num);
> +phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num);
> +bool pci_bar_is64(pcidevaddr_t dev, int bar_num);
>  bool pci_bar_is_memory(pcidevaddr_t dev, int bar_num);
>  bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num);
>  
> diff --git a/lib/x86/asm/pci.h b/lib/x86/asm/pci.h
> index 821a2c1e180a..7384b91adba1 100644
> --- a/lib/x86/asm/pci.h
> +++ b/lib/x86/asm/pci.h
> @@ -35,4 +35,10 @@ static inline void pci_config_writel(pcidevaddr_t dev, uint8_t reg, uint32_t val
>      outl(val, 0xCFC);
>  }
>  
> +static inline
> +phys_addr_t pci_translate_addr(pcidevaddr_t dev __unused, uint64_t addr)
> +{
> +    return addr;
> +}
> +
>  #endif
> -- 
> 1.8.3.1
>

Besides the comment edits

Reviewed-by: Andrew Jones <drjones@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux