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

Re: [PATCH v2 09/11] xen/arm: Setup MMIO range trap handlers for hardware domain


  • To: Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
  • Date: Mon, 27 Sep 2021 08:47:44 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=zoFPAhxzEd2nryFaPLGz3Dx6D3fkNJesHTJ3hanOgN0=; b=Szt6jZING72GHUjCMexYQY4HIR6JIPc5NDy1FSIvjZU+51sGpwqNJx+h2K46vkyQ87478JxnXOj10oib05CU4yJFhqiFgvUapyBF2keqs/gRcu/veMf8ggLa4pGYXOdbMWw2QG6vAVW2o5gRheH5UiUD3kjPJO54R/1zPYK58xN/2Fc30VdoR2m5brAZ6jBbBFwbZcqZ3xQTnhfy4PF9YmDSyyJOA9v8tpgKWWy+Ps2mF4KjfWDV47bg/NJ3hP1V7bhrhorZLOTq58QJIe9DRPc77Uvj8w93e0jR+oiCa9isqm/HwEUC9P43BNLuKyE7j5k29YlT2XFguiqRU2pGsw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=bn1uJpqLqNhQSmpZywtc/kcWKi/C0ES5AZw85Vj55rltzEICCbW70vh73dGauFTvVrUfLLopOlekmHbUpxEFUZTt/M3+S5WY5iM9CmtOdrS1e2Z/whvLZydTcXlkhHvWqtBYZBvxJAVwrn+Ijk0T0LbTL/p26JdtHj5WdCl8IkBo+4vACQ0oUaDOGIPTMCUIFrepSrfRV+C0eefU/cjefIpQcUzsUoup8cnfF7jy+l4vQrw2jtl+lUUDkLhEYU5/rp9spYbSe/9/lBpc8LKdjWLJuu3X6/Ke8oB9bEZolA03lxV2y4fiUoMzX5ZNkB3IlYthwb/6hRwjmYtZLVirEA==
  • Authentication-results: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=epam.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "julien@xxxxxxx" <julien@xxxxxxx>, Oleksandr Tyshchenko <Oleksandr_Tyshchenko@xxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Artem Mygaiev <Artem_Mygaiev@xxxxxxxx>, "roger.pau@xxxxxxxxxx" <roger.pau@xxxxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Rahul Singh <rahul.singh@xxxxxxx>, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
  • Delivery-date: Mon, 27 Sep 2021 08:47:53 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHXsHo395a0MNZ4jUyUZjWX7U0qP6uz5McAgAOy8QA=
  • Thread-topic: [PATCH v2 09/11] xen/arm: Setup MMIO range trap handlers for hardware domain

On 25.09.21 03:18, Stefano Stabellini wrote:
> On Thu, 23 Sep 2021, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>>
>> In order for vPCI to work it needs to maintain guest and hardware
>> domain's views on the configuration space. For example, BARs and
>                   ^ of
>
Ack
>> COMMAND registers require emulation for guests and the guest view
>> on the registers needs to be in sync with the real contents of the
>    ^ of
Ack
>
>> relevant registers. For that ECAM address space needs to also be
>> trapped for the hardware domain, so we need to implement PCI host
>> bridge specific callbacks to properly setup MMIO handlers for those
>> ranges depending on particular host bridge implementation.
>>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> The patch looks pretty good, only a couple of minor comments below
>
>
>> ---
>> Since v1:
>>   - Dynamically calculate the number of MMIO handlers required for vPCI
>>     and update the total number accordingly
>>   - s/clb/cb
>>   - Do not introduce a new callback for MMIO handler setup
>> ---
>>   xen/arch/arm/domain.c              |  2 ++
>>   xen/arch/arm/pci/pci-host-common.c | 28 +++++++++++++++++++++++++
>>   xen/arch/arm/vpci.c                | 33 ++++++++++++++++++++++++++++++
>>   xen/arch/arm/vpci.h                |  6 ++++++
>>   xen/include/asm-arm/pci.h          |  7 +++++++
>>   5 files changed, 76 insertions(+)
>>
>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>> index 854e8fed0393..c7b25bc70439 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -733,6 +733,8 @@ int arch_domain_create(struct domain *d,
>>       if ( (rc = domain_vgic_register(d, &count)) != 0 )
>>           goto fail;
>>   
>> +    count += domain_vpci_get_num_mmio_handlers(d);
>> +
>>       if ( (rc = domain_io_init(d, count + MAX_IO_HANDLER)) != 0 )
>>           goto fail;
>>   
>> diff --git a/xen/arch/arm/pci/pci-host-common.c 
>> b/xen/arch/arm/pci/pci-host-common.c
>> index 1567b6e2956c..155f2a2743af 100644
>> --- a/xen/arch/arm/pci/pci-host-common.c
>> +++ b/xen/arch/arm/pci/pci-host-common.c
>> @@ -300,6 +300,34 @@ struct dt_device_node *pci_find_host_bridge_node(struct 
>> device *dev)
>>       }
>>       return bridge->dt_node;
>>   }
>> +
>> +int pci_host_iterate_bridges(struct domain *d,
>> +                             int (*cb)(struct domain *d,
>> +                                       struct pci_host_bridge *bridge))
>> +{
>> +    struct pci_host_bridge *bridge;
>> +    int err;
>> +
>> +    list_for_each_entry( bridge, &pci_host_bridges, node )
>> +    {
>> +        err = cb(d, bridge);
>> +        if ( err )
>> +            return err;
>> +    }
>> +    return 0;
>> +}
>> +
>> +int pci_host_get_num_bridges(void)
>> +{
>> +    struct pci_host_bridge *bridge;
>> +    int count = 0;
>> +
>> +    list_for_each_entry( bridge, &pci_host_bridges, node )
>> +        count++;
>> +
>> +    return count;
>> +}
>> +
>>   /*
>>    * Local variables:
>>    * mode: C
>> diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
>> index 76c12b92814f..14947e975d69 100644
>> --- a/xen/arch/arm/vpci.c
>> +++ b/xen/arch/arm/vpci.c
>> @@ -80,17 +80,50 @@ static const struct mmio_handler_ops vpci_mmio_handler = 
>> {
>>       .write = vpci_mmio_write,
>>   };
>>   
>> +static int vpci_setup_mmio_handler(struct domain *d,
>> +                                   struct pci_host_bridge *bridge)
>> +{
>> +    struct pci_config_window *cfg = bridge->cfg;
>> +
>> +    register_mmio_handler(d, &vpci_mmio_handler,
>> +                          cfg->phys_addr, cfg->size, NULL);
>> +    return 0;
>> +}
>> +
>>   int domain_vpci_init(struct domain *d)
>>   {
>>       if ( !has_vpci(d) )
>>           return 0;
>>   
>> +    if ( is_hardware_domain(d) )
>> +        return pci_host_iterate_bridges(d, vpci_setup_mmio_handler);
>> +
>> +    /* Guest domains use what is programmed in their device tree. */
>>       register_mmio_handler(d, &vpci_mmio_handler,
>>                             GUEST_VPCI_ECAM_BASE, GUEST_VPCI_ECAM_SIZE, 
>> NULL);
>>   
>>       return 0;
>>   }
>>   
>> +int domain_vpci_get_num_mmio_handlers(struct domain *d)
>> +{
>> +    int count = 0;
>> +
>> +    if ( is_hardware_domain(d) )
>> +        /* For each PCI host bridge's configuration space. */
>> +        count += pci_host_get_num_bridges();
> NIT: why += instead of = ?
Will fix
>
>
>> +    else
>> +        /*
>> +         * VPCI_MSIX_MEM_NUM handlers for MSI-X tables per each PCI device
>> +         * being passed through. Maximum number of supported devices
>> +         * is 32 as virtual bus topology emulates the devices as embedded
>> +         * endpoints.
>> +         * +1 for a single emulated host bridge's configuration space. */
>> +        count = VPCI_MSIX_MEM_NUM * 32 + 1;
>> +
>> +    return count;
>> +}
>> +
>>   /*
>>    * Local variables:
>>    * mode: C
>> diff --git a/xen/arch/arm/vpci.h b/xen/arch/arm/vpci.h
>> index d8a7b0e3e802..27a2b069abd2 100644
>> --- a/xen/arch/arm/vpci.h
>> +++ b/xen/arch/arm/vpci.h
>> @@ -17,11 +17,17 @@
>>   
>>   #ifdef CONFIG_HAS_VPCI
>>   int domain_vpci_init(struct domain *d);
>> +int domain_vpci_get_num_mmio_handlers(struct domain *d);
>>   #else
>>   static inline int domain_vpci_init(struct domain *d)
>>   {
>>       return 0;
>>   }
>> +
>> +static inline int domain_vpci_get_num_mmio_handlers(struct domain *d)
>> +{
>> +    return 0;
>> +}
>>   #endif
>>   
>>   #endif /* __ARCH_ARM_VPCI_H__ */
>> diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
>> index 5b100556225e..7618f0b6725b 100644
>> --- a/xen/include/asm-arm/pci.h
>> +++ b/xen/include/asm-arm/pci.h
>> @@ -15,6 +15,8 @@
>>   #ifndef __ARM_PCI_H__
>>   #define __ARM_PCI_H__
>>   
>> +#include <asm/mmio.h>
> This should be no longer required.
I'll check and remove
>
>
>>   #ifdef CONFIG_HAS_PCI
>>   
>>   #define pci_to_dev(pcidev) (&(pcidev)->arch.dev)
>> @@ -109,6 +111,11 @@ static always_inline bool 
>> is_pci_passthrough_enabled(void)
>>   {
>>       return !!pci_passthrough_enabled;
>>   }
>> +
>> +int pci_host_iterate_bridges(struct domain *d,
>> +                             int (*clb)(struct domain *d,
>> +                                        struct pci_host_bridge *bridge));
>> +int pci_host_get_num_bridges(void);
>>   #else   /*!CONFIG_HAS_PCI*/
>>   
>>   #define pci_passthrough_enabled (false)

Thank you,

Oleksandr

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.