Permissive devices in Xen

Permissive devices in Xen

[Lukas Hejtmanek]

Hello,

is there any reason why even permissive devices cannot write some areas in the 
PCI config space?

The PCI_COMMAND is handled in special way so that the device is
enabled/disabled properly, am I right?

Why is PCI_INTERRUPT_LINE read from dev->irq and not from the config space?

Why the PCI address bars are handled in very different way? And it seems that
writing these bars is never committed to the real devices. Why?

Thanks for answers.

-- 
Lukáš Hejtmánek

Re: Permissive devices in Xen

[Keir Fraser]

On 8/7/07 02:05, "Lukas Hejtmanek" <xhejtman@ics.muni.cz> wrote:

> The PCI_COMMAND is handled in special way so that the device is
> enabled/disabled properly, am I right?
> 
> Why is PCI_INTERRUPT_LINE read from dev->irq and not from the config space?

dom0 will have correctly routed the interrupt,a nd stored the correct irq
number in dev->irq. The number in the config space may not be correct, or
may indicate only the irq number for legacy PCI-ISA link.

> Why the PCI address bars are handled in very different way? And it seems that
> writing these bars is never committed to the real devices. Why?

domU is not trusted to manage the i/o address space. We assume dom0 has set
up the values sanely and domU should have no reason to change them.

 -- Keir

Re: Permissive devices in Xen

[Lukas Hejtmanek]

On Sun, Jul 08, 2007 at 10:55:16AM +0100, Keir Fraser wrote:
> > Why the PCI address bars are handled in very different way? And it seems
> > that writing these bars is never committed to the real devices. Why?
> 
> domU is not trusted to manage the i/o address space. We assume dom0 has set
> up the values sanely and domU should have no reason to change them.

OK, I see. But there is problem with, e.g., InfiniBand card that during
initialization performs reset of the card. The reset destroys the PCI config
space therefore the driver reads 16 dwords from the config space and after
reset, it writes them back. In DomU, it obviously does not restore address
bars and the device fails to respond via MMIO.

So, I've tried to allow DomU (via pciback driver) to write also the address 
bars but in this case, I'm facing to huge SLAB corruptions. In Dom0 or non-Xen
kernel, it works pretty well.

Do you have any hints how to track it down?

-- 
Lukáš Hejtmánek

Re: Permissive devices in Xen

[Keir Fraser]

On 8/7/07 11:00, "Lukas Hejtmanek" <xhejtman@ics.muni.cz> wrote:

>> domU is not trusted to manage the i/o address space. We assume dom0 has set
>> up the values sanely and domU should have no reason to change them.
> 
> OK, I see. But there is problem with, e.g., InfiniBand card that during
> initialization performs reset of the card. The reset destroys the PCI config
> space therefore the driver reads 16 dwords from the config space and after
> reset, it writes them back. In DomU, it obviously does not restore address
> bars and the device fails to respond via MMIO.
> 
> So, I've tried to allow DomU (via pciback driver) to write also the address
> bars but in this case, I'm facing to huge SLAB corruptions. In Dom0 or non-Xen
> kernel, it works pretty well.
> 
> Do you have any hints how to track it down?

This was tracked down by Alex Williamson some time ago. There is code in
drivers/xen/pciback/conf_space_capability_pm.c:pm_ctrl_write() to restore
bars after a D3->D0 transition. I guess either your kernel does not have
that fix, or the fix needs to be extended slightly to cover whatever case
you are seeing.

 -- Keir

Re: Permissive devices in Xen

[Lukas Hejtmanek]

On Sun, Jul 08, 2007 at 11:10:12AM +0100, Keir Fraser wrote:
> This was tracked down by Alex Williamson some time ago. There is code in
> drivers/xen/pciback/conf_space_capability_pm.c:pm_ctrl_write() to restore
> bars after a D3->D0 transition. I guess either your kernel does not have
> that fix, or the fix needs to be extended slightly to cover whatever case
> you are seeing.

Would you be happy with patch like this?

--- /root/xen-3.1.0-src/linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_header.c    2007-05-18 16:45:21.000000000 +0200
+++ conf_space_header.c 2007-07-10 01:05:57.165749185 +0200
@@ -75,8 +75,15 @@
         */
        if (value == ~PCI_ROM_ADDRESS_ENABLE)
                bar->which = 1;
-       else
+       else {
+               u32 tmpval;
+               pci_read_config_dword(dev, offset, &tmpval);
+               if(tmpval != bar->val && value == bar->val) {
+                       /* We are trying to restore bar value. This should be allowed. */
+                       pci_write_config_dword(dev, offset, bar->val);
+               }
                bar->which = 0;
+       }
 
        /* Do we need to support enabling/disabling the rom address here? */
 
@@ -102,8 +109,15 @@
         */
        if (value == ~0)
                bar->which = 1;
-       else
+       else {
+               u32 tmpval;
+               pci_read_config_dword(dev, offset, &tmpval);
+               if(tmpval != bar->val && value == bar->val) {
+                       /* We are trying to restore bar value. This should be allowed. */
+                       pci_write_config_dword(dev, offset, bar->val);
+               }
                bar->which = 0;
+       }
 
        return 0;
 }


-- 
Lukáš Hejtmánek

Re: Permissive devices in Xen

[Keir Fraser]

On 10/7/07 00:14, "Lukas Hejtmanek" <xhejtman@ics.muni.cz> wrote:

> On Sun, Jul 08, 2007 at 11:10:12AM +0100, Keir Fraser wrote:
>> This was tracked down by Alex Williamson some time ago. There is code in
>> drivers/xen/pciback/conf_space_capability_pm.c:pm_ctrl_write() to restore
>> bars after a D3->D0 transition. I guess either your kernel does not have
>> that fix, or the fix needs to be extended slightly to cover whatever case
>> you are seeing.
> 
> Would you be happy with patch like this?

Possibly, although I'm intrigued why the existing code to auto-reset the
BARs, in pm_ctrl_write(), doesn't work for you? Perhaps you could add some
tracing and find out? There might be a shorter patch that could be applied
there to get the existing code path working for you.

On the other hand, I don't really have anything against the patch you
submitted. If you have no joy with pm_ctrl_write() you can properly submit
your patch with a Signed-off-by line, and I'll apply it.

 -- Keir

Re: Permissive devices in Xen

[Lukas Hejtmanek]

On Tue, Jul 10, 2007 at 09:31:30AM +0100, Keir Fraser wrote:
> Possibly, although I'm intrigued why the existing code to auto-reset the
> BARs, in pm_ctrl_write(), doesn't work for you? Perhaps you could add some
> tracing and find out? There might be a shorter patch that could be applied
> there to get the existing code path working for you.

I think it is not really related to D3->D0 transition. Allowing to write
proper values into bars at drivers' request may be better approach in general.

> On the other hand, I don't really have anything against the patch you
> submitted. If you have no joy with pm_ctrl_write() you can properly submit
> your patch with a Signed-off-by line, and I'll apply it.

OK, I send it.

-- 
Lukáš Hejtmánek