RE: [Xen-devel] [PATCH] [pv-ops] fix dom0 S3 when MSI is used.

["Cui, Dexuan" <dexuan.cui@xxxxxxxxx>]

Konrad Rzeszutek Wilk wrote:
> 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@xxxxxxxxx>
>> 
>> 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
Hi Konrad,
Thanks for the comments!
The patch is used to act as "a better workaround" for the actual Dom0 S3 issue 
for now.
I think the MSI implementation in pv-ops dom0 will be re-worked in future, just 
like the IOAPIC rework that happened.
This patch is not meant to fix all the existing issues. :-)

> (say PPC), this will throw a huge problem.
> 
>> +            struct physdev_restore_msi physdev;
>> +
>> +            if (!dev->msi_enabled && !dev->msix_enabled)
>> +                    return;
> 
> This seems redundant.
Seems not redundant. 
if (!dev->msi_enabled && !dev->msix_enabled), we don't need to go further to 
invoke the hypercall.

> 
> I think the problem you are trying to address is doing to show up when
> doing PC passthrough using pciback and xen-pcifront.
Actually the bug I'm trying to fix is only limited to pv-dom0 itself:
In my host, after Dom0 S3 resume, the SATA disk can't work properly in Dom0 and 
Dom0's filesytem would become inaccessibe.
With pci=nomsi, or with the patch, Dom0 S3 can work fine.

> 
> 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.
Agree. I  think this could be considered in the coming MSI rework in pv-ops 
dom0.

Thanks,
-- Dexuan

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