[PATCH] fix XSA-46 regression with xend/xm

[PATCH] fix XSA-46 regression with xend/xm

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 2409 bytes --]

The hypervisor side changes for XSA-46 require the tool stack to now
always map the guest pIRQ before granting access permission to the
underlying host IRQ (GSI). This in particular requires that pciif.py
no longer can skip this step (assuming qemu would do it) for HVM
guests.

This in turn exposes, however, an inconsistency between xend and qemu:
The former wants to always establish 1:1 mappings between pIRQ and host
IRQ (for non-MSI only of course), while the latter always wants to
allocate an arbitrary mapping. Since the whole tool stack obviously
should always agree on the mapping model, make libxc enforce the 1:1
mapping as the more natural one (as well as being the one that allows
for easier debugging, since there no need to find out the extra
mapping). Users of libxc that want to establish a particular (rather
than an allocated) mapping are still free to do so, as well as tool
stacks not based on libxc wanting to implement an allocation based
model (which is why it's not the hypervisor that's being changed to
enforce either model).

Since libxl, like xend, already uses a 1:1 model, it's unaffected by
the libxc change (and it being unaffected by the original hypervisor
side changes is - afaict - simply due to qemu getting spawned at a
later point in time compared to the xend event flow).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)

--- a/tools/libxc/xc_physdev.c
+++ b/tools/libxc/xc_physdev.c
@@ -49,7 +49,7 @@ int xc_physdev_map_pirq(xc_interface *xc
     map.domid = domid;
     map.type = MAP_PIRQ_TYPE_GSI;
     map.index = index;
-    map.pirq = *pirq;
+    map.pirq = *pirq < 0 ? index : *pirq;
 
     rc = do_physdev_op(xch, PHYSDEVOP_map_pirq, &map, sizeof(map));
 
--- a/tools/python/xen/xend/server/pciif.py
+++ b/tools/python/xen/xend/server/pciif.py
@@ -340,7 +340,7 @@ class PciController(DevController):
                 raise VmError(('pci: failed to configure I/O memory on device '+
                             '%s - errno=%d')%(dev.name,rc))
 
-        if not self.vm.info.is_hvm() and dev.irq:
+        if dev.irq > 0:
             rc = xc.physdev_map_pirq(domid = fe_domid,
                                    index = dev.irq,
                                    pirq  = dev.irq)




[-- Attachment #2: xsa46-fix.patch --]
[-- Type: text/plain, Size: 2441 bytes --]

fix XSA-46 regression with xend/xm

The hypervisor side changes for XSA-46 require the tool stack to now
always map the guest pIRQ before granting access permission to the
underlying host IRQ (GSI). This in particular requires that pciif.py
no longer can skip this step (assuming qemu would do it) for HVM
guests.

This in turn exposes, however, an inconsistency between xend and qemu:
The former wants to always establish 1:1 mappings between pIRQ and host
IRQ (for non-MSI only of course), while the latter always wants to
allocate an arbitrary mapping. Since the whole tool stack obviously
should always agree on the mapping model, make libxc enforce the 1:1
mapping as the more natural one (as well as being the one that allows
for easier debugging, since there no need to find out the extra
mapping). Users of libxc that want to establish a particular (rather
than an allocated) mapping are still free to do so, as well as tool
stacks not based on libxc wanting to implement an allocation based
model (which is why it's not the hypervisor that's being changed to
enforce either model).

Since libxl, like xend, already uses a 1:1 model, it's unaffected by
the libxc change (and it being unaffected by the original hypervisor
side changes is - afaict - simply due to qemu getting spawned at a
later point in time compared to the xend event flow).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)

--- a/tools/libxc/xc_physdev.c
+++ b/tools/libxc/xc_physdev.c
@@ -49,7 +49,7 @@ int xc_physdev_map_pirq(xc_interface *xc
     map.domid = domid;
     map.type = MAP_PIRQ_TYPE_GSI;
     map.index = index;
-    map.pirq = *pirq;
+    map.pirq = *pirq < 0 ? index : *pirq;
 
     rc = do_physdev_op(xch, PHYSDEVOP_map_pirq, &map, sizeof(map));
 
--- a/tools/python/xen/xend/server/pciif.py
+++ b/tools/python/xen/xend/server/pciif.py
@@ -340,7 +340,7 @@ class PciController(DevController):
                 raise VmError(('pci: failed to configure I/O memory on device '+
                             '%s - errno=%d')%(dev.name,rc))
 
-        if not self.vm.info.is_hvm() and dev.irq:
+        if dev.irq > 0:
             rc = xc.physdev_map_pirq(domid = fe_domid,
                                    index = dev.irq,
                                    pirq  = dev.irq)

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] fix XSA-46 regression with xend/xm

[Ian Campbell]

On Tue, 2013-05-21 at 09:40 +0100, Jan Beulich wrote:
> The hypervisor side changes for XSA-46 require the tool stack to now
> always map the guest pIRQ before granting access permission to the
> underlying host IRQ (GSI). This in particular requires that pciif.py
> no longer can skip this step (assuming qemu would do it) for HVM
> guests.
> 
> This in turn exposes, however, an inconsistency between xend and qemu:
> The former wants to always establish 1:1 mappings between pIRQ and host
> IRQ (for non-MSI only of course), while the latter always wants to
> allocate an arbitrary mapping. Since the whole tool stack obviously
> should always agree on the mapping model, make libxc enforce the 1:1
> mapping as the more natural one (as well as being the one that allows
> for easier debugging, since there no need to find out the extra
> mapping). Users of libxc that want to establish a particular (rather
> than an allocated) mapping are still free to do so, as well as tool
> stacks not based on libxc wanting to implement an allocation based
> model (which is why it's not the hypervisor that's being changed to
> enforce either model).
> 
> Since libxl, like xend, already uses a 1:1 model, it's unaffected by
> the libxc change (and it being unaffected by the original hypervisor
> side changes is - afaict - simply due to qemu getting spawned at a
> later point in time compared to the xend event flow).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
> Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)

In both cases tested with xend rather than xl or both?

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> --- a/tools/libxc/xc_physdev.c
> +++ b/tools/libxc/xc_physdev.c
> @@ -49,7 +49,7 @@ int xc_physdev_map_pirq(xc_interface *xc
>      map.domid = domid;
>      map.type = MAP_PIRQ_TYPE_GSI;
>      map.index = index;
> -    map.pirq = *pirq;
> +    map.pirq = *pirq < 0 ? index : *pirq;
>  
>      rc = do_physdev_op(xch, PHYSDEVOP_map_pirq, &map, sizeof(map));
>  
> --- a/tools/python/xen/xend/server/pciif.py
> +++ b/tools/python/xen/xend/server/pciif.py
> @@ -340,7 +340,7 @@ class PciController(DevController):
>                  raise VmError(('pci: failed to configure I/O memory on device '+
>                              '%s - errno=%d')%(dev.name,rc))
>  
> -        if not self.vm.info.is_hvm() and dev.irq:
> +        if dev.irq > 0:
>              rc = xc.physdev_map_pirq(domid = fe_domid,
>                                     index = dev.irq,
>                                     pirq  = dev.irq)
> 
> 
> 

Re: [PATCH] fix XSA-46 regression with xend/xm

[Gordan Bobic]

 On Tue, 21 May 2013 09:56:27 +0100, Ian Campbell 
 <Ian.Campbell@citrix.com> wrote:
> On Tue, 2013-05-21 at 09:40 +0100, Jan Beulich wrote:
>> The hypervisor side changes for XSA-46 require the tool stack to now
>> always map the guest pIRQ before granting access permission to the
>> underlying host IRQ (GSI). This in particular requires that pciif.py
>> no longer can skip this step (assuming qemu would do it) for HVM
>> guests.
>>
>> This in turn exposes, however, an inconsistency between xend and 
>> qemu:
>> The former wants to always establish 1:1 mappings between pIRQ and 
>> host
>> IRQ (for non-MSI only of course), while the latter always wants to
>> allocate an arbitrary mapping. Since the whole tool stack obviously
>> should always agree on the mapping model, make libxc enforce the 1:1
>> mapping as the more natural one (as well as being the one that 
>> allows
>> for easier debugging, since there no need to find out the extra
>> mapping). Users of libxc that want to establish a particular (rather
>> than an allocated) mapping are still free to do so, as well as tool
>> stacks not based on libxc wanting to implement an allocation based
>> model (which is why it's not the hypervisor that's being changed to
>> enforce either model).
>>
>> Since libxl, like xend, already uses a 1:1 model, it's unaffected by
>> the libxc change (and it being unaffected by the original hypervisor
>> side changes is - afaict - simply due to qemu getting spawned at a
>> later point in time compared to the xend event flow).
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
>> Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)
>
> In both cases tested with xend rather than xl or both?

 My tests were done using xm only. I never tried xl so don't know if
 it was affected in the first place. I will try xl tonight to verify
 that it works.

 Gordan

Re: [PATCH] fix XSA-46 regression with xend/xm

[Jan Beulich]

>>> On 21.05.13 at 10:56, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Tue, 2013-05-21 at 09:40 +0100, Jan Beulich wrote:
>> The hypervisor side changes for XSA-46 require the tool stack to now
>> always map the guest pIRQ before granting access permission to the
>> underlying host IRQ (GSI). This in particular requires that pciif.py
>> no longer can skip this step (assuming qemu would do it) for HVM
>> guests.
>> 
>> This in turn exposes, however, an inconsistency between xend and qemu:
>> The former wants to always establish 1:1 mappings between pIRQ and host
>> IRQ (for non-MSI only of course), while the latter always wants to
>> allocate an arbitrary mapping. Since the whole tool stack obviously
>> should always agree on the mapping model, make libxc enforce the 1:1
>> mapping as the more natural one (as well as being the one that allows
>> for easier debugging, since there no need to find out the extra
>> mapping). Users of libxc that want to establish a particular (rather
>> than an allocated) mapping are still free to do so, as well as tool
>> stacks not based on libxc wanting to implement an allocation based
>> model (which is why it's not the hypervisor that's being changed to
>> enforce either model).
>> 
>> Since libxl, like xend, already uses a 1:1 model, it's unaffected by
>> the libxc change (and it being unaffected by the original hypervisor
>> side changes is - afaict - simply due to qemu getting spawned at a
>> later point in time compared to the xend event flow).
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
>> Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)
> 
> In both cases tested with xend rather than xl or both?

Both of them run xend only afaik.

> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
>> --- a/tools/libxc/xc_physdev.c
>> +++ b/tools/libxc/xc_physdev.c
>> @@ -49,7 +49,7 @@ int xc_physdev_map_pirq(xc_interface *xc
>>      map.domid = domid;
>>      map.type = MAP_PIRQ_TYPE_GSI;
>>      map.index = index;
>> -    map.pirq = *pirq;
>> +    map.pirq = *pirq < 0 ? index : *pirq;
>>  
>>      rc = do_physdev_op(xch, PHYSDEVOP_map_pirq, &map, sizeof(map));
>>  
>> --- a/tools/python/xen/xend/server/pciif.py
>> +++ b/tools/python/xen/xend/server/pciif.py
>> @@ -340,7 +340,7 @@ class PciController(DevController):
>>                  raise VmError(('pci: failed to configure I/O memory on 
> device '+
>>                              '%s - errno=%d')%(dev.name,rc))
>>  
>> -        if not self.vm.info.is_hvm() and dev.irq:
>> +        if dev.irq > 0:
>>              rc = xc.physdev_map_pirq(domid = fe_domid,
>>                                     index = dev.irq,
>>                                     pirq  = dev.irq)
>> 
>> 
>> 

Re: [PATCH] fix XSA-46 regression with xend/xm

[Gordan Bobic]

On 05/21/2013 09:56 AM, Ian Campbell wrote:
> On Tue, 2013-05-21 at 09:40 +0100, Jan Beulich wrote:
>> The hypervisor side changes for XSA-46 require the tool stack to now
>> always map the guest pIRQ before granting access permission to the
>> underlying host IRQ (GSI). This in particular requires that pciif.py
>> no longer can skip this step (assuming qemu would do it) for HVM
>> guests.
>>
>> This in turn exposes, however, an inconsistency between xend and qemu:
>> The former wants to always establish 1:1 mappings between pIRQ and host
>> IRQ (for non-MSI only of course), while the latter always wants to
>> allocate an arbitrary mapping. Since the whole tool stack obviously
>> should always agree on the mapping model, make libxc enforce the 1:1
>> mapping as the more natural one (as well as being the one that allows
>> for easier debugging, since there no need to find out the extra
>> mapping). Users of libxc that want to establish a particular (rather
>> than an allocated) mapping are still free to do so, as well as tool
>> stacks not based on libxc wanting to implement an allocation based
>> model (which is why it's not the hypervisor that's being changed to
>> enforce either model).
>>
>> Since libxl, like xend, already uses a 1:1 model, it's unaffected by
>> the libxc change (and it being unaffected by the original hypervisor
>> side changes is - afaict - simply due to qemu getting spawned at a
>> later point in time compared to the xend event flow).
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
>> Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)
>
> In both cases tested with xend rather than xl or both?

I can confirm that my VMs start fine with xl with this patch applied.

Gordan

Re: [PATCH] fix XSA-46 regression with xend/xm

[George Dunlap]

On Wed, May 22, 2013 at 9:51 PM, Gordan Bobic <gordan@bobich.net> wrote:
> On 05/21/2013 09:56 AM, Ian Campbell wrote:
>>
>> On Tue, 2013-05-21 at 09:40 +0100, Jan Beulich wrote:
>>>
>>> The hypervisor side changes for XSA-46 require the tool stack to now
>>> always map the guest pIRQ before granting access permission to the
>>> underlying host IRQ (GSI). This in particular requires that pciif.py
>>> no longer can skip this step (assuming qemu would do it) for HVM
>>> guests.
>>>
>>> This in turn exposes, however, an inconsistency between xend and qemu:
>>> The former wants to always establish 1:1 mappings between pIRQ and host
>>> IRQ (for non-MSI only of course), while the latter always wants to
>>> allocate an arbitrary mapping. Since the whole tool stack obviously
>>> should always agree on the mapping model, make libxc enforce the 1:1
>>> mapping as the more natural one (as well as being the one that allows
>>> for easier debugging, since there no need to find out the extra
>>> mapping). Users of libxc that want to establish a particular (rather
>>> than an allocated) mapping are still free to do so, as well as tool
>>> stacks not based on libxc wanting to implement an allocation based
>>> model (which is why it's not the hypervisor that's being changed to
>>> enforce either model).
>>>
>>> Since libxl, like xend, already uses a 1:1 model, it's unaffected by
>>> the libxc change (and it being unaffected by the original hypervisor
>>> side changes is - afaict - simply due to qemu getting spawned at a
>>> later point in time compared to the xend event flow).
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
>>> Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)
>>
>>
>> In both cases tested with xend rather than xl or both?
>
>
> I can confirm that my VMs start fine with xl with this patch applied.

...and they are HVM guests with devices passed through?

 -George

Re: [PATCH] fix XSA-46 regression with xend/xm

[Gordan Bobic]

 On Fri, 24 May 2013 12:19:32 +0100, George Dunlap 
 <George.Dunlap@eu.citrix.com> wrote:
> On Wed, May 22, 2013 at 9:51 PM, Gordan Bobic <gordan@bobich.net> 
> wrote:
>> On 05/21/2013 09:56 AM, Ian Campbell wrote:
>>>
>>> On Tue, 2013-05-21 at 09:40 +0100, Jan Beulich wrote:
>>>>
>>>> The hypervisor side changes for XSA-46 require the tool stack to 
>>>> now
>>>> always map the guest pIRQ before granting access permission to the
>>>> underlying host IRQ (GSI). This in particular requires that 
>>>> pciif.py
>>>> no longer can skip this step (assuming qemu would do it) for HVM
>>>> guests.
>>>>
>>>> This in turn exposes, however, an inconsistency between xend and 
>>>> qemu:
>>>> The former wants to always establish 1:1 mappings between pIRQ and 
>>>> host
>>>> IRQ (for non-MSI only of course), while the latter always wants to
>>>> allocate an arbitrary mapping. Since the whole tool stack 
>>>> obviously
>>>> should always agree on the mapping model, make libxc enforce the 
>>>> 1:1
>>>> mapping as the more natural one (as well as being the one that 
>>>> allows
>>>> for easier debugging, since there no need to find out the extra
>>>> mapping). Users of libxc that want to establish a particular 
>>>> (rather
>>>> than an allocated) mapping are still free to do so, as well as 
>>>> tool
>>>> stacks not based on libxc wanting to implement an allocation based
>>>> model (which is why it's not the hypervisor that's being changed 
>>>> to
>>>> enforce either model).
>>>>
>>>> Since libxl, like xend, already uses a 1:1 model, it's unaffected 
>>>> by
>>>> the libxc change (and it being unaffected by the original 
>>>> hypervisor
>>>> side changes is - afaict - simply due to qemu getting spawned at a
>>>> later point in time compared to the xend event flow).
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
>>>> Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)
>>>
>>>
>>> In both cases tested with xend rather than xl or both?
>>
>>
>> I can confirm that my VMs start fine with xl with this patch 
>> applied.
>
> ...and they are HVM guests with devices passed through?

 Yes, HVM guests with PCI passthrough NIC, VGA passthrough, and USB
 passthrough.

 Gordan

Re: [PATCH] fix XSA-46 regression with xend/xm

[George Dunlap]

On 24/05/13 12:41, Gordan Bobic wrote:
> On Fri, 24 May 2013 12:19:32 +0100, George Dunlap 
> <George.Dunlap@eu.citrix.com> wrote:
>> On Wed, May 22, 2013 at 9:51 PM, Gordan Bobic <gordan@bobich.net> wrote:
>>> On 05/21/2013 09:56 AM, Ian Campbell wrote:
>>>>
>>>> On Tue, 2013-05-21 at 09:40 +0100, Jan Beulich wrote:
>>>>>
>>>>> The hypervisor side changes for XSA-46 require the tool stack to now
>>>>> always map the guest pIRQ before granting access permission to the
>>>>> underlying host IRQ (GSI). This in particular requires that pciif.py
>>>>> no longer can skip this step (assuming qemu would do it) for HVM
>>>>> guests.
>>>>>
>>>>> This in turn exposes, however, an inconsistency between xend and 
>>>>> qemu:
>>>>> The former wants to always establish 1:1 mappings between pIRQ and 
>>>>> host
>>>>> IRQ (for non-MSI only of course), while the latter always wants to
>>>>> allocate an arbitrary mapping. Since the whole tool stack obviously
>>>>> should always agree on the mapping model, make libxc enforce the 1:1
>>>>> mapping as the more natural one (as well as being the one that allows
>>>>> for easier debugging, since there no need to find out the extra
>>>>> mapping). Users of libxc that want to establish a particular (rather
>>>>> than an allocated) mapping are still free to do so, as well as tool
>>>>> stacks not based on libxc wanting to implement an allocation based
>>>>> model (which is why it's not the hypervisor that's being changed to
>>>>> enforce either model).
>>>>>
>>>>> Since libxl, like xend, already uses a 1:1 model, it's unaffected by
>>>>> the libxc change (and it being unaffected by the original hypervisor
>>>>> side changes is - afaict - simply due to qemu getting spawned at a
>>>>> later point in time compared to the xend event flow).
>>>>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
>>>>> Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)
>>>>
>>>>
>>>> In both cases tested with xend rather than xl or both?
>>>
>>>
>>> I can confirm that my VMs start fine with xl with this patch applied.
>>
>> ...and they are HVM guests with devices passed through?
>
> Yes, HVM guests with PCI passthrough NIC, VGA passthrough, and USB
> passthrough.

Great, thanks!  I'll check this off my list.

  -George

Re: [PATCH] fix XSA-46 regression with xend/xm

[Andrew Cooper]

On 21/05/13 09:40, Jan Beulich wrote:
> The hypervisor side changes for XSA-46 require the tool stack to now
> always map the guest pIRQ before granting access permission to the
> underlying host IRQ (GSI). This in particular requires that pciif.py
> no longer can skip this step (assuming qemu would do it) for HVM
> guests.
>
> This in turn exposes, however, an inconsistency between xend and qemu:
> The former wants to always establish 1:1 mappings between pIRQ and host
> IRQ (for non-MSI only of course), while the latter always wants to
> allocate an arbitrary mapping. Since the whole tool stack obviously
> should always agree on the mapping model, make libxc enforce the 1:1
> mapping as the more natural one (as well as being the one that allows
> for easier debugging, since there no need to find out the extra
> mapping). Users of libxc that want to establish a particular (rather
> than an allocated) mapping are still free to do so, as well as tool
> stacks not based on libxc wanting to implement an allocation based
> model (which is why it's not the hypervisor that's being changed to
> enforce either model).
>
> Since libxl, like xend, already uses a 1:1 model, it's unaffected by
> the libxc change (and it being unaffected by the original hypervisor
> side changes is - afaict - simply due to qemu getting spawned at a
> later point in time compared to the xend event flow).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
> Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> --- a/tools/libxc/xc_physdev.c
> +++ b/tools/libxc/xc_physdev.c
> @@ -49,7 +49,7 @@ int xc_physdev_map_pirq(xc_interface *xc
>      map.domid = domid;
>      map.type = MAP_PIRQ_TYPE_GSI;
>      map.index = index;
> -    map.pirq = *pirq;
> +    map.pirq = *pirq < 0 ? index : *pirq;
>  
>      rc = do_physdev_op(xch, PHYSDEVOP_map_pirq, &map, sizeof(map));
>  
> --- a/tools/python/xen/xend/server/pciif.py
> +++ b/tools/python/xen/xend/server/pciif.py
> @@ -340,7 +340,7 @@ class PciController(DevController):
>                  raise VmError(('pci: failed to configure I/O memory on device '+
>                              '%s - errno=%d')%(dev.name,rc))
>  
> -        if not self.vm.info.is_hvm() and dev.irq:
> +        if dev.irq > 0:
>              rc = xc.physdev_map_pirq(domid = fe_domid,
>                                     index = dev.irq,
>                                     pirq  = dev.irq)
>
>
>

Re: [PATCH] fix XSA-46 regression with xend/xm

[George Dunlap]

On Tue, May 21, 2013 at 9:40 AM, Jan Beulich <JBeulich@suse.com> wrote:
> The hypervisor side changes for XSA-46 require the tool stack to now
> always map the guest pIRQ before granting access permission to the
> underlying host IRQ (GSI). This in particular requires that pciif.py
> no longer can skip this step (assuming qemu would do it) for HVM
> guests.
>
> This in turn exposes, however, an inconsistency between xend and qemu:
> The former wants to always establish 1:1 mappings between pIRQ and host
> IRQ (for non-MSI only of course), while the latter always wants to
> allocate an arbitrary mapping. Since the whole tool stack obviously
> should always agree on the mapping model, make libxc enforce the 1:1
> mapping as the more natural one (as well as being the one that allows
> for easier debugging, since there no need to find out the extra
> mapping). Users of libxc that want to establish a particular (rather
> than an allocated) mapping are still free to do so, as well as tool
> stacks not based on libxc wanting to implement an allocation based
> model (which is why it's not the hypervisor that's being changed to
> enforce either model).
>
> Since libxl, like xend, already uses a 1:1 model, it's unaffected by
> the libxc change (and it being unaffected by the original hypervisor
> side changes is - afaict - simply due to qemu getting spawned at a
> later point in time compared to the xend event flow).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
> Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)

I think to get a release ack, someone will need to commit to testing
it with xl for 4.3.

 -George

Re: [PATCH] fix XSA-46 regression with xend/xm

[Jan Beulich]

>>> On 21.05.13 at 11:44, George Dunlap <George.Dunlap@eu.citrix.com> wrote:
> On Tue, May 21, 2013 at 9:40 AM, Jan Beulich <JBeulich@suse.com> wrote:
>> The hypervisor side changes for XSA-46 require the tool stack to now
>> always map the guest pIRQ before granting access permission to the
>> underlying host IRQ (GSI). This in particular requires that pciif.py
>> no longer can skip this step (assuming qemu would do it) for HVM
>> guests.
>>
>> This in turn exposes, however, an inconsistency between xend and qemu:
>> The former wants to always establish 1:1 mappings between pIRQ and host
>> IRQ (for non-MSI only of course), while the latter always wants to
>> allocate an arbitrary mapping. Since the whole tool stack obviously
>> should always agree on the mapping model, make libxc enforce the 1:1
>> mapping as the more natural one (as well as being the one that allows
>> for easier debugging, since there no need to find out the extra
>> mapping). Users of libxc that want to establish a particular (rather
>> than an allocated) mapping are still free to do so, as well as tool
>> stacks not based on libxc wanting to implement an allocation based
>> model (which is why it's not the hypervisor that's being changed to
>> enforce either model).
>>
>> Since libxl, like xend, already uses a 1:1 model, it's unaffected by
>> the libxc change (and it being unaffected by the original hypervisor
>> side changes is - afaict - simply due to qemu getting spawned at a
>> later point in time compared to the xend event flow).
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
>> Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)
> 
> I think to get a release ack, someone will need to commit to testing
> it with xl for 4.3.

It is pretty obvious (see the description) that xl is unaffected.

Jan

Re: [PATCH] fix XSA-46 regression with xend/xm

[George Dunlap]

On 05/21/2013 10:56 AM, Jan Beulich wrote:
>>>> On 21.05.13 at 11:44, George Dunlap <George.Dunlap@eu.citrix.com> wrote:
>> On Tue, May 21, 2013 at 9:40 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>> The hypervisor side changes for XSA-46 require the tool stack to now
>>> always map the guest pIRQ before granting access permission to the
>>> underlying host IRQ (GSI). This in particular requires that pciif.py
>>> no longer can skip this step (assuming qemu would do it) for HVM
>>> guests.
>>>
>>> This in turn exposes, however, an inconsistency between xend and qemu:
>>> The former wants to always establish 1:1 mappings between pIRQ and host
>>> IRQ (for non-MSI only of course), while the latter always wants to
>>> allocate an arbitrary mapping. Since the whole tool stack obviously
>>> should always agree on the mapping model, make libxc enforce the 1:1
>>> mapping as the more natural one (as well as being the one that allows
>>> for easier debugging, since there no need to find out the extra
>>> mapping). Users of libxc that want to establish a particular (rather
>>> than an allocated) mapping are still free to do so, as well as tool
>>> stacks not based on libxc wanting to implement an allocation based
>>> model (which is why it's not the hypervisor that's being changed to
>>> enforce either model).
>>>
>>> Since libxl, like xend, already uses a 1:1 model, it's unaffected by
>>> the libxc change (and it being unaffected by the original hypervisor
>>> side changes is - afaict - simply due to qemu getting spawned at a
>>> later point in time compared to the xend event flow).
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
>>> Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)
>>
>> I think to get a release ack, someone will need to commit to testing
>> it with xl for 4.3.
>
> It is pretty obvious (see the description) that xl is unaffected.

It's pretty obvious that you think so, but it's my job to be skeptical. 
:-)  If both xend and xl assume a 1:1 model, and this patch changes 
things for xend, why is it not possible for this to have an effect on 
xl?  You have a guess, but it's marked "afaict".

In any case it should be pretty straightforward to have done.  We could 
even check it in and just put a release blocker, "Someone tests 
pass-through with xl" to make sure we don't forget it.

  -George

Re: [PATCH] fix XSA-46 regression with xend/xm

[Jan Beulich]

>>> On 21.05.13 at 12:06, George Dunlap <george.dunlap@eu.citrix.com> wrote:
> On 05/21/2013 10:56 AM, Jan Beulich wrote:
>>>>> On 21.05.13 at 11:44, George Dunlap <George.Dunlap@eu.citrix.com> wrote:
>>> On Tue, May 21, 2013 at 9:40 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> The hypervisor side changes for XSA-46 require the tool stack to now
>>>> always map the guest pIRQ before granting access permission to the
>>>> underlying host IRQ (GSI). This in particular requires that pciif.py
>>>> no longer can skip this step (assuming qemu would do it) for HVM
>>>> guests.
>>>>
>>>> This in turn exposes, however, an inconsistency between xend and qemu:
>>>> The former wants to always establish 1:1 mappings between pIRQ and host
>>>> IRQ (for non-MSI only of course), while the latter always wants to
>>>> allocate an arbitrary mapping. Since the whole tool stack obviously
>>>> should always agree on the mapping model, make libxc enforce the 1:1
>>>> mapping as the more natural one (as well as being the one that allows
>>>> for easier debugging, since there no need to find out the extra
>>>> mapping). Users of libxc that want to establish a particular (rather
>>>> than an allocated) mapping are still free to do so, as well as tool
>>>> stacks not based on libxc wanting to implement an allocation based
>>>> model (which is why it's not the hypervisor that's being changed to
>>>> enforce either model).
>>>>
>>>> Since libxl, like xend, already uses a 1:1 model, it's unaffected by
>>>> the libxc change (and it being unaffected by the original hypervisor
>>>> side changes is - afaict - simply due to qemu getting spawned at a
>>>> later point in time compared to the xend event flow).
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1)
>>>> Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2)
>>>
>>> I think to get a release ack, someone will need to commit to testing
>>> it with xl for 4.3.
>>
>> It is pretty obvious (see the description) that xl is unaffected.
> 
> It's pretty obvious that you think so, but it's my job to be skeptical. 
> :-)  If both xend and xl assume a 1:1 model, and this patch changes 
> things for xend, why is it not possible for this to have an effect on 
> xl?  You have a guess, but it's marked "afaict".
> 
> In any case it should be pretty straightforward to have done.  We could 
> even check it in and just put a release blocker, "Someone tests 
> pass-through with xl" to make sure we don't forget it.

Please do so then, because I had committed it already before you
even raised your concern (on the basis of it being a bug fix).

Jan