Re: [GIT PULL] Xen for 2.6.30 #2

[Ingo Molnar <mingo@elte.hu>]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Hi Linus,
>
> This is the second (and final) set of Xen updates for 2.6.30.  
> This set contains Xen dom0 support, which includes:
>
>    * adding to the existing Xen support to allow running as a
>      privileged domain
>    * some hooks into the apic setup code to allow Xen interrupts to get
>      routed properly
>    * a Xen implementation of the DMA ops API
>    * a Xen MTRR driver
>    * other misc things
>
> These changes only hook into init/setup code, and will have no 
> runtime performance impact on native execution when 
> CONFIG_PARAVIRT is enabled, and are completely compiled out 
> without it.
>
> These changes are more or less unchanged since their first posting 
> about 3-4 weeks ago.  The x86 parts have been reviewed (mainly by 
> HPA) and considered not too objectionable.

This stuff is still on my list of things to look at.

AFAIK hpa had a look and found it not too objectionable, but didnt 
ack it, which means the green lights to:

  - review it in detail
 1- then after a round of review feedbacks merge it into the x86 tree
  - then to test it there
  - then to fix the (inevitable) bugs and go to 1 until bug-free
  - then to stage it to linux-next
  - then after many weeks and months, to eventually send it to Linus

That's NOT the same thing as you sending it straight to Linus, 
without the broad acks from the x86 maintainers for all details.

I had a quick look, and stuff like this is not acceptable:

 static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
 {
-       struct io_apic __iomem *io_apic = io_apic_base(apic);
+       struct io_apic __iomem *io_apic;
+
+       if (xen_initial_domain())
+               return xen_io_apic_read(apic, reg);
+
+       io_apic = io_apic_base(apic);

Should be done by introducing your own xen specific irqchip. And 
this is not news to you, it has been told you in _early February_:

  http://lkml.indiana.edu/hypermail/linux/kernel/0902.1/00410.html

You didnt reply to that feedback of mine and you didnt fix it.

We are not putting some xen-specific hack into core x86 code ... The 
irqchip method wont put overhead and ugliness into native Linux. 
It's an existing abstraction for such stuff, use it and extend it if 
needed.

And stuff like this in arch/x86/kernel/pci-swiotlb.c:

  dma_addr_t swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t paddr)
  {
 +#ifdef CONFIG_PCI_XEN
 +       if (xen_pv_domain())
 +               return xen_phys_to_bus(paddr);
 +#endif
         return paddr;
  }

and the other PCI bits very much need the ack of the PCI and 
sw-IOMMU folks (Fujita Tomonori mainly). I'd be surprised if they 
werent disgusted by it.

I dont mind pull requests outside of maintenance boundaries, as long 
as the changes are good.

You know our stance which is very simple: dont put in Xen-only hooks 
that slow down native, and get rid of the existing Xen-only hooks.

	Ingo