Re: [Xen-devel] [PATCH v3 5/6] xen/x86: add PHYSDEVOP_msi_msix_set_enable

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Sat, Jan 26, 2019 at 03:31:16AM +0100, Marek Marczykowski-Górecki wrote:
> Allow device model running in stubdomain to enable/disable MSI(-X),
> bypassing pciback. While pciback is still used to access config space
> from within stubdomain, it refuse to write to
> PCI_MSI_FLAGS_ENABLE/PCI_MSIX_FLAGS_ENABLE in non-permissive mode. Which
> is the right thing to do for PV domain (the main use case for pciback),
> as PV domain should use XEN_PCI_OP_* commands for that. Unfortunately
> those commands are not good for stubdomain use, as they configure MSI in
> dom0's kernel too, which should not happen for HVM domain.
> 
> This new physdevop is allowed only for stubdomain controlling the domain
> which own the device.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> Changes in v3:
>  - new patch
> 
> This is rather RFC. Any suggestions for shorter name? Also, I'm not sure
> if physdev_msi_msix_set_enable.flag is the best name/idea.

I'm bad at naming things, so I will refrain from commenting on this.

> 
> Should it be plugged into XSM? Any suggestions how exactly? New
> function with XSM_DM_PRIV default action? Should it get target domain
> only, or also machine_bdf?

It should be hooked into XSM -- other sub-ops already do that.

> ---
>  xen/arch/x86/msi.c           | 16 ++++++++++++++++
>  xen/arch/x86/physdev.c       | 24 ++++++++++++++++++++++++
>  xen/include/asm-x86/msi.h    |  1 +
>  xen/include/public/physdev.h | 13 +++++++++++++
>  4 files changed, 54 insertions(+)
> 
> diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
> index babc414..9ba934c 100644
> --- a/xen/arch/x86/msi.c
> +++ b/xen/arch/x86/msi.c
> @@ -1474,6 +1474,22 @@ int pci_restore_msi_state(struct pci_dev *pdev)
>      return 0;
>  }
>  
> +int msi_msix_set_enable(struct pci_dev *pdev, int flag, int enable)
> +{
> +    if ( !current->domain->target || pdev->domain != current->domain->target 
> )
> +        return -EPERM;
> +
> +    switch ( flag ) {

{ should be on a new line.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel