When assign multiple devices to guest which uses PIC, ISA IRQ alias may occur. This patch splits ISA IRQ and GSI eoi function. In ISA IRQ eoi function, searches all assigned mirqs and does eoi for the corresponding mirqs which match the eoi ISA IRQ. Therefore fix ISA IRQ alias issue. Signed-off-by: Weidong Han <weidong.han@intel.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
A concerning amount of code churn and I''m not clear it works for EOI of PCI-ISA IRQs via the IO-APIC. This probably isn''t for 3.2.0. -- Keir On 15/11/07 06:32, "Han, Weidong" <weidong.han@intel.com> wrote:> When assign multiple devices to guest which uses PIC, ISA IRQ alias may > occur. This patch splits ISA IRQ and GSI eoi function. In ISA IRQ eoi > function, searches all assigned mirqs and does eoi for the corresponding > mirqs which match the eoi ISA IRQ. Therefore fix ISA IRQ alias issue. > > > Signed-off-by: Weidong Han <weidong.han@intel.com> > _______________________________________________ > 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
This patch doesn''t change EOI of PCI-ISA IRQs via IO-APIC, it just uses a separate function to handle EOI of PCI-ISA IRQs via the PIC. Without this patch, assigning multiple devices to guest which uses PIC maybe results in disabling the IRQ. -- Weidong Keir Fraser wrote:> A concerning amount of code churn and I''m not clear it works for EOI > of PCI-ISA IRQs via the IO-APIC. This probably isn''t for 3.2.0. > > -- Keir > > On 15/11/07 06:32, "Han, Weidong" <weidong.han@intel.com> wrote: > >> When assign multiple devices to guest which uses PIC, ISA IRQ alias >> may occur. This patch splits ISA IRQ and GSI eoi function. In ISA >> IRQ eoi function, searches all assigned mirqs and does eoi for the >> corresponding mirqs which match the eoi ISA IRQ. Therefore fix ISA >> IRQ alias issue. >> >> >> Signed-off-by: Weidong Han <weidong.han@intel.com> >> _______________________________________________ >> 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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
So what if a PCI-ISA IRQ is delivered via, and EOIed via, the IO-APIC? -- Keir On 17/11/07 01:37, "Han, Weidong" <weidong.han@intel.com> wrote:> This patch doesn''t change EOI of PCI-ISA IRQs via IO-APIC, it just uses > a separate function to handle EOI of PCI-ISA IRQs via the PIC. Without > this patch, assigning multiple devices to guest which uses PIC maybe > results in disabling the IRQ. > > -- Weidong > > Keir Fraser wrote: >> A concerning amount of code churn and I''m not clear it works for EOI >> of PCI-ISA IRQs via the IO-APIC. This probably isn''t for 3.2.0. >> >> -- Keir >> >> On 15/11/07 06:32, "Han, Weidong" <weidong.han@intel.com> wrote: >> >>> When assign multiple devices to guest which uses PIC, ISA IRQ alias >>> may occur. This patch splits ISA IRQ and GSI eoi function. In ISA >>> IRQ eoi function, searches all assigned mirqs and does eoi for the >>> corresponding mirqs which match the eoi ISA IRQ. Therefore fix ISA >>> IRQ alias issue. >>> >>> >>> Signed-off-by: Weidong Han <weidong.han@intel.com> >>> _______________________________________________ >>> 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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
It''s my misunderstanding. I should also add ISA IRQ handling for EOI via IO-APIC. -- Weidong Keir Fraser wrote:> So what if a PCI-ISA IRQ is delivered via, and EOIed via, the IO-APIC? > > -- Keir > > On 17/11/07 01:37, "Han, Weidong" <weidong.han@intel.com> wrote: > >> This patch doesn''t change EOI of PCI-ISA IRQs via IO-APIC, it just >> uses a separate function to handle EOI of PCI-ISA IRQs via the PIC. >> Without this patch, assigning multiple devices to guest which uses >> PIC maybe results in disabling the IRQ. >> >> -- Weidong >> >> Keir Fraser wrote: >>> A concerning amount of code churn and I''m not clear it works for EOI >>> of PCI-ISA IRQs via the IO-APIC. This probably isn''t for 3.2.0. >>> >>> -- Keir >>> >>> On 15/11/07 06:32, "Han, Weidong" <weidong.han@intel.com> wrote: >>> >>>> When assign multiple devices to guest which uses PIC, ISA IRQ alias >>>> may occur. This patch splits ISA IRQ and GSI eoi function. In ISA >>>> IRQ eoi function, searches all assigned mirqs and does eoi for the >>>> corresponding mirqs which match the eoi ISA IRQ. Therefore fix ISA >>>> IRQ alias issue. >>>> >>>> >>>> Signed-off-by: Weidong Han <weidong.han@intel.com> >>>> _______________________________________________ >>>> 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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir, attached patch adds ISA IRQ handling for EOI of ISA IRQs via IO-APIC, does it eliminate your concern? -- Weidong Keir Fraser wrote:> So what if a PCI-ISA IRQ is delivered via, and EOIed via, the IO-APIC? > > -- Keir > > On 17/11/07 01:37, "Han, Weidong" <weidong.han@intel.com> wrote: > >> This patch doesn''t change EOI of PCI-ISA IRQs via IO-APIC, it just >> uses a separate function to handle EOI of PCI-ISA IRQs via the PIC. >> Without this patch, assigning multiple devices to guest which uses >> PIC maybe results in disabling the IRQ. >> >> -- Weidong >> >> Keir Fraser wrote: >>> A concerning amount of code churn and I''m not clear it works for EOI >>> of PCI-ISA IRQs via the IO-APIC. This probably isn''t for 3.2.0. >>> >>> -- Keir >>> >>> On 15/11/07 06:32, "Han, Weidong" <weidong.han@intel.com> wrote: >>> >>>> When assign multiple devices to guest which uses PIC, ISA IRQ alias >>>> may occur. This patch splits ISA IRQ and GSI eoi function. In ISA >>>> IRQ eoi function, searches all assigned mirqs and does eoi for the >>>> corresponding mirqs which match the eoi ISA IRQ. Therefore fix ISA >>>> IRQ alias issue. >>>> >>>> >>>> Signed-off-by: Weidong Han <weidong.han@intel.com> >>>> _______________________________________________ >>>> 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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
hvm_set_pci_link_route dereferences hvm_irq->dpci without checking for NULL. hvm_dpci_eoi tests guest_gsi in isairq_mask without checking < NR_ISAIRQS. hvm_dpci_isairq_eoi can be a private vt-d function. All external callers can continue to go through hvm_dpci_eoi. link_mask doesn''t look like a very useful field. Get rid of it? isairq_mask isn''t a great name. It''s purpose is quite different from dirq_mask for example. Can we come up with a better name for either of these fields? -- Keir On 17/11/07 05:15, "Han, Weidong" <weidong.han@intel.com> wrote:> Keir, attached patch adds ISA IRQ handling for EOI of ISA IRQs via > IO-APIC, does it eliminate your concern? > > -- Weidong > > Keir Fraser wrote: >> So what if a PCI-ISA IRQ is delivered via, and EOIed via, the IO-APIC? >> >> -- Keir >> >> On 17/11/07 01:37, "Han, Weidong" <weidong.han@intel.com> wrote: >> >>> This patch doesn''t change EOI of PCI-ISA IRQs via IO-APIC, it just >>> uses a separate function to handle EOI of PCI-ISA IRQs via the PIC. >>> Without this patch, assigning multiple devices to guest which uses >>> PIC maybe results in disabling the IRQ. >>> >>> -- Weidong >>> >>> Keir Fraser wrote: >>>> A concerning amount of code churn and I''m not clear it works for EOI >>>> of PCI-ISA IRQs via the IO-APIC. This probably isn''t for 3.2.0. >>>> >>>> -- Keir >>>> >>>> On 15/11/07 06:32, "Han, Weidong" <weidong.han@intel.com> wrote: >>>> >>>>> When assign multiple devices to guest which uses PIC, ISA IRQ alias >>>>> may occur. This patch splits ISA IRQ and GSI eoi function. In ISA >>>>> IRQ eoi function, searches all assigned mirqs and does eoi for the >>>>> corresponding mirqs which match the eoi ISA IRQ. Therefore fix ISA >>>>> IRQ alias issue. >>>>> >>>>> >>>>> Signed-off-by: Weidong Han <weidong.han@intel.com> >>>>> _______________________________________________ >>>>> 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 >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir, I still use link_mask field, but rename it as link_map. I think it''s a easy way to record mapped Links, therefore record mapped ISA IRQs in hvm_set_pci_link_route(). Except this, I adopt all of your other suggestions. New patch is attached. Thanks! -- Weidong (Randy) Keir Fraser wrote:> hvm_set_pci_link_route dereferences hvm_irq->dpci without checking > for NULL. > > hvm_dpci_eoi tests guest_gsi in isairq_mask without checking < > NR_ISAIRQS. > > hvm_dpci_isairq_eoi can be a private vt-d function. All external > callers can continue to go through hvm_dpci_eoi. > > link_mask doesn''t look like a very useful field. Get rid of it? > > isairq_mask isn''t a great name. It''s purpose is quite different from > dirq_mask for example. Can we come up with a better name for either > of these fields? > > -- Keir > > On 17/11/07 05:15, "Han, Weidong" <weidong.han@intel.com> wrote: > >> Keir, attached patch adds ISA IRQ handling for EOI of ISA IRQs via >> IO-APIC, does it eliminate your concern? >> >> -- Weidong >> >> Keir Fraser wrote: >>> So what if a PCI-ISA IRQ is delivered via, and EOIed via, the >>> IO-APIC? >>> >>> -- Keir >>> >>> On 17/11/07 01:37, "Han, Weidong" <weidong.han@intel.com> wrote: >>> >>>> This patch doesn''t change EOI of PCI-ISA IRQs via IO-APIC, it just >>>> uses a separate function to handle EOI of PCI-ISA IRQs via the PIC. >>>> Without this patch, assigning multiple devices to guest which uses >>>> PIC maybe results in disabling the IRQ. >>>> >>>> -- Weidong >>>> >>>> Keir Fraser wrote: >>>>> A concerning amount of code churn and I''m not clear it works for >>>>> EOI of PCI-ISA IRQs via the IO-APIC. This probably isn''t for >>>>> 3.2.0. >>>>> >>>>> -- Keir >>>>> >>>>> On 15/11/07 06:32, "Han, Weidong" <weidong.han@intel.com> wrote: >>>>> >>>>>> When assign multiple devices to guest which uses PIC, ISA IRQ >>>>>> alias may occur. This patch splits ISA IRQ and GSI eoi function. >>>>>> In ISA IRQ eoi function, searches all assigned mirqs and does >>>>>> eoi for the corresponding mirqs which match the eoi ISA IRQ. >>>>>> Therefore fix ISA IRQ alias issue. >>>>>> >>>>>> >>>>>> Signed-off-by: Weidong Han <weidong.han@intel.com> >>>>>> _______________________________________________ >>>>>> 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 >> > > > > _______________________________________________ > 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 23/11/07 15:49, "Han, Weidong" <weidong.han@intel.com> wrote:> I still use link_mask field, but rename it as link_map. I think it''s a > easy way to record mapped Links, therefore record mapped ISA IRQs in > hvm_set_pci_link_route(). Except this, I adopt all of your other > suggestions. New patch is attached. Thanks!Applied as changeset 16436. I modified your changes to arch/x86/hvm/irq.c -- please check it''s still okay. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
I checked it. It is okay! -- Weidong Keir Fraser wrote:> On 23/11/07 15:49, "Han, Weidong" <weidong.han@intel.com> wrote: > >> I still use link_mask field, but rename it as link_map. I think it''s >> a easy way to record mapped Links, therefore record mapped ISA IRQs >> in hvm_set_pci_link_route(). Except this, I adopt all of your other >> suggestions. New patch is attached. Thanks! > > Applied as changeset 16436. I modified your changes to > arch/x86/hvm/irq.c -- please check it''s still okay. > > -- Keir_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel