Xen devel - Oct 2007 - [PATCH 0/5] Add MSI support to xen environment

Hi, 
The followed series patches add MSI support to xen environment. Please
have a look on it and give me any suggestion and I''d like to update
accordingly.

The basic idea including:
1) Keep vector global resource owned by xen, while split pirq into per
domain information.
2) domain0 kernel will operate msi resource for domain0/domU, while QEMU
will operate MSI resource for HVM domain.
3) Xen will do EOI for MSI interrupt.

pirq_per_domain.patch is mainly changes to xen source tree for
per-domain pirq.
domain_pirq_kernel.patch is changes to linux tree for per-domain pirq.

msi_irq_xen.patch changes xen to support msi for domain0/domainU.
msi_kernel.patch changes linux tree to support msi.

msi_passthrough.patch add msi support to passthrough hvm domain.

Signed-off-by: Yunhong Jiang <yunhong.jiang@intel.com>

Thanks
Yunhong Jiang

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

As you are intending this not only for dom0, but also for domU - what are
the implications of a domU (intentionally or accidentally) programming the
address and/or data values to something other than the intended values?

Jan
>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 25.10.07
18:46 >>>
Hi, 
The followed series patches add MSI support to xen environment. Please
have a look on it and give me any suggestion and I''d like to update
accordingly.

The basic idea including:
1) Keep vector global resource owned by xen, while split pirq into per
domain information.
2) domain0 kernel will operate msi resource for domain0/domU, while QEMU
will operate MSI resource for HVM domain.
3) Xen will do EOI for MSI interrupt.

pirq_per_domain.patch is mainly changes to xen source tree for
per-domain pirq.
domain_pirq_kernel.patch is changes to linux tree for per-domain pirq.

msi_irq_xen.patch changes xen to support msi for domain0/domainU.
msi_kernel.patch changes linux tree to support msi.

msi_passthrough.patch add msi support to passthrough hvm domain.

Signed-off-by: Yunhong Jiang <yunhong.jiang@intel.com>

Thanks
Yunhong Jiang

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


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

Jan Beulich <mailto:jbeulich@novell.com> wrote:
> As you are intending this not only for dom0, but also for domU - what
are
> the implications of a domU (intentionally or accidentally)
> programming the
> address and/or data values to something other than the intended
values?

Thanks for your review. For MSI config space, domU will access them
through PCI backend, and PCI backend will block all write unless stated
clearly by one config entry.
For MSI-X , yes, we need block all access to the MMIO range through mmio
rangeset. I will add that logic to control panel.

-- Yunhong Jiang
> 
> Jan
> 
>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com>
25.10.07 18:46 >>>
> Hi,
> The followed series patches add MSI support to xen environment. Please
> have a look on it and give me any suggestion and I''d like to
update
> accordingly. 
> 
> The basic idea including:
> 1) Keep vector global resource owned by xen, while split pirq into per
> domain information. 2) domain0 kernel will operate msi resource for
> domain0/domU, 
> while QEMU
> will operate MSI resource for HVM domain.
> 3) Xen will do EOI for MSI interrupt.
> 
> pirq_per_domain.patch is mainly changes to xen source tree for
per-domain
> pirq. domain_pirq_kernel.patch is changes to linux tree for per-domain
pirq.
> 
> msi_irq_xen.patch changes xen to support msi for domain0/domainU.
> msi_kernel.patch changes linux tree to support msi.
> 
> msi_passthrough.patch add msi support to passthrough hvm domain.
> 
> Signed-off-by: Yunhong Jiang <yunhong.jiang@intel.com>
> 
> Thanks
> Yunhong Jiang
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

The PHYSDEVOP_map_irq concept is a bit broken. Firstly the new op is not
protected from domU usage. Secondly it looks like the current code will
break with old dom0 kernels that do not use PHYSDEVOP_map_irq. That''s
not
allowed.

I think it''d be better for pciback to poke the irq configuration into
xenstore, and then that can be picked up by the domain builder before
starting the domain. Then xend can be responsible for actually setting up
the mapping to a pirq, and all these new hypercalls can be domctls. The
advantage of this is that they''re not then set in stone. I''m
not confident
about overhauling the irq interface and declaring it fully baked just before
a tree freeze.

 -- Keir

On 25/10/07 17:46, "Jiang, Yunhong" <yunhong.jiang@intel.com>
wrote:
> Hi, 
> The followed series patches add MSI support to xen environment. Please
> have a look on it and give me any suggestion and I''d like to
update
> accordingly.
> 
> The basic idea including:
> 1) Keep vector global resource owned by xen, while split pirq into per
> domain information.
> 2) domain0 kernel will operate msi resource for domain0/domU, while QEMU
> will operate MSI resource for HVM domain.
> 3) Xen will do EOI for MSI interrupt.
> 
> pirq_per_domain.patch is mainly changes to xen source tree for
> per-domain pirq.
> domain_pirq_kernel.patch is changes to linux tree for per-domain pirq.
> 
> msi_irq_xen.patch changes xen to support msi for domain0/domainU.
> msi_kernel.patch changes linux tree to support msi.
> 
> msi_passthrough.patch add msi support to passthrough hvm domain.
> 
> Signed-off-by: Yunhong Jiang <yunhong.jiang@intel.com>
> 
> Thanks
> Yunhong Jiang


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

Keir Fraser <mailto:Keir.Fraser@cl.cam.ac.uk>
wrote:
> The PHYSDEVOP_map_irq concept is a bit broken. Firstly the new
> op is not
> protected from domU usage. Secondly it looks like the current code
will
> break with old dom0 kernels that do not use PHYSDEVOP_map_irq.
> That''s not
> allowed.
For the first point , what I thought is, as a interface,  domU may need
to map vector into it''s 
own pirq namespace, but yes, we need add permissoin check to make sure
it is legal. The only question is, we may need permission to both vector
and physical IRQ.
For the second point, sorry I missed this point when doing the patch. An
alternative method is, for the ioapic interrupt, we do the map to
domain0 when assign_irq_vector(), and do the mapping for domU in control
panel, before start the domain. I think this is similar to your option
below.
> 
> I think it''d be better for pciback to poke the irq configuration
into
> xenstore, and then that can be picked up by the domain builder before
> starting the domain. Then xend can be responsible for actually
> setting up
> the mapping to a pirq, and all these new hypercalls can be domctls.
The
> advantage of this is that they''re not then set in stone.
I''m
> not confident
> about overhauling the irq interface and declaring it fully
> baked just before
> a tree freeze.
Understand, so when will the tree be freeze and be open again? Do you
have any plan? What I wonder is,  do you think of the map_irq change, is
required in future? Shall we split the pirq and physical irq in future?

For pciback to poke irq configuration, I think for IOAPIC irq is done
already, we only need add the mapping at that time. For MSI , domain U
will do that dynamically, so not sure if viable, especially considering
for MSI-x, only driver knows how many entry will be used.

-- Yunhong Jiang
> 
> -- Keir
> 
> On 25/10/07 17:46, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
> 
>> Hi,
>> The followed series patches add MSI support to xen environment.
Please
>> have a look on it and give me any suggestion and I''d like to
update
>> accordingly. 
>> 
>> The basic idea including:
>> 1) Keep vector global resource owned by xen, while split pirq into
per
>> domain information. 2) domain0 kernel will operate msi resource for
>> domain0/domU, while QEMU will operate MSI resource for HVM domain.
>> 3) Xen will do EOI for MSI interrupt.
>> 
>> pirq_per_domain.patch is mainly changes to xen source tree for
per-domain
>> pirq. domain_pirq_kernel.patch is changes to linux tree for
per-domain
>> pirq. 
>> 
>> msi_irq_xen.patch changes xen to support msi for domain0/domainU.
>> msi_kernel.patch changes linux tree to support msi.
>> 
>> msi_passthrough.patch add msi support to passthrough hvm domain.
>> 
>> Signed-off-by: Yunhong Jiang <yunhong.jiang@intel.com>
>> 
>> Thanks
>> Yunhong Jiang
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 26/10/07 15:35, "Jiang, Yunhong" <yunhong.jiang@intel.com>
wrote:
> For pciback to poke irq configuration, I think for IOAPIC irq is done
> already, we only need add the mapping at that time. For MSI , domain U
> will do that dynamically, so not sure if viable, especially considering
> for MSI-x, only driver knows how many entry will be used.
dom0 needs to be involved, since we can''t let domU map any arbitrary
vector
into its pirq space. Since dom0 has to be involved in access control to the
irq vector space, can''t it do the mapping?

 -- Keir



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

xen-devel-bounces@lists.xensource.com <> wrote:
> On 26/10/07 15:35, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
> 
>> For pciback to poke irq configuration, I think for IOAPIC irq is done
>> already, we only need add the mapping at that time. For MSI , domain
U
>> will do that dynamically, so not sure if viable, especially
considering
>> for MSI-x, only driver knows how many entry will be used.
> 
> dom0 needs to be involved, since we can''t let domU map any
arbitrary
vector
> into its pirq space. Since dom0 has to be involved in access control
to the
> irq vector space, can''t it do the mapping?
yes, what I mean is, "before starting the domain" works for IOAPIC
IRQ,
not MSI. MSI will still through communcation between PCI
frontend/backend directly.
> 
> -- Keir
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 26/10/07 16:02, "Jiang, Yunhong" <yunhong.jiang@intel.com>
wrote:
>> dom0 needs to be involved, since we can''t let domU map any
arbitrary
> vector
>> into its pirq space. Since dom0 has to be involved in access control
> to the
>> irq vector space, can''t it do the mapping?
> 
> yes, what I mean is, "before starting the domain" works for
IOAPIC IRQ,
> not MSI. MSI will still through communcation between PCI
> frontend/backend directly.
Oh, I see. Then it probably has to be a phydevop and let dom0 kernel do it.
But there should be no reason to let domU use the map_irq physdev_op at all.

 -- Keir



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

So, Keir, how about the attached method for the per-domain pirq?
Now there is no need to change domain0 any more. Also domain U can''t do
the map. I verified current domain0/domU works on it.
But it still changes the control panel and hope that is acceptable.

Thanks
Yunhong Jiang

xen-devel-bounces@lists.xensource.com <> wrote:
> On 26/10/07 16:02, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
> 
>>> dom0 needs to be involved, since we can''t let domU map any
arbitrary
>>> vector into its pirq space. Since dom0 has to be involved in access
>>> control to the irq vector space, can''t it do the mapping?
>> 
>> yes, what I mean is, "before starting the domain" works for
IOAPIC
IRQ,
>> not MSI. MSI will still through communcation between PCI
>> frontend/backend directly.
> 
> Oh, I see. Then it probably has to be a phydevop and let dom0 kernel
do it.
> But there should be no reason to let domU use the map_irq physdev_op
at all.
> 
> -- Keir
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

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

Er, maybe. Does this slot in with some of the other patches you previously
sent? Are we shooting to get this into 3.2.0 (scary!)?

 -- Keir

On 30/10/07 14:27, "Jiang, Yunhong" <yunhong.jiang@intel.com>
wrote:
> So, Keir, how about the attached method for the per-domain pirq?
> Now there is no need to change domain0 any more. Also domain U
can''t do
> the map. I verified current domain0/domU works on it.
> But it still changes the control panel and hope that is acceptable.
> 
> Thanks
> Yunhong Jiang
> 
> xen-devel-bounces@lists.xensource.com <> wrote:
>> On 26/10/07 16:02, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
>> 
>>>> dom0 needs to be involved, since we can''t let domU map
any arbitrary
>>>> vector into its pirq space. Since dom0 has to be involved in
access
>>>> control to the irq vector space, can''t it do the
mapping?
>>> 
>>> yes, what I mean is, "before starting the domain" works
for IOAPIC
> IRQ,
>>> not MSI. MSI will still through communcation between PCI
>>> frontend/backend directly.
>> 
>> Oh, I see. Then it probably has to be a phydevop and let dom0 kernel
> do it.
>> But there should be no reason to let domU use the map_irq physdev_op
> at all.
>> 
>> -- Keir
>> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel


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

Keir, attached is the updated patchset.

A xen option "msi_irq_enable" is added. the pirq_domain method will be
enabled only when msi_irq_enable=1.

pirq_per_domain.patch is the changes for per domain pirq. When
msi_irq_enable=0, it in fact still use old method.
msi_irq_xen.patch changes xen for MSI  support. Currently we are using
ACK_NEW method to avoid possible interrupt storm in some device.
msi_passthrough.patch add MSI support to VT-d domain.
msix-permission.patch is to disable MSI-x MMIO permission for domain U.

msi_kernel.patch add MSI/MSI-X support to domain0/domainU.
msi_disable.patch changes the configuration file. current settting is to
disable MSI by default.

-- Yunhong Jiang

Keir Fraser <mailto:Keir.Fraser@cl.cam.ac.uk>
wrote:
> Er, maybe. Does this slot in with some of the other patches
> you previously
> sent? Are we shooting to get this into 3.2.0 (scary!)?
> 
> -- Keir
> 
> On 30/10/07 14:27, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
> 
>> So, Keir, how about the attached method for the per-domain pirq?
>> Now there is no need to change domain0 any more. Also domain U
can''t
do
>> the map. I verified current domain0/domU works on it.
>> But it still changes the control panel and hope that is acceptable.
>> 
>> Thanks
>> Yunhong Jiang
>> 
>> xen-devel-bounces@lists.xensource.com <> wrote:
>>> On 26/10/07 16:02, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
>>> 
>>>>> dom0 needs to be involved, since we can''t let domU
map any
arbitrary
>>>>> vector into its pirq space. Since dom0 has to be involved
in
access
>>>>> control to the irq vector space, can''t it do the
mapping?
>>>> 
>>>> yes, what I mean is, "before starting the domain"
works for IOAPIC
IRQ,
>>>> not MSI. MSI will still through communcation between PCI
>>>> frontend/backend directly.
>>> 
>>> Oh, I see. Then it probably has to be a phydevop and let dom0
kernel
do
>>> it. But there should be no reason to let domU use the map_irq
physdev_op
>>> at all. 
>>> 
>>> -- Keir
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xensource.com
>>> http://lists.xensource.com/xen-devel






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

The patches need a signed-off-by line.

 -- Keir

On 1/11/07 09:33, "Jiang, Yunhong" <yunhong.jiang@intel.com>
wrote:
> Keir, attached is the updated patchset.
> 
> A xen option "msi_irq_enable" is added. the pirq_domain method
will be
> enabled only when msi_irq_enable=1.
> 
> pirq_per_domain.patch is the changes for per domain pirq. When
> msi_irq_enable=0, it in fact still use old method.
> msi_irq_xen.patch changes xen for MSI  support. Currently we are using
> ACK_NEW method to avoid possible interrupt storm in some device.
> msi_passthrough.patch add MSI support to VT-d domain.
> msix-permission.patch is to disable MSI-x MMIO permission for domain U.
> 
> msi_kernel.patch add MSI/MSI-X support to domain0/domainU.
> msi_disable.patch changes the configuration file. current settting is to
> disable MSI by default.
> 
> -- Yunhong Jiang
> 
> Keir Fraser <mailto:Keir.Fraser@cl.cam.ac.uk> wrote:
>> Er, maybe. Does this slot in with some of the other patches
>> you previously
>> sent? Are we shooting to get this into 3.2.0 (scary!)?
>> 
>> -- Keir
>> 
>> On 30/10/07 14:27, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
>> 
>>> So, Keir, how about the attached method for the per-domain pirq?
>>> Now there is no need to change domain0 any more. Also domain U
can''t
> do
>>> the map. I verified current domain0/domU works on it.
>>> But it still changes the control panel and hope that is acceptable.
>>> 
>>> Thanks
>>> Yunhong Jiang
>>> 
>>> xen-devel-bounces@lists.xensource.com <> wrote:
>>>> On 26/10/07 16:02, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
>>>> 
>>>>>> dom0 needs to be involved, since we can''t let
domU map any
> arbitrary
>>>>>> vector into its pirq space. Since dom0 has to be
involved in
> access
>>>>>> control to the irq vector space, can''t it do
the mapping?
>>>>> 
>>>>> yes, what I mean is, "before starting the domain"
works for IOAPIC
> IRQ,
>>>>> not MSI. MSI will still through communcation between PCI
>>>>> frontend/backend directly.
>>>> 
>>>> Oh, I see. Then it probably has to be a phydevop and let dom0
kernel
> do
>>>> it. But there should be no reason to let domU use the map_irq
> physdev_op
>>>> at all. 
>>>> 
>>>> -- Keir
>>>> 
>>>> 
>>>> 
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xensource.com
>>>> http://lists.xensource.com/xen-devel


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

I update the description of the patch, and add signed-off-by line.
the sequence is :
For xen tree: pirq_per_domain.patch -- msi_irq_xen.patch --
msi_passthrough.patch -- msix-permission.patch
For kernel tree: msi_kernel.patch -- msi_disable.patch
Please review the patches.

Thanks
Yunhong Jiang

Keir Fraser <mailto:Keir.Fraser@cl.cam.ac.uk>
wrote:
> The patches need a signed-off-by line.
> 
> -- Keir
> 
> On 1/11/07 09:33, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
> 
>> Keir, attached is the updated patchset.
>> 
>> A xen option "msi_irq_enable" is added. the pirq_domain
method will
be
>> enabled only when msi_irq_enable=1.
>> 
>> pirq_per_domain.patch is the changes for per domain pirq. When
>> msi_irq_enable=0, it in fact still use old method.
>> msi_irq_xen.patch changes xen for MSI  support. Currently we are
using
>> ACK_NEW method to avoid possible interrupt storm in some device.
>> msi_passthrough.patch add MSI support to VT-d domain.
>> msix-permission.patch is to disable MSI-x MMIO permission for domain
U.
>> 
>> msi_kernel.patch add MSI/MSI-X support to domain0/domainU.
>> msi_disable.patch changes the configuration file. current settting is
to
>> disable MSI by default. 
>> 
>> -- Yunhong Jiang
>> 
>> Keir Fraser <mailto:Keir.Fraser@cl.cam.ac.uk> wrote:
>>> Er, maybe. Does this slot in with some of the other patches
>>> you previously
>>> sent? Are we shooting to get this into 3.2.0 (scary!)?
>>> 
>>> -- Keir
>>> 
>>> On 30/10/07 14:27, "Jiang, Yunhong"
<yunhong.jiang@intel.com> wrote:
>>> 
>>>> So, Keir, how about the attached method for the per-domain
pirq?
>>>> Now there is no need to change domain0 any more. Also domain U
can''t do
>>>> the map. I verified current domain0/domU works on it.
>>>> But it still changes the control panel and hope that is
acceptable.
>>>> 
>>>> Thanks
>>>> Yunhong Jiang
>>>> 
>>>> xen-devel-bounces@lists.xensource.com <> wrote:
>>>>> On 26/10/07 16:02, "Jiang, Yunhong"
> <yunhong.jiang@intel.com> wrote:
>>>>> 
>>>>>>> dom0 needs to be involved, since we can''t
let domU map any
arbitrary
>>>>>>> vector into its pirq space. Since dom0 has to be
involved in
access
>>>>>>> control to the irq vector space, can''t it
do the mapping?
>>>>>> 
>>>>>> yes, what I mean is, "before starting the
domain" works for
IOAPIC IRQ,
>>>>>> not MSI. MSI will still through communcation between
PCI
>>>>>> frontend/backend directly.
>>>>> 
>>>>> Oh, I see. Then it probably has to be a phydevop and let
dom0
kernel do
>>>>> it. But there should be no reason to let domU use the
map_irq
>>>>> physdev_op at all. 
>>>>> 
>>>>> -- Keir
>>>>> 
>>>>> 
>>>>> 
>>>>> _______________________________________________
>>>>> Xen-devel mailing list
>>>>> Xen-devel@lists.xensource.com
>>>>> http://lists.xensource.com/xen-devel






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