Re: [PATCH] [pv-ops] fix dom0 S3 when MSI is used.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Mar 17, 2010 at 04:46:47PM +0800, Cui, Dexuan wrote:
> The old commit a234848f works only when the device supports D3hot; when the
> device only supports D3cold, the device doesn't work properly after resuming
> from Dom0 S3.
> A better workaround is invoking the PHYSDEVOP_restore_msi hypercall.
> The patch reverts the old commit and invokes the hypercall.
> 
> Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index b40c6d0..c6bffe2 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -20,6 +20,7 @@
>  #include <asm/errno.h>
>  #include <asm/io.h>
>  
> +#include <asm/xen/hypercall.h>
>  #include <asm/xen/hypervisor.h>
>  
>  #include "pci.h"
> @@ -271,8 +272,7 @@ void write_msi_msg(unsigned int irq, struct msi_msg *msg)
>  {
>  	struct irq_desc *desc = irq_to_desc(irq);
>  
> -	if (!xen_initial_domain())
> -		write_msi_msg_desc(desc, msg);
> +	write_msi_msg_desc(desc, msg);

Nice. That will remove the other platofmr build problem.
>  }
>  
>  static int msi_free_irqs(struct pci_dev* dev);
> @@ -347,6 +347,20 @@ static void __pci_restore_msix_state(struct pci_dev *dev)
>  
>  void pci_restore_msi_state(struct pci_dev *dev)
>  {
> +	if (xen_initial_domain()) {

That won't do. If you try to compile this kernel on other platforms (say
PPC), this will throw a huge problem.

> +		struct physdev_restore_msi physdev;
> +
> +		if (!dev->msi_enabled && !dev->msix_enabled)
> +			return;

This seems redundant.

I think the problem you are trying to address is doing to show up when
doing PC passthrough using pciback and xen-pcifront.

The mechanims that was employed there to make it work, was to utilize
the arch_setup_msi_irqs in (arch/x86/kernel/apic/io_apic.c) and make it
call functions in arch/x86/pci/xen.c. That code then figures out if you
are running in priviliged or un-priviliged mode and makes the appropiate
call.

Perhaps you should using that idea and expand on it. I would suggest you
take a look at the how PPC implements this and see if there is something
that can be borrowed from their mechanism.

> +
> +		pci_intx_for_msi(dev, 0);
> +
> +		physdev.bus = dev->bus->number;
> +		physdev.devfn = dev->devfn;
> +		HYPERVISOR_physdev_op(PHYSDEVOP_restore_msi, &physdev);
> +
> +		return;
> +	}
>  	__pci_restore_msi_state(dev);
>  	__pci_restore_msix_state(dev);
>  }
> diff --git a/include/xen/interface/physdev.h b/include/xen/interface/physdev.h
> index ac5de37..516d2b4 100644
> --- a/include/xen/interface/physdev.h
> +++ b/include/xen/interface/physdev.h
> @@ -144,6 +144,13 @@ struct physdev_manage_pci {
>  	uint8_t devfn;
>  };
>  
> +#define PHYSDEVOP_restore_msi		19
> +struct physdev_restore_msi {
> +	/* IN */
> +	uint8_t bus;
> +	uint8_t devfn;
> +};
> +
>  #define PHYSDEVOP_manage_pci_add_ext	20
>  struct physdev_manage_pci_ext {
>  	/* IN */


> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel