xen: add more Xen dom0 support
Subsystems (12)
PATCH 1/1 xen: add more Xen dom0 support
Hi Ingo,
Here's the chunk of patches to add Xen Dom0 support (it's probably worth creating a new xen/dom0 topic branch for it).
A dom0 Xen domain is basically the same as a normal domU domain, but it has extra privileges to directly access hardware. There are two issues to deal with:
- translating to and from the domain's pseudo-physical addresses and real machine addresses (for ioremap and setting up DMA)
- routing hardware interrupts into the domain
ioremap is relatively easy to deal with. An earlier patch introduced the _PAGE_IOMAP pte flag, which we use to distinguish between a regular pseudo-physical mapping and a real machine mapping. Everything falls out pretty cleanly. A consequence is that the various pieces of table-parsing code - DMI, ACPI, MP, etc - work out of the box.
Similarly, the series adds hooks into swiotlb so that architectures can allocate the swiotlb memory appropriately; on the x86/xen side, Xen hooks these allocation functions to make special hypercalls to guarantee that the allocated memory is contiguous in machine memory.
Interrupts are a very different affair. The descriptions in each patch describe how it all fits together in detail, but the overview is:
1. Xen owns the local APICs; the dom0 kernel controls the IO APICs
2. Hardware interrupts are delivered on event channels like everything else
3. To set this up, we intercept at pcibios_enable_irq:
- given a dev+pin, we use ACPI to get a gsi
- hook acpi_register_gsi to call xen_register_gsi, which
- allocates an irq (generally not 1:1 with the gsi)
- asks Xen for a vector and event channel for the irq
- program the IO APIC to deliver the hardware interrupt to the allocated vector
The upshot is that the device driver gets an irq, and when the hardware raises an interrupt, it gets delivered on that irq.
We maintain our own irq allocation space, since the hardware-bound event channel irqs are intermixed with all the other normal Xen event channel irqs (inter-domain, timers, IPIs, etc). For compatibility the irqs 0-15 are reserved for legacy device interrupts, but the rest of the range is dynamically allocated.
Initialization also requires care. The dom0 kernel parses the ACPI tables as usual, in order to discover the local and IO APICs, and all the rest of the ACPI-provided data the kernel requires. However, because the kernel doesn't own the local APICs and can't directly map the IO APICs, we must be sure to avoid actually touching the hardware when running under Xen.
TODO: work out how to fit MSI into all this.
So, in summary, this series contains:
- dom0 console support
- dom0 xenbus support
- CPU features and IO access for a privleged domain
- mtrrs
- making ioremap work on machine addresses
- swiotlb allocation hooks
- interrupts
- introduce PV io_apic operations
- add Xen-specific IRQ allocator
- switch to using all-Xen event delivery
- add pirq Xen interrupt type
- table parsing and setup
- intercept driver interrupt registration
All this code will compile away to nothing when CONFIG_XEN_DOM0 is not enabled. If it is enabled, it will only have an effect if booted as a dom0 kernel; normal native execution and domU execution should be unaffected.
Thanks,
J
PATCH 1/1 xen: various whitespace and other formatting cleanups
From: Tej <[email protected]>
--- arch/x86/xen/enlighten.c | 18 ++++++++++-------- arch/x86/xen/mmu.c | 17 ++++++++++------- arch/x86/xen/multicalls.c | 2 +- arch/x86/xen/setup.c | 9 +++++---- 4 files changed, 26 insertions(+), 20 deletions(-)
arch/x86/xen/enlighten.c +9 -7
@@ -793,7 +793,7 @@ ret =0 ; -switch (msr) { +switch (msr) {#ifdef CONFIG_X86_64 unsigned which;u64 base;
@@ -1453,7 +1453,7 @@ ident_pte =0 ; pfn =0 ; -for (pmdidx =0 ; pmdidx <PTRS_PER_PMD && pfn < max_pfn; pmdidx++) { +for (pmdidx =0 ; pmdidx <PTRS_PER_PMD && pfn < max_pfn; pmdidx++) {pte_t *pte_page;/* Reuse or allocate a page of ptes */
@@ -1471,7 +1471,7 @@ }/* Install mappings */ -for (pteidx =0 ; pteidx <PTRS_PER_PTE ; pteidx++, pfn++) { +for (pteidx =0 ; pteidx <PTRS_PER_PTE ; pteidx++, pfn++) {pte_t pte;if (pfn > max_pfn_mapped)
@@ -1485,7 +1485,7 @@ } } -for (pteidx =0 ; pteidx < ident_pte; pteidx +=PTRS_PER_PTE ) +for (pteidx =0 ; pteidx < ident_pte; pteidx +=PTRS_PER_PTE )set_page_prot (&level1_ident_pgt[pteidx],PAGE_KERNEL_RO );set_page_prot (pmd,PAGE_KERNEL_RO );
@@ -1499,7 +1499,7 @@/* All levels are converted the same way, so just treat them as ptes. */ -for (i =0 ; i <PTRS_PER_PTE ; i++) +for (i =0 ; i <PTRS_PER_PTE ; i++) pte[i] =xen_make_pte (pte[i].pte); }
@@ -1514,7 +1514,8 @@* of the physical mapping once some sort of allocator has been set * up. */ -static __init pgd_t *xen_setup_kernel_pagetable (pgd_t *pgd,unsigned long max_pfn) +static __init pgd_t *xen_setup_kernel_pagetable (pgd_t *pgd, +unsigned long max_pfn) {pud_t *l3;pmd_t *l2;
@@ -1577,7 +1578,8 @@#else /* !CONFIG_X86_64 */ static pmd_t level2_kernel_pgt[PTRS_PER_PMD] __page_aligned_bss; -static __init pgd_t *xen_setup_kernel_pagetable (pgd_t *pgd,unsigned long max_pfn) +static __init pgd_t *xen_setup_kernel_pagetable (pgd_t *pgd, +unsigned long max_pfn) {pmd_t *kernel_pmd;
arch/x86/xen/mmu.c +10 -7
@@ -154,13 +154,13 @@ {unsigned pfn, idx; -for (pfn =0 ; pfn <MAX_DOMAIN_PAGES ; pfn +=P2M_ENTRIES_PER_PAGE ) { +for (pfn =0 ; pfn <MAX_DOMAIN_PAGES ; pfn +=P2M_ENTRIES_PER_PAGE ) {unsigned topidx =p2m_top_index (pfn); p2m_top_mfn[topidx] =virt_to_mfn (p2m_top[topidx]); } -for (idx =0 ; idx <ARRAY_SIZE (p2m_top_mfn_list); idx++) { +for (idx =0 ; idx <ARRAY_SIZE (p2m_top_mfn_list); idx++) {unsigned topidx = idx *P2M_ENTRIES_PER_PAGE ; p2m_top_mfn_list[idx] =virt_to_mfn (&p2m_top_mfn[topidx]); }
@@ -179,7 +179,7 @@unsigned long max_pfn =min (MAX_DOMAIN_PAGES , xen_start_info->nr_pages);unsigned pfn; -for (pfn =0 ; pfn < max_pfn; pfn +=P2M_ENTRIES_PER_PAGE ) { +for (pfn =0 ; pfn < max_pfn; pfn +=P2M_ENTRIES_PER_PAGE ) {unsigned topidx =p2m_top_index (pfn); p2m_top[topidx] = &mfn_list[pfn];
@@ -207,7 +207,7 @@ p = (void *)__get_free_page (GFP_KERNEL | __GFP_NOFAIL);BUG_ON (p ==NULL ); -for (i =0 ; i <P2M_ENTRIES_PER_PAGE ; i++) +for (i =0 ; i <P2M_ENTRIES_PER_PAGE ; i++) p[i] =INVALID_P2M_ENTRY ;if (cmpxchg (pp, p2m_missing, p) != p2m_missing)
@@ -407,7 +407,8 @@preempt_enable (); } -pte_t xen_ptep_modify_prot_start (struct mm_struct *mm,unsigned long addr,pte_t *ptep) +pte_t xen_ptep_modify_prot_start (struct mm_struct *mm, +unsigned long addr,pte_t *ptep) {/* Just return the pte as-is. We preserve the bits on commit */ return *ptep;
@@ -871,7 +872,8 @@if (user_pgd) {xen_pin_page (mm,virt_to_page (user_pgd),PT_PGD ); -xen_do_pin (MMUEXT_PIN_L4_TABLE ,PFN_DOWN (__pa (user_pgd))); +xen_do_pin (MMUEXT_PIN_L4_TABLE , +PFN_DOWN (__pa (user_pgd))); } }#else /* CONFIG_X86_32 */
@@ -986,7 +988,8 @@pgd_t *user_pgd =xen_get_user_pgd (pgd);if (user_pgd) { -xen_do_pin (MMUEXT_UNPIN_TABLE ,PFN_DOWN (__pa (user_pgd))); +xen_do_pin (MMUEXT_UNPIN_TABLE , +PFN_DOWN (__pa (user_pgd)));xen_unpin_page (mm,virt_to_page (user_pgd),PT_PGD ); } }
arch/x86/xen/multicalls.c +1 -1
@@ -154,7 +154,7 @@ ret, smp_processor_id());dump_stack ();for (i =0 ; i < b->mcidx; i++) { - printk(" call %2d/%d: op=%lu arg=[%lx] result=%ld\n", + printk(KERN_DEBUG " call %2d/%d: op=%lu arg=[%lx] result=%ld\n", i+1, b->mcidx, b->debug[i].op, b->debug[i].args[0],
arch/x86/xen/setup.c +5 -4
@@ -28,6 +28,9 @@/* These are code, but not functions. Defined in entry.S */ extern const char xen_hypervisor_callback[];extern const char xen_failsafe_callback[]; +extern void xen_sysenter_target (void ); +extern void xen_syscall_target (void ); +extern void xen_syscall32_target (void );/**
@@ -110,7 +113,6 @@void __cpuinit xen_enable_sysenter (void ) { -extern void xen_sysenter_target (void );int ret;unsigned sysenter_feature;
@@ -132,8 +134,6 @@ {#ifdef CONFIG_X86_64 int ret; -extern void xen_syscall_target (void ); -extern void xen_syscall32_target (void ); ret =register_callback (CALLBACKTYPE_syscall, xen_syscall_target);if (ret !=0 ) {
@@ -160,7 +160,8 @@HYPERVISOR_vm_assist (VMASST_CMD_enable, VMASST_TYPE_writable_pagetables);if (!xen_feature (XENFEAT_auto_translated_physmap)) -HYPERVISOR_vm_assist (VMASST_CMD_enable, VMASST_TYPE_pae_extended_cr3); +HYPERVISOR_vm_assist (VMASST_CMD_enable, + VMASST_TYPE_pae_extended_cr3);if (register_callback (CALLBACKTYPE_event, xen_hypervisor_callback) ||register_callback (CALLBACKTYPE_failsafe, xen_failsafe_callback))
PATCH 1/1 x86: remove unused iommu_nr_pages
In reply to Jeremy FitzhardingeThe last usage was removed by the patch set culminating in
commit e3c449f526cebb8d287241c7e82faafd9709668b
Author: Joerg Roedel <[email protected]> Date: Wed Oct 15 22:02:11 2008 -0700
x86, AMD IOMMU: convert driver to generic iommu_num_pages function
--- arch/x86/include/asm/iommu.h | 2 -- arch/x86/kernel/pci-dma.c | 7 ------- 2 files changed, 9 deletions(-)
arch/x86/include/asm/iommu.h +0 -2
@@ -8,8 +8,6 @@extern int iommu_detected;extern int dmar_disabled; -extern unsigned long iommu_nr_pages (unsigned long addr,unsigned long len); -/* 10 seconds */ #define DMAR_OPERATION_TIMEOUT ((cycles_t) tsc_khz*10*1000)
arch/x86/kernel/pci-dma.c +0 -7
@@ -125,13 +125,6 @@pci_swiotlb_init (); } -unsigned long iommu_nr_pages (unsigned long addr,unsigned long len) -{ -unsigned long size =roundup ((addr & ~PAGE_MASK ) + len,PAGE_SIZE ); - -return size >>PAGE_SHIFT ; -} -EXPORT_SYMBOL (iommu_nr_pages);#endif void *dma_generic_alloc_coherent(struct device *dev, size_t size,
PATCH 1/1 swiotlb: allow architectures to override swiotlb pool allocation
In reply to Jeremy FitzhardingeArchitectures may need to allocate memory specially for use with the swiotlb. Create the weak function swiotlb_alloc_boot() and swiotlb_alloc() defaulting to the current behaviour.
--- include/linux/swiotlb.h | 3 +++ lib/swiotlb.c | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-)
include/linux/swiotlb.h +3 -0
@@ -10,6 +10,9 @@extern void swiotlb_init (void ); +extern void *swiotlb_alloc_boot (size_t bytes,unsigned long nslabs); +extern void *swiotlb_alloc (unsigned order,unsigned long nslabs); +extern void *swiotlb_alloc_coherent (struct device *hwdev,size_t size,dma_addr_t *dma_handle,gfp_t flags);
lib/swiotlb.c +13 -3
@@ -21,6 +21,7 @@#include <linux/mm.h> #include <linux/module.h> #include <linux/spinlock.h> +#include <linux/swiotlb.h> #include <linux/string.h> #include <linux/types.h> #include <linux/ctype.h>
@@ -126,6 +127,16 @@__setup ("swiotlb=" , setup_io_tlb_npages);/* make io_tlb_overflow tunable too? */ +void *__weak swiotlb_alloc_boot (size_t size,unsigned long nslabs) +{ +return alloc_bootmem_low_pages (size); +} + +void *__weak swiotlb_alloc (unsigned order,unsigned long nslabs) +{ +return (void *)__get_free_pages (GFP_DMA | __GFP_NOWARN, order); +} +/* * Statically reserve bounce buffer space and initialize bounce buffer data * structures for the software IO TLB used to implement the DMA API.
@@ -145,7 +156,7 @@/* * Get IO TLB memory from the low pages */ - io_tlb_start =alloc_bootmem_low_pages (bytes); + io_tlb_start =swiotlb_alloc_boot (bytes, io_tlb_nslabs);if (!io_tlb_start)panic ("Cannot allocate SWIOTLB buffer" ); io_tlb_end = io_tlb_start + bytes;
@@ -202,8 +213,7 @@ bytes = io_tlb_nslabs <<IO_TLB_SHIFT ;while ((SLABS_PER_PAGE << order) >IO_TLB_MIN_SLABS ) { - io_tlb_start = (char *)__get_free_pages (GFP_DMA | __GFP_NOWARN, - order); + io_tlb_start =swiotlb_alloc (order, io_tlb_nslabs);if (io_tlb_start)break ; order--;
Re: [Xen-devel] [PATCH 03 of 38] swiotlb: allow architectures tooverride swiotlb pool allocation
Not directly related to this patch alone, but to the combined set of changes to swiotlb: I don't see any handling of CONFIG_HIGHMEM here (or at least a note that this a known limitation needing work). I mention this because this was the largest part of the changes I had posted long ago to make lib/swiotlb.c Xen-ready, and which got rejected due to their ugliness.
While perhaps less intrusive to take care of, I also didn't see an equivalent of the range_straddles_page_boundary() logic, without which I can't see how this would work in the common case.
Jan
Quoted 1 line
Quoted 1 line
Quoted 1 line
Jeremy Fitzhardinge <[email protected]> 13.11.08 20:10 >>>
Architectures may need to allocate memory specially for use with the swiotlb. Create the weak function swiotlb_alloc_boot() and swiotlb_alloc() defaulting to the current behaviour.
--- include/linux/swiotlb.h | 3 +++ lib/swiotlb.c | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-)
include/linux/swiotlb.h +3 -0
@@ -10,6 +10,9 @@extern void swiotlb_init (void ); +extern void *swiotlb_alloc_boot (size_t bytes,unsigned long nslabs); +extern void *swiotlb_alloc (unsigned order,unsigned long nslabs); +extern void *swiotlb_alloc_coherent (struct device *hwdev,size_t size,dma_addr_t *dma_handle,gfp_t flags);
lib/swiotlb.c +13 -3
@@ -21,6 +21,7 @@#include <linux/mm.h> #include <linux/module.h> #include <linux/spinlock.h> +#include <linux/swiotlb.h> #include <linux/string.h> #include <linux/types.h> #include <linux/ctype.h>
@@ -126,6 +127,16 @@__setup ("swiotlb=" , setup_io_tlb_npages);/* make io_tlb_overflow tunable too? */ +void *__weak swiotlb_alloc_boot (size_t size,unsigned long nslabs) +{ +return alloc_bootmem_low_pages (size); +} + +void *__weak swiotlb_alloc (unsigned order,unsigned long nslabs) +{ +return (void *)__get_free_pages (GFP_DMA | __GFP_NOWARN, order); +} +/* * Statically reserve bounce buffer space and initialize bounce buffer data * structures for the software IO TLB used to implement the DMA API.
@@ -145,7 +156,7 @@/* * Get IO TLB memory from the low pages */ - io_tlb_start =alloc_bootmem_low_pages (bytes); + io_tlb_start =swiotlb_alloc_boot (bytes, io_tlb_nslabs);if (!io_tlb_start)panic ("Cannot allocate SWIOTLB buffer" ); io_tlb_end = io_tlb_start + bytes;
@@ -202,8 +213,7 @@ bytes = io_tlb_nslabs <<IO_TLB_SHIFT ;while ((SLABS_PER_PAGE << order) >IO_TLB_MIN_SLABS ) { - io_tlb_start = (char *)__get_free_pages (GFP_DMA | __GFP_NOWARN, - order); + io_tlb_start =swiotlb_alloc (order, io_tlb_nslabs);if (io_tlb_start)break ; order--;
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Re: [Xen-devel] [PATCH 03 of 38] swiotlb: allow architectures tooverride swiotlb pool allocation
Quoted 6 lines · Jan Beulich
Not directly related to this patch alone, but to the combined set of changes to swiotlb: I don't see any handling of
CONFIG_HIGHMEMhere (or at least a note that this a known limitation needing work). I mention this because this was the largest part of the changes I had posted long ago to makelib/swiotlb.cXen-ready, and which got rejected due to their ugliness.
Was that Andi's objection on the grounds that he didn't think that Xen should need swiotlb at all?
I have to admit I didn't follow that thread very closely (or threads, as I seem to remember). Do you have a pointer to the pertinent bits?
Quoted 4 lines
While perhaps less intrusive to take care of, I also didn't see an equivalent of the
range_straddles_page_boundary()logic, without which I can't see how this would work in the common case.
Could you be more specific? The swiotlb allocation should be machine contiguous and so there's no stradding required, but I think I'm missing your point.
J
Re: [Xen-devel] [PATCH 03 of 38] swiotlb: allow architectures tooverride swiotlb pool allocation
Quoted 7 lines · Jeremy Fitzhardinge
Quoted 4 lines
While perhaps less intrusive to take care of, I also didn't see an equivalent of the
range_straddles_page_boundary()logic, without which I can't see how this would work in the common case.Could you be more specific? The swiotlb allocation should be machine contiguous and so there's no stradding required, but I think I'm missing your point.
In general, I think you are right; swiotlb should be machine contiguous, so it works in the normal case. The range_straddles_page_boundary function takes care of a corner case, where you can run into swiotlb exhaustion when you really shouldn't. As I understand it, it comes about because it is possible to get a swiotlb request with two pages that just happen to be machine contiguous, but were *not* allocated through xen_create_contiguous_region (and hence weren't marked in the contiguous_bitmap as such). In this case, you split the request into two separate requests, and this can more easily lead to exhaustion. range_straddles_page_boundary works around this by checking whether any two pages coming through the swiotlb layer are machine contiguous, and if they are, not splitting the request.
Signature · 1 line
Chris Lalancette
Re: [Xen-devel] [PATCH 03 of 38] swiotlb: allow architectures tooverride swiotlb pool allocation
Quoted 4 lines · Chris Lalancette
Quoted 3 lines
Could you be more specific? The swiotlb allocation should be machine contiguous and so there's no stradding required, but I think I'm missing your point.
Quoted 13 lines
In general, I think you are right; swiotlb should be machine contiguous, so it works in the normal case. The
range_straddles_page_boundaryfunction takes care
of a corner case, where you can run into swiotlb exhaustion when you really shouldn't. As I understand it, it comes about because it is possible to get a swiotlb request with two pages that just happen to be machine contiguous, but were *not* allocated throughxen_create_contiguous_region(and hence weren't marked in thecontiguous_bitmapas such). In this case, you split the request into two separate requests, and this can more easily lead to exhaustion.range_straddles_page_boundaryworks around this by checking whether any two pages coming through the swiotlb layer are machine contiguous, and if they are,
not splitting the request.
A more specific problem solved by range_straddle_page_boundary() in our 2.6.18 kernel was that the block layer would do bio merging because it checked that pages really were contiguous, and then swiotlb (without r_s_p_b) would decide that the pages weren't contiguous (because the contiguity was random luck rather than explicitly requested) and hence do bounce buffering. Result was that sufficiently aggressive I/O would exhaust swiotlb resources and crash the kernel.
In the 2.6.18 port we actually got rid of contiguous_bitmap[] entirely.
-- Keir
Re: [Xen-devel] [PATCH 03 of 38] swiotlb: allow architectures tooverrideswiotlb pool allocation
In reply to Jeremy FitzhardingeQuoted 11 lines
Quoted 1 line
Quoted 1 line
Jeremy Fitzhardinge <[email protected]> 14.11.08 20:33 >>>
Quoted 6 lines · Jan Beulich
Not directly related to this patch alone, but to the combined set of changes to swiotlb: I don't see any handling of
CONFIG_HIGHMEMhere (or at least a note that this a known limitation needing work). I mention this because this was the largest part of the changes I had posted long ago to makelib/swiotlb.cXen-ready, and which got rejected due to their ugliness.Was that Andi's objection on the grounds that he didn't think that Xen should need swiotlb at all?
No, Tony Luck actually merged it, but someone else (I don't recall who it was) requested it to be reverted again.
Quoted 2 lines
I have to admit I didn't follow that thread very closely (or threads, as I seem to remember). Do you have a pointer to the pertinent bits?
Quoted 7 lines
Quoted 4 lines
While perhaps less intrusive to take care of, I also didn't see an equivalent of the
range_straddles_page_boundary()logic, without which I can't see how this would work in the common case.Could you be more specific? The swiotlb allocation should be machine contiguous and so there's no stradding required, but I think I'm missing your point.
The question is whether a multi-page piece of memory must be funneled through the swiotlb in the first place. In native code, checking whether the first/last byte satisfies the address_needs_mapping() check is sufficient, but in Xen you also need to check whether the known to be physically contiguous pages are also machine-contiguous.
Jan
PATCH 1/1 swiotlb: move some definitions to header
In reply to Jeremy FitzhardingeFrom: Ian Campbell <[email protected]>
--- include/linux/swiotlb.h | 14 ++++++++++++++ lib/swiotlb.c | 14 +------------- 2 files changed, 15 insertions(+), 13 deletions(-)
include/linux/swiotlb.h +14 -0
@@ -7,6 +7,20 @@struct dma_attrs ;struct scatterlist ; +/* +* Maximum allowable number of contiguous slabs to map, +* must be a power of 2. What is the appropriate value ? +* The complexity of {map,unmap}_single is linearly dependent on this value. +*/ +#define IO_TLB_SEGSIZE 128 + + +/* +* log of the size of each IO TLB slab. The number of slabs is command line +* controllable. +*/ +#define IO_TLB_SHIFT 11 +extern void swiotlb_init (void );
lib/swiotlb.c +1 -13
@@ -23,6 +23,7 @@#include <linux/spinlock.h> #include <linux/swiotlb.h> #include <linux/string.h> +#include <linux/swiotlb.h> #include <linux/types.h> #include <linux/ctype.h>
@@ -40,19 +41,6 @@#define SG_ENT_VIRT_ADDRESS (sg) (sg_virt((sg)))#define SG_ENT_PHYS_ADDRESS (sg) virt_to_bus(SG_ENT_VIRT_ADDRESS(sg)) -/* -* Maximum allowable number of contiguous slabs to map, -* must be a power of 2. What is the appropriate value ? -* The complexity of {map,unmap}_single is linearly dependent on this value. -*/ -#define IO_TLB_SEGSIZE 128 - -/* -* log of the size of each IO TLB slab. The number of slabs is command line -* controllable. -*/ -#define IO_TLB_SHIFT 11 -#define SLABS_PER_PAGE (1 << (PAGE_SHIFT - IO_TLB_SHIFT))/*
Re: [PATCH 04 of 38] swiotlb: move some definitions to header
Quoted 20 lines · On Thu, 13 Nov 2008 11:10:02 -0800 Jeremy Fitzhardinge <[email protected]> wrote:
From: Ian Campbell <[email protected]>
Signed-off-by Ian Campbell <[email protected]>Signed-off-by Jeremy Fitzhardinge <[email protected]>--- include/linux/swiotlb.h | 14 ++++++++++++++ lib/swiotlb.c | 14 +------------- 2 files changed, 15 insertions(+), 13 deletions(-)include/linux/swiotlb.h +4
-0@@ -7,6 +7,20 @@struct dma_attrs ;struct scatterlist ; +/* +* Maximum allowable number of contiguous slabs to map, +* must be a power of 2. What is the appropriate value ? +* The complexity of {map,unmap}_single is linearly dependent on this value.
include/linux/swiotlb.h line 23
Quoted 10 lines
include/linux/swiotlb.h +10
-0+*/ +#define IO_TLB_SEGSIZE 128 + + +/* +* log of the size of each IO TLB slab. The number of slabs is command line +* controllable. +*/ +#define IO_TLB_SHIFT 11 +
Why do we need to export IO_TLB_SEGSIZE and IO_TLB_SHIFT to everyone in include/linux?
Re: [PATCH 04 of 38] swiotlb: move some definitions to header
Quoted 2 lines · FUJITA Tomonori
Why do we need to export
IO_TLB_SEGSIZEandIO_TLB_SHIFTto everyone ininclude/linux?
A subsequent Xen patch needs to make use of them, although I can't see it in the patchset Jeremy posted so here it is (not fully baked yet)
Subject: xen swiotlb: fixup swiotlb is chunks smaller than MAX_CONTIG_ORDER
From: Ian Campbell <[email protected]>
--- arch/x86/kernel/pci-swiotlb_64.c | 7 +------ drivers/pci/xen-iommu.c | 30 ++++++++++++++++++++---------- 2 files changed, 21 insertions(+), 16 deletions(-)
===================================================================
arch/x86/kernel/pci-swiotlb_64.c +1 -6
@@ -28,12 +28,7 @@void *swiotlb_alloc (unsigned order,unsigned long nslabs) { -void *ret = (void *)__get_free_pages (GFP_DMA | __GFP_NOWARN, order); - -if (ret &&xen_pv_domain ()) -xen_swiotlb_fixup (ret,1u << order, nslabs); - -return ret; +BUG (); } static dma_addr_t
===================================================================
drivers/pci/xen-iommu.c +20 -10
@@ -6,6 +6,7 @@#include <linux/version.h> #include <linux/scatterlist.h> #include <linux/bio.h> +#include <linux/swiotlb.h> #include <linux/io.h> #include <linux/bug.h>
@@ -43,19 +44,27 @@unsigned long *bitmap; }; +static int max_dma_bits =32 ; +void xen_swiotlb_fixup (void *buf,size_t size,unsigned long nslabs) { -unsigned order =get_order (size); +int i, rc; +int dma_bits; -printk (KERN_DEBUG "xen_swiotlb_fixup: buf=%p size=%zu order=%u\n" , - buf, size, order); +printk (KERN_DEBUG "xen_swiotlb_fixup: buf=%p size=%zu\n" , + buf, size); -if (WARN_ON (size != (PAGE_SIZE << order))) -return ; - -if (xen_create_contiguous_region ((unsigned long )buf, - order,0xffffffff )) -printk (KERN_ERR "xen_create_contiguous_region failed\n" ); + dma_bits =get_order (IO_TLB_SEGSIZE <<IO_TLB_SHIFT ) +PAGE_SHIFT ; +for (i =0 ; i < nslabs; i +=IO_TLB_SEGSIZE ) { +do { + rc =xen_create_contiguous_region ( + (unsigned long )buf + (i <<IO_TLB_SHIFT ), +get_order (IO_TLB_SEGSIZE <<IO_TLB_SHIFT ), + dma_bits); + }while (rc && dma_bits++ < max_dma_bits); +if (rc) +panic (KERN_ERR "xen_create_contiguous_region failed\n" ); + } } static inline int address_needs_mapping(struct device *hwdev,
@@ -117,7 +126,8 @@if (check_pages_physically_contiguous (pfn, offset, size))return 0 ; -printk ("range_straddles_page_boundary: p=%Lx size=%d pfn=%lx\n" , +printk (KERN_WARNING "range_straddles_page_boundary: " +"p=%Lx size=%zd pfn=%lx\n" , p, size, pfn);return 1 ; }
PATCH 1/1 swiotlb: allow architectures to override virt<->bus<->virt conversions
In reply to Jeremy FitzhardingeFrom: Ian Campbell <[email protected]>
Architectures may need to override these conversions. Implement a __weak hook point containing the default implementation.
--- include/linux/swiotlb.h | 3 +++ lib/swiotlb.c | 42 ++++++++++++++++++++++++++---------------- 2 files changed, 29 insertions(+), 16 deletions(-)
include/linux/swiotlb.h +3 -0
@@ -27,6 +27,9 @@extern void *swiotlb_alloc_boot (size_t bytes,unsigned long nslabs);extern void *swiotlb_alloc (unsigned order,unsigned long nslabs); +extern unsigned long __weak swiotlb_virt_to_bus (volatile void *address); +extern void *__weak swiotlb_bus_to_virt (unsigned long address); +extern void *swiotlb_alloc_coherent (struct device *hwdev,size_t size,dma_addr_t *dma_handle,gfp_t flags);
lib/swiotlb.c +26 -16
144 lines not on this page · raw message
PATCH 1/1 xen: clean up asm-x86/xen/hypervisor.h
In reply to Jeremy Fitzhardingehypervisor.h had accumulated a lot of crud, including lots of spurious #includes. Clean it all up, and go around fixing up everything else accordingly.
--- arch/x86/include/asm/xen/hypercall.h | 6 +++++ arch/x86/include/asm/xen/hypervisor.h | 39 ++++++--------------------------- arch/x86/include/asm/xen/page.h | 5 ++++ arch/x86/xen/enlighten.c | 1 drivers/xen/balloon.c | 4 ++- drivers/xen/features.c | 6 ++++- drivers/xen/grant-table.c | 1 include/xen/interface/event_channel.h | 2 + 8 files changed, 31 insertions(+), 33 deletions(-)
arch/x86/include/asm/xen/hypercall.h +6 -0
@@ -33,8 +33,14 @@#ifndef _ASM_X86_XEN_HYPERCALL_H#define _ASM_X86_XEN_HYPERCALL_H +#include <linux/kernel.h> +#include <linux/spinlock.h> #include <linux/errno.h> #include <linux/string.h> +#include <linux/types.h> + +#include <asm/page.h> +#include <asm/pgtable.h> #include <xen/interface/xen.h> #include <xen/interface/sched.h>
arch/x86/include/asm/xen/hypervisor.h +8 -31
@@ -33,39 +33,10 @@#ifndef _ASM_X86_XEN_HYPERVISOR_H#define _ASM_X86_XEN_HYPERVISOR_H -#include <linux/types.h> -#include <linux/kernel.h> - -#include <xen/interface/xen.h> -#include <xen/interface/version.h> - -#include <asm/ptrace.h> -#include <asm/page.h> -#include <asm/desc.h> -#if defined(__i386__) -# ifdef CONFIG_X86_PAE -# include <asm-generic/pgtable-nopud.h> -# else -# include <asm-generic/pgtable-nopmd.h> -# endif -#endif -#include <asm/xen/hypercall.h> -/* arch/i386/kernel/setup.c */ extern struct shared_info *HYPERVISOR_shared_info;extern struct start_info *xen_start_info; -/* arch/i386/mach-xen/evtchn.c */ -/* Force a proper event-channel callback from Xen. */ -extern void force_evtchn_callback (void ); - -/* Turn jiffies into Xen system time. */ -u64 jiffies_to_st (unsigned long jiffies); - - -#define MULTI_UVMFLAGS_INDEX 3 -#define MULTI_UVMDOMID_INDEX 4 -enum xen_domain_type {XEN_NATIVE , XEN_PV_DOMAIN,
@@ -74,9 +45,15 @@extern enum xen_domain_type xen_domain_type; +#ifdef CONFIG_XEN #define xen_domain () (xen_domain_type != XEN_NATIVE) -#define xen_pv_domain () (xen_domain_type == XEN_PV_DOMAIN) +#else +#define xen_domain () (0) +#endif + +#define xen_pv_domain () (xen_domain() && xen_domain_type == XEN_PV_DOMAIN) +#define xen_hvm_domain () (xen_domain() && xen_domain_type == XEN_HVM_DOMAIN) +#define xen_initial_domain () (xen_pv_domain() && xen_start_info->flags & SIF_INITDOMAIN) -#define xen_hvm_domain () (xen_domain_type == XEN_HVM_DOMAIN)#endif /* _ASM_X86_XEN_HYPERVISOR_H */
arch/x86/include/asm/xen/page.h +5 -0
@@ -1,11 +1,16 @@#ifndef _ASM_X86_XEN_PAGE_H#define _ASM_X86_XEN_PAGE_H +#include <linux/kernel.h> +#include <linux/types.h> +#include <linux/spinlock.h> #include <linux/pfn.h> #include <asm/uaccess.h> +#include <asm/page.h> #include <asm/pgtable.h> +#include <xen/interface/xen.h> #include <xen/features.h> /* Xen machine address */
arch/x86/xen/enlighten.c +1 -0
@@ -28,6 +28,7 @@#include <linux/console.h> #include <xen/interface/xen.h> +#include <xen/interface/version.h> #include <xen/interface/physdev.h> #include <xen/interface/vcpu.h> #include <xen/features.h>
drivers/xen/balloon.c +3 -1
@@ -44,13 +44,15 @@#include <linux/list.h> #include <linux/sysdev.h> -#include <asm/xen/hypervisor.h> #include <asm/page.h> #include <asm/pgalloc.h> #include <asm/pgtable.h> #include <asm/uaccess.h> #include <asm/tlb.h> +#include <asm/xen/hypervisor.h> +#include <asm/xen/hypercall.h> +#include <xen/interface/xen.h> #include <xen/interface/memory.h> #include <xen/xenbus.h> #include <xen/features.h>
drivers/xen/features.c +5 -1
@@ -8,7 +8,11 @@#include <linux/types.h> #include <linux/cache.h> #include <linux/module.h> -#include <asm/xen/hypervisor.h> + +#include <asm/xen/hypercall.h> + +#include <xen/interface/xen.h> +#include <xen/interface/version.h> #include <xen/features.h> u8 xen_features[XENFEAT_NR_SUBMAPS *32 ]__read_mostly ;
drivers/xen/grant-table.c +1 -0
@@ -40,6 +40,7 @@#include <xen/interface/xen.h> #include <xen/page.h> #include <xen/grant_table.h> +#include <asm/xen/hypercall.h> #include <asm/pgtable.h> #include <asm/sync_bitops.h>
include/xen/interface/event_channel.h +2 -0
@@ -9,6 +9,8 @@#ifndef __XEN_PUBLIC_EVENT_CHANNEL_H__#define __XEN_PUBLIC_EVENT_CHANNEL_H__ +#include <xen/interface/xen.h> +typedef uint32_t evtchn_port_t ;DEFINE_GUEST_HANDLE (evtchn_port_t);
PATCH 1/1 x86: add io_apic_ops
In reply to Jeremy FitzhardingeXen dom0 needs to paravirtualize IO operations to the IO APIC, so add a io_apic_ops for it to intercept. Do this as ops structure because there's at least some chance that another paravirtualized environment may want to intercept these.
--- arch/x86/include/asm/io_apic.h | 9 +++++++ arch/x86/kernel/io_apic.c | 50 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 3 deletions(-)
arch/x86/include/asm/io_apic.h +9 -0
@@ -21,6 +21,15 @@#define IO_APIC_REDIR_LEVEL_TRIGGER (1 << 15)#define IO_APIC_REDIR_MASKED (1 << 16) +struct io_apic_ops { +void (*init)(void ); +unsigned int (*read)(unsigned int apic,unsigned int reg); +void (*write)(unsigned int apic,unsigned int reg,unsigned int value); +void (*modify)(unsigned int apic,unsigned int reg,unsigned int value); +}; + +void __init set_io_apic_ops (const struct io_apic_ops *); +/* * The structure of the IO-APIC: */
arch/x86/kernel/io_apic.c +47 -3
86 lines not on this page · raw message
PATCH 1/1 x86: make apic_* operations inline functions
In reply to Jeremy FitzhardingeMainly to get proper type-checking and consistency.
--- arch/x86/include/asm/apic.h | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-)
arch/x86/include/asm/apic.h +29 -6
@@ -125,12 +125,35 @@extern struct apic_ops *apic_ops; -#define apic_read (apic_ops->read) -#define apic_write (apic_ops->write) -#define apic_icr_read (apic_ops->icr_read) -#define apic_icr_write (apic_ops->icr_write) -#define apic_wait_icr_idle (apic_ops->wait_icr_idle) -#define safe_apic_wait_icr_idle (apic_ops->safe_wait_icr_idle) +static inline u32 apic_read (u32 reg) +{ +return apic_ops->read (reg); +} + +static inline void apic_write (u32 reg,u32 val) +{ + apic_ops->write (reg, val); +} + +static inline u64 apic_icr_read (void ) +{ +return apic_ops->icr_read (); +} + +static inline void apic_icr_write (u32 low,u32 high) +{ + apic_ops->icr_write (low, high); +} + +static inline void apic_wait_icr_idle (void ) +{ + apic_ops->wait_icr_idle (); +} + +static inline u32 safe_apic_wait_icr_idle (void ) +{ +return apic_ops->safe_wait_icr_idle (); +}extern int get_physical_broadcast (void );
PATCH 1/1 x86: make sure we really have an hpet mapping before using it
In reply to Jeremy FitzhardingeWhen booting in Xen dom0, the hpet isn't really accessible, so make sure the mapping is non-NULL before use.
--- arch/x86/kernel/hpet.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/x86/kernel/hpet.c +3 -2
@@ -834,10 +834,11 @@ hpet_address = force_hpet_address;hpet_enable (); -if (!hpet_virt_address) -return -ENODEV ; } +if (!hpet_virt_address) +return -ENODEV ; +hpet_reserve_platform_timers (hpet_readl (HPET_ID ));for_each_online_cpu (cpu) {
PATCH 1/1 x86: add handle_irq() to allow interrupt injection
In reply to Jeremy FitzhardingeXen uses a different interrupt path, so introduce handle_irq() to allow interrupts to be inserted into the normal interrupt path. This is handled slightly differently on 32 and 64-bit.
--- arch/x86/include/asm/irq.h | 1 + arch/x86/kernel/irq_32.c | 34 +++++++++++++++++++++------------- arch/x86/kernel/irq_64.c | 27 ++++++++++++++++++--------- 3 files changed, 40 insertions(+), 22 deletions(-)
arch/x86/include/asm/irq.h +1 -0
@@ -39,6 +39,7 @@extern unsigned int do_IRQ (struct pt_regs *regs);extern void init_IRQ (void );extern void native_init_IRQ (void ); +extern bool handle_irq (unsigned irq,struct pt_regs *regs);/* Interrupt vector management */ extern DECLARE_BITMAP(used_vectors, NR_VECTORS);
arch/x86/kernel/irq_32.c +21 -13
@@ -191,6 +191,26 @@ execute_on_irq_stack(int overflow, struct irq_desc *desc, int irq) { return 0; }#endif +bool handle_irq (unsigned irq,struct pt_regs *regs) +{ +struct irq_desc *desc; +int overflow; + + overflow =check_stack_overflow (); + + desc =irq_to_desc (irq); +if (unlikely (!desc)) +return false; + +if (!execute_on_irq_stack (overflow, desc, irq)) { +if (unlikely (overflow)) +print_stack_overflow (); + desc->handle_irq (irq, desc); + } + +return true; +} +/* * do_IRQ handles all normal device IRQ's (the special * SMP cross-CPU interrupts have their own specific
@@ -200,31 +220,19 @@ {struct pt_regs *old_regs;/* high bit used in ret_from_ code */ -int overflow;unsigned vector = ~regs->orig_ax; -struct irq_desc *desc;unsigned irq; - old_regs =set_irq_regs (regs);irq_enter (); irq =__get_cpu_var (vector_irq)[vector]; - overflow =check_stack_overflow (); - - desc =irq_to_desc (irq); -if (unlikely (!desc)) { +if (!handle_irq (irq, regs)) {printk (KERN_EMERG "%s: cannot handle IRQ %d vector %#x cpu %d\n" , __func__, irq, vector,smp_processor_id ());BUG (); } -if (!execute_on_irq_stack (overflow, desc, irq)) { -if (unlikely (overflow)) -print_stack_overflow (); - desc->handle_irq (irq, desc); - } -irq_exit ();set_irq_regs (old_regs);return 1 ;
arch/x86/kernel/irq_64.c +18 -9
@@ -42,6 +42,22 @@ }#endif +bool handle_irq (unsigned irq,struct pt_regs *regs) +{ +struct irq_desc *desc; + +#ifdef CONFIG_DEBUG_STACKOVERFLOW +stack_overflow_check (regs); +#endif + + desc =irq_to_desc (irq); +if (unlikely (!desc)) +return false; + +generic_handle_irq_desc (irq, desc); +return true; +} +/* * do_IRQ handles all normal device IRQ's (the special * SMP cross-CPU interrupts have their own specific
@@ -50,7 +66,6 @@ asmlinkage unsigned int do_IRQ(struct pt_regs *regs) {struct pt_regs *old_regs =set_irq_regs (regs); -struct irq_desc *desc;/* high bit used in ret_from_ code */ unsigned vector = ~regs->orig_ax;
@@ -58,16 +73,10 @@exit_idle ();irq_enter (); + irq =__get_cpu_var (vector_irq)[vector]; -#ifdef CONFIG_DEBUG_STACKOVERFLOW -stack_overflow_check (regs); -#endif - - desc =irq_to_desc (irq); -if (likely (desc)) -generic_handle_irq_desc (irq, desc); -else { +if (!handle_irq (irq, regs)) {if (!disable_apic)ack_APIC_irq ();
PATCH 1/1 x86: define no-op exit_idle() on 32-bit
In reply to Jeremy Fitzhardingeexit_idle() is only used on 64-bit, but define a noop version on 32-bit to allow subsequent unification.
--- arch/x86/include/asm/idle.h | 10 ++++++++++ arch/x86/kernel/apic.c | 7 +------ arch/x86/kernel/io_apic.c | 2 -- 3 files changed, 11 insertions(+), 8 deletions(-)
arch/x86/include/asm/idle.h +10 -0
@@ -8,8 +8,18 @@void idle_notifier_register (struct notifier_block *n);void idle_notifier_unregister (struct notifier_block *n); +#ifdef CONFIG_X86_64 void enter_idle (void );void exit_idle (void ); +#else +static inline void enter_idle (void ) +{ +} + +static inline void exit_idle (void ) +{ +} +#endif /* CONFIG_X86_64 */ void c1e_remove_cpu (int cpu);
arch/x86/kernel/apic.c +1 -6
@@ -808,9 +808,8 @@* Besides, if we don't timer interrupts ignore the global * interrupt lock, which is the WrongThing (tm) to do. */ -#ifdef CONFIG_X86_64 exit_idle (); -#endif +irq_enter ();local_apic_timer_interrupt ();irq_exit ();
@@ -1668,9 +1667,7 @@ {u32 v; -#ifdef CONFIG_X86_64 exit_idle (); -#endif irq_enter ();/* * Check if this really is a spurious interrupt and ACK it
@@ -1699,9 +1696,7 @@ {u32 v, v1; -#ifdef CONFIG_X86_64 exit_idle (); -#endif irq_enter ();/* First tickle the hardware, only then report what went on. -- REW */ v =apic_read (APIC_ESR );
arch/x86/kernel/io_apic.c +0 -2
@@ -2242,9 +2242,7 @@ {unsigned vector, me;ack_APIC_irq (); -#ifdef CONFIG_X86_64 exit_idle (); -#endif irq_enter (); me =smp_processor_id ();
PATCH 1/1 xen/dom0: handle acpi lapic parsing in Xen dom0
In reply to Jeremy FitzhardingeWhen running in Xen dom0, we still want to parse the ACPI tables to find out about local and IO apics, but we don't want to actually use the lapics.
Put a couple of tests for Xen to prevent lapics from being mapped or accessed. This is very Xen-specific behaviour, so there didn't seem to be any point in adding more indirection.
--- arch/x86/include/asm/xen/hypervisor.h | 2 ++ arch/x86/kernel/acpi/boot.c | 10 ++++++++++ 2 files changed, 12 insertions(+)
arch/x86/include/asm/xen/hypervisor.h +2 -0
@@ -46,6 +46,8 @@extern enum xen_domain_type xen_domain_type;#ifdef CONFIG_XEN +#include <xen/interface/xen.h> +#define xen_domain () (xen_domain_type != XEN_NATIVE)#else #define xen_domain () (0)
arch/x86/kernel/acpi/boot.c +10 -0
@@ -42,6 +42,8 @@#include <asm/mpspec.h> #include <asm/smp.h> +#include <asm/xen/hypervisor.h> +#ifdef CONFIG_X86_LOCAL_APIC # include <mach_apic.h> #endif
@@ -234,6 +236,10 @@ {unsigned int ver =0 ; +/* We don't want to register lapics when in Xen dom0 */ +if (xen_initial_domain ()) +return ; +if (!enabled) { ++disabled_cpus;return ;
@@ -755,6 +761,10 @@static void __init acpi_register_lapic_address (unsigned long address) { +/* Xen dom0 doesn't have usable lapics */ +if (xen_initial_domain ()) +return ; + mp_lapic_addr = address;set_fixmap_nocache (FIX_APIC_BASE , address);
PATCH 1/1 x86: unstatic mp_find_ioapic so it can be used elsewhere
In reply to Jeremy Fitzhardinge--- arch/x86/include/asm/mpspec.h | 1 + arch/x86/kernel/acpi/boot.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
arch/x86/include/asm/mpspec.h +1 -0
@@ -63,6 +63,7 @@#ifdef CONFIG_X86_IO_APIC extern int mp_config_acpi_gsi (unsigned char number,unsigned int devfn,u8 pin,u32 gsi,int triggering,int polarity); +extern int mp_find_ioapic (int gsi);#else static inline int mp_config_acpi_gsi(unsigned char number, unsigned int devfn, u8 pin,
arch/x86/kernel/acpi/boot.c +1 -1
@@ -865,7 +865,7 @@DECLARE_BITMAP (pin_programmed,MP_MAX_IOAPIC_PIN +1 ); } mp_ioapic_routing[MAX_IO_APICS ]; -static int mp_find_ioapic (int gsi) +int mp_find_ioapic (int gsi) {int i =0 ;
PATCH 1/1 x86: add mp_find_ioapic_pin
In reply to Jeremy FitzhardingeAdd mp_find_ioapic_pin() to find an IO APIC's specific pin from a GSI, and use this function within acpi/boot. Make it non-static so other code can use it too.
--- arch/x86/include/asm/mpspec.h | 1 + arch/x86/kernel/acpi/boot.c | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-)
arch/x86/include/asm/mpspec.h +1 -0
@@ -64,6 +64,7 @@extern int mp_config_acpi_gsi (unsigned char number,unsigned int devfn,u8 pin,u32 gsi,int triggering,int polarity);extern int mp_find_ioapic (int gsi); +extern int mp_find_ioapic_pin (int ioapic,int gsi);#else static inline int mp_config_acpi_gsi(unsigned char number, unsigned int devfn, u8 pin,
arch/x86/kernel/acpi/boot.c +13 -3
@@ -880,6 +880,16 @@return -1 ; } +int mp_find_ioapic_pin (int ioapic,int gsi) +{ +if (WARN_ON (ioapic ==-1 )) +return -1 ; +if (WARN_ON (gsi > mp_ioapic_routing[ioapic].gsi_end)) +return -1 ; + +return gsi - mp_ioapic_routing[ioapic].gsi_base; +} +static u8 __init uniq_ioapic_id (u8 id) {#ifdef CONFIG_X86_32
@@ -992,7 +1002,7 @@ ioapic =mp_find_ioapic (gsi);if (ioapic <0 )return ; - pin = gsi - mp_ioapic_routing[ioapic].gsi_base; + pin =mp_find_ioapic_pin (ioapic, gsi);/* * TBD: This check is for faulty timer entries, where the override
@@ -1114,7 +1124,7 @@return gsi; } - ioapic_pin = gsi - mp_ioapic_routing[ioapic].gsi_base; + ioapic_pin =mp_find_ioapic_pin (ioapic, gsi);#ifdef CONFIG_X86_32 if (ioapic_renumber_irq)
@@ -1203,7 +1213,7 @@ mp_irq.mp_srcbusirq = (((devfn >>3 ) &0x1f ) <<2 ) | ((pin -1 ) &3 ); ioapic =mp_find_ioapic (gsi); mp_irq.mp_dstapic = mp_ioapic_routing[ioapic].apic_id; - mp_irq.mp_dstirq = gsi - mp_ioapic_routing[ioapic].gsi_base; + mp_irq.mp_dstirq =mp_find_ioapic_pin (ioapic, gsi);save_mp_irq (&mp_irq);#endif
PATCH 1/1 x86: unstatic ioapic entry funcs
In reply to Jeremy FitzhardingeUnstatic ioapic_write_entry and setup_ioapic_entry functions so that the Xen code can do its own ioapic routing setup.
--- arch/x86/include/asm/io_apic.h | 6 ++++++ arch/x86/kernel/io_apic.c | 10 +++++----- 2 files changed, 11 insertions(+), 5 deletions(-)
arch/x86/include/asm/io_apic.h +6 -0
@@ -209,6 +209,12 @@extern int probe_nr_irqs (void ); +extern int setup_ioapic_entry (int apic,int irq, +struct IO_APIC_route_entry *entry, +unsigned int destination,int trigger, +int polarity,int vector); +extern void ioapic_write_entry (int apic,int pin, +struct IO_APIC_route_entry e);#else /* !CONFIG_X86_IO_APIC */ #define io_apic_assign_pci_irqs 0static const int timer_through_8259 =0 ;
arch/x86/kernel/io_apic.c +5 -5
@@ -337,7 +337,7 @@io_apic_write (apic,0x10 +2 *pin, eu.w1); } -static void ioapic_write_entry (int apic,int pin,struct IO_APIC_route_entry e) +void ioapic_write_entry (int apic,int pin,struct IO_APIC_route_entry e) {unsigned long flags;spin_lock_irqsave (&ioapic_lock, flags);
@@ -1275,10 +1275,10 @@ handle_edge_irq, "edge"); } -static int setup_ioapic_entry (int apic,int irq, -struct IO_APIC_route_entry *entry, -unsigned int destination,int trigger, -int polarity,int vector) +int setup_ioapic_entry (int apic,int irq, +struct IO_APIC_route_entry *entry, +unsigned int destination,int trigger, +int polarity,int vector) {/* * add it to the IO-APIC irq-routing table:
PATCH 1/1 x86: include linux/init.h in asm/numa_64.h
In reply to Jeremy FitzhardingeIt uses __init/__cpuinit, which are not defined otherwise.
--- arch/x86/include/asm/numa_64.h | 1 + 1 file changed, 1 insertion(+)
arch/x86/include/asm/numa_64.h +1 -0
@@ -1,6 +1,7 @@#ifndef _ASM_X86_NUMA_64_H#define _ASM_X86_NUMA_64_H +#include <linux/init.h> #include <linux/nodemask.h> #include <asm/apicdef.h>
PATCH 1/1 x86: add swiotlb allocation functions
In reply to Jeremy FitzhardingeAdd x86-specific swiotlb allocation functions. These are purely default for the moment.
--- arch/x86/kernel/pci-swiotlb_64.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
arch/x86/kernel/pci-swiotlb_64.c +12 -0
@@ -3,6 +3,8 @@#include <linux/pci.h> #include <linux/cache.h> #include <linux/module.h> +#include <linux/swiotlb.h> +#include <linux/bootmem.h> #include <linux/dma-mapping.h> #include <asm/iommu.h>
@@ -11,6 +13,16 @@int swiotlb__read_mostly ; +void *swiotlb_alloc_boot (size_t size,unsigned long nslabs) +{ +return alloc_bootmem_low_pages (size); +} + +void *swiotlb_alloc (unsigned order,unsigned long nslabs) +{ +return (void *)__get_free_pages (GFP_DMA | __GFP_NOWARN, order); +} +static dma_addr_t swiotlb_map_single_phys (struct device *hwdev,phys_addr_t paddr,size_t size,int direction)
PATCH 1/1 x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
In reply to Jeremy Fitzhardingeswiotlb on 32 bit will be used by Xen domain 0 support.
--- arch/x86/include/asm/dma-mapping.h | 6 +----- arch/x86/include/asm/pci.h | 2 ++ arch/x86/include/asm/pci_64.h | 1 - arch/x86/kernel/Makefile | 3 ++- arch/x86/kernel/pci-dma.c | 6 ++++-- arch/x86/kernel/pci-swiotlb_64.c | 2 ++ arch/x86/mm/init_32.c | 3 +++ 7 files changed, 14 insertions(+), 9 deletions(-)
arch/x86/include/asm/dma-mapping.h +1 -5
@@ -66,21 +66,17 @@return dma_ops; else return dev->archdata.dma_ops; -#endif /* _ASM_X86_DMA_MAPPING_H */ +#endif }/* Make sure we keep the same behaviour */ static inline int dma_mapping_error (struct device *dev,dma_addr_t dma_addr) { -#ifdef CONFIG_X86_32 -return 0 ; -#else struct dma_mapping_ops *ops =get_dma_ops (dev);if (ops->mapping_error)return ops->mapping_error (dev, dma_addr);return (dma_addr == bad_dma_address); -#endif }#define dma_alloc_noncoherent (d, s, h, f) dma_alloc_coherent(d, s, h, f)
arch/x86/include/asm/pci.h +2 -0
@@ -84,6 +84,8 @@static inline void early_quirks (void ) { }#endif +extern void pci_iommu_alloc (void ); +#endif /* __KERNEL__ */ #ifdef CONFIG_X86_32
arch/x86/include/asm/pci_64.h +0 -1
@@ -23,7 +23,6 @@ int reg, int len, u32 value);extern void dma32_reserve_bootmem (void ); -extern void pci_iommu_alloc (void );/* The PCI address space does equal the physical memory * address space. The networking and block device layers use
arch/x86/kernel/Makefile +2 -1
@@ -115,6 +115,8 @@ obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o +obj-$(CONFIG_SWIOTLB) += pci-swiotlb_64.o # NB rename without _64 + ### # 64 bit specific files ifeq ($(CONFIG_X86_64),y)
@@ -128,7 +130,6 @@ obj-$(CONFIG_GART_IOMMU) += pci-gart_64.o aperture_64.o obj-$(CONFIG_CALGARY_IOMMU) += pci-calgary_64.o tce_64.o obj-$(CONFIG_AMD_IOMMU) += amd_iommu_init.o amd_iommu.o - obj-$(CONFIG_SWIOTLB) += pci-swiotlb_64.o obj-$(CONFIG_PCI_MMCONFIG) += mmconf-fam10h_64.o endif
arch/x86/kernel/pci-dma.c +4 -2
@@ -105,11 +105,15 @@ dma32_bootmem_ptr =NULL ; dma32_bootmem_size = 0; } +#endif void __init pci_iommu_alloc (void ) { +#ifdef CONFIG_X86_64 /* free the range so iommu could get some range less than 4G */ dma32_free_bootmem (); +#endif +/* * The order of these functions is important for * fall-back/fail-over reasons
@@ -125,8 +129,6 @@pci_swiotlb_init (); } -#endif -void *dma_generic_alloc_coherent (struct device *dev,size_t size,dma_addr_t *dma_addr,gfp_t flag) {
arch/x86/kernel/pci-swiotlb_64.c +2 -0
@@ -62,8 +62,10 @@void __init pci_swiotlb_init (void ) {/* don't initialize swiotlb if iommu=off (no_iommu=1) */ +#ifdef CONFIG_X86_64 if (!iommu_detected && !no_iommu && max_pfn >MAX_DMA32_PFN ) swiotlb =1 ; +#endif if (swiotlb_force) swiotlb =1 ;if (swiotlb) {
arch/x86/mm/init_32.c +3 -0
@@ -21,6 +21,7 @@#include <linux/init.h> #include <linux/highmem.h> #include <linux/pagemap.h> +#include <linux/pci.h> #include <linux/pfn.h> #include <linux/poison.h> #include <linux/bootmem.h>
@@ -971,6 +972,8 @@int codesize, reservedpages, datasize, initsize;int tmp; +pci_iommu_alloc (); +#ifdef CONFIG_FLATMEM BUG_ON (!mem_map);#endif
Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
Quoted 1 line · On Thu, 13 Nov 2008 11:10:16 -0800 Jeremy Fitzhardinge <[email protected]> wrote:
swiotlb on 32 bit will be used by Xen domain 0 support.
If you want swiotlb on 32 bit, you need more modifications, I think.
For example, the following code assumes that the mask needs to be 64 bits.
static void *map_single(struct device *hwdev, char *buffer, size_t size, int dir)
{
unsigned long flags;
char *dma_addr;
unsigned int nslots, stride, index, wrap;
int i;
unsigned long start_dma_addr;
unsigned long mask;
unsigned long offset_slots;
unsigned long max_slots;
mask = dma_get_seg_boundary(hwdev);
start_dma_addr = virt_to_bus(io_tlb_start) & mask;
offset_slots = ALIGN(start_dma_addr, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT;
max_slots = mask + 1
? ALIGN(mask + 1, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT
: 1UL << (BITS_PER_LONG - IO_TLB_SHIFT);Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
Quoted 6 lines · FUJITA Tomonori
Quoted 1 line · On Thu, 13 Nov 2008 11:10:16 -0800 Jeremy Fitzhardinge <[email protected]> wrote:
swiotlb on 32 bit will be used by Xen domain 0 support.
If you want swiotlb on 32 bit, you need more modifications, I think.
Possibly. It currently "Works For Me(tm)", but I should check it over.
Quoted 2 lines
For example, the following code assumes that the mask needs to be 64 bits.
The use of unsigned long for the mask is throughout the API and not simply limited to swiotlb.c. All the callers of dma_set_seg_boundary (PCI and SCSI subsys it seems) do not use a value >4G anywhere I can see. Presumably if something was we would see "warning: overflow in implicit constant conversion" somewhere along the line. If no value is set then the default is 0xffffffff which is safe on 32 bit.
I suspect that even with PAE addresses above 4G aren't seen very often due to pre-existing subsystem specific bounce buffers or other existing limitations (like network buffers being in lowmem).
Perhaps dma_addr_t should be used though?
Ian.
Quoted 21 lines
static void *
map_single(struct device *hwdev, char *buffer, size_t size, int dir){ unsigned long flags; char *dma_addr; unsigned int nslots, stride, index, wrap; int i; unsigned long start_dma_addr; unsigned long mask; unsigned long offset_slots; unsigned long max_slots; mask = dma_get_seg_boundary(hwdev); start_dma_addr = virt_to_bus(io_tlb_start) & mask; offset_slots = ALIGN(start_dma_addr, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT; max_slots = mask + 1 ? ALIGN(mask + 1, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT : 1UL << (BITS_PER_LONG - IO_TLB_SHIFT);
Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
Quoted 7 lines · On Mon, 17 Nov 2008 16:16:06 +0000 Ian Campbell <[email protected]> wrote:
Quoted 2 lines
For example, the following code assumes that the mask needs to be 64 bits.
The use of unsigned long for the mask is throughout the API and not simply limited to
swiotlb.c. All the callers ofdma_set_seg_boundary(PCI and SCSI subsys it seems) do not use a value >4G anywhere I can see.
32bit is large enough for dma segment boundary mask, I think.
The problem that I talked about in the previous mail:
Quoted 3 lines
max_slots = mask + 1 ? ALIGN(mask + 1, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT : 1UL << (BITS_PER_LONG - IO_TLB_SHIFT);
Since the popular value of the mask is 0xffffffff. So the above code (mask + 1 ?) works wrongly if the size of mask is 32bit (well, accidentally the result of max_slots is identical though).
Quoted 7 lines
Presumably if something was we would see "warning: overflow in implicit constant conversion" somewhere along the line. If no value is set then the default is
0xffffffffwhich is safe on 32 bit.I suspect that even with PAE addresses above 4G aren't seen very often due to pre-existing subsystem specific bounce buffers or other existing limitations (like network buffers being in lowmem).
I guess that you talk about the dma_mask (and coherent_dma_mask) in struct device. The dma segment boundary mask represents the different dma limitation of a device.
Quoted 1 line
Perhaps
dma_addr_tshould be used though?
I think that 'unsigned long' is better for the dma segment boundary mask since it represents the hardware limitation. The size of the value are not related with kernel configurations at all.
Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
Quoted 15 lines · FUJITA Tomonori
Quoted 7 lines · On Mon, 17 Nov 2008 16:16:06 +0000 Ian Campbell <[email protected]> wrote:
Quoted 2 lines
For example, the following code assumes that the mask needs to be 64 bits.
The use of unsigned long for the mask is throughout the API and not simply limited to
swiotlb.c. All the callers ofdma_set_seg_boundary(PCI and SCSI subsys it seems) do not use a value >4G anywhere I can see.32bit is large enough for dma segment boundary mask, I think.
The problem that I talked about in the previous mail:
Quoted 7 lines
Quoted 3 lines
max_slots = mask + 1 ? ALIGN(mask + 1, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT : 1UL << (BITS_PER_LONG - IO_TLB_SHIFT);Since the popular value of the mask is
0xffffffff. So the above code (mask + 1 ?) works wrongly if the size of mask is 32bit (well, accidentally the result ofmax_slotsis identical though).
Ah, I hadn't spotted this, you are right it probably works but just by chance. Thanks for pointing it out.
Quoted 11 lines
Quoted 7 lines
Presumably if something was we would see "warning: overflow in implicit constant conversion" somewhere along the line. If no value is set then the default is
0xffffffffwhich is safe on 32 bit.I suspect that even with PAE addresses above 4G aren't seen very often due to pre-existing subsystem specific bounce buffers or other existing limitations (like network buffers being in lowmem).
I guess that you talk about the
dma_mask(andcoherent_dma_mask) in struct device. The dma segment boundary mask represents the different dma limitation of a device.
I was talking about the segment_boundary_mask in struct device_dma_parameters which is the source of the "mask" value in the code you quoted.
Quoted 5 lines
Quoted 1 line
Perhaps
dma_addr_tshould be used though?I think that 'unsigned long' is better for the dma segment boundary mask since it represents the hardware limitation. The size of the value are not related with kernel configurations at all.
Right, it's just that on occasion we have to cope with slightly larger values while manipulating things.
Ian.
Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
In reply to FUJITA TomonoriQuoted 10 lines · FUJITA Tomonori
The problem that I talked about in the previous mail:
Quoted 3 lines
max_slots = mask + 1 ? ALIGN(mask + 1, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT : 1UL << (BITS_PER_LONG - IO_TLB_SHIFT);Since the popular value of the mask is
0xffffffff. So the above code (mask + 1 ?) works wrongly if the size of mask is 32bit (well, accidentally the result ofmax_slotsis identical though).
I've just been looking at this again and I don't think it is an accident that this evaluates to the correct value when mask + 1 == 0.
The patch which adds the "mask + 1 ? ... : 1UL << ..." stuff is:
commit b15a3891c916f32a29832886a053a48be2741d4d
Author: Jan Beulich <[email protected]>
Date: Thu Mar 13 09:13:30 2008 +0000
avoid endless loops in lib/swiotlb.c
Commit 681cc5cd3efbeafca6386114070e0bfb5012e249 ("iommu sg merging:
swiotlb: respect the segment boundary limits") introduced two possibilities for entering an endless loop in lib/swiotlb.c:
- if max_slots is zero (possible if mask is ~0UL) [...]
I think the existing code is the nicest way to handle this corner case and it is necessary anyway to handle the ~0UL case on 64 bit.
Ian.
Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
Quoted 27 lines · On Fri, 21 Nov 2008 14:21:32 +0000 Ian Campbell <[email protected]> wrote:
Quoted 10 lines · FUJITA Tomonori
The problem that I talked about in the previous mail:
Quoted 3 lines
max_slots = mask + 1 ? ALIGN(mask + 1, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT : 1UL << (BITS_PER_LONG - IO_TLB_SHIFT);Since the popular value of the mask is
0xffffffff. So the above code (mask + 1 ?) works wrongly if the size of mask is 32bit (well, accidentally the result ofmax_slotsis identical though).I've just been looking at this again and I don't think it is an accident that this evaluates to the correct value when mask + 1 == 0.
The patch which adds the "mask + 1 ? ... : 1UL << ..." stuff is:
commit
b15a3891c916f32a29832886a053a48be2741d4d
Author: Jan Beulich <[email protected]>
Date: Thu Mar 13 09:13:30 2008 +0000avoid endless loops in
lib/swiotlb.cCommit
681cc5cd3efbeafca6386114070e0bfb5012e249("iommu sg merging:
swiotlb: respect the segment boundary limits") introduced two possibilities for entering an endless loop inlib/swiotlb.c:
Quoted 5 lines
- if
max_slotsis zero (possible if mask is ~0UL) [...]I think the existing code is the nicest way to handle this corner case and it is necessary anyway to handle the ~0UL case on 64 bit.
Ah, I vaguely remember this patch. The ~0ULL mask didn't happen here (nobody uses it) so the possibility was false. IMHO, if we use this code on 32bit architectures, the mask should be u64 and the overflow should be handled explicitly. But as you pointed out, looks like that this patch takes account of the overflow.
Thanks,
Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
Quoted 33 lines · FUJITA Tomonori
Quoted 30 lines · On Fri, 21 Nov 2008 14:21:32 +0000 Ian Campbell <[email protected]> wrote:
Quoted 10 lines · FUJITA Tomonori
The problem that I talked about in the previous mail:
Quoted 3 lines
max_slots = mask + 1 ? ALIGN(mask + 1, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT : 1UL << (BITS_PER_LONG - IO_TLB_SHIFT);Since the popular value of the mask is
0xffffffff. So the above code (mask + 1 ?) works wrongly if the size of mask is 32bit (well, accidentally the result ofmax_slotsis identical though).I've just been looking at this again and I don't think it is an accident that this evaluates to the correct value when mask + 1 == 0.
The patch which adds the "mask + 1 ? ... : 1UL << ..." stuff is:
commit
b15a3891c916f32a29832886a053a48be2741d4d
Author: Jan Beulich <[email protected]>
Date: Thu Mar 13 09:13:30 2008 +0000avoid endless loops in
lib/swiotlb.cCommit
681cc5cd3efbeafca6386114070e0bfb5012e249("iommu sg merging:
swiotlb: respect the segment boundary limits") introduced two possibilities for entering an endless loop inlib/swiotlb.c:- if
max_slotsis zero (possible if mask is ~0UL) [...]
Quoted 8 lines
Quoted 2 lines
I think the existing code is the nicest way to handle this corner case and it is necessary anyway to handle the ~0UL case on 64 bit.
Ah, I vaguely remember this patch. The ~0ULL mask didn't happen here (nobody uses it) so the possibility was false. IMHO, if we use this code on 32bit architectures, the mask should be u64 and the overflow should be handled explicitly. But as you pointed out, looks like that this patch takes account of the overflow.
Something like this?
Ian.
---
swiotlb: explicitly handle segment boundary mask overflow.
When swiotlb is used on 32 bit we can overflow mask + 1 in the common case where mask is 0xffffffffUL. This overflow was previously caught by the case which attempts to handle a mask of ~0UL on 64 bit.
lib/swiotlb.c +2 -1
@@ -303,7 +303,7 @@unsigned int nslots, stride, index, wrap;int i;unsigned long start_dma_addr; -unsigned long mask; +u64 mask;unsigned long offset_slots;unsigned long max_slots;
@@ -314,6 +314,7 @@ max_slots = mask +1 ?ALIGN (mask +1 ,1 <<IO_TLB_SHIFT ) >>IO_TLB_SHIFT :1UL << (BITS_PER_LONG -IO_TLB_SHIFT ); +BUG_ON (max_slots >1UL << (BITS_PER_LONG -IO_TLB_SHIFT ));/* * For mappings greater than a page, we limit the stride (and
Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
Quoted 69 lines · On Mon, 24 Nov 2008 11:41:37 +0000 Ian Campbell <[email protected]> wrote:
Quoted 33 lines · FUJITA Tomonori
Quoted 30 lines · On Fri, 21 Nov 2008 14:21:32 +0000 Ian Campbell <[email protected]> wrote:
Quoted 10 lines · FUJITA Tomonori
The problem that I talked about in the previous mail:
Quoted 3 lines
max_slots = mask + 1 ? ALIGN(mask + 1, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT : 1UL << (BITS_PER_LONG - IO_TLB_SHIFT);Since the popular value of the mask is
0xffffffff. So the above code (mask + 1 ?) works wrongly if the size of mask is 32bit (well, accidentally the result ofmax_slotsis identical though).I've just been looking at this again and I don't think it is an accident that this evaluates to the correct value when mask + 1 == 0.
The patch which adds the "mask + 1 ? ... : 1UL << ..." stuff is:
commit
b15a3891c916f32a29832886a053a48be2741d4d
Author: Jan Beulich <[email protected]>
Date: Thu Mar 13 09:13:30 2008 +0000avoid endless loops in
lib/swiotlb.cCommit
681cc5cd3efbeafca6386114070e0bfb5012e249("iommu sg merging:
swiotlb: respect the segment boundary limits") introduced two possibilities for entering an endless loop inlib/swiotlb.c:- if
max_slotsis zero (possible if mask is ~0UL) [...]Quoted 8 lines
Quoted 2 lines
I think the existing code is the nicest way to handle this corner case and it is necessary anyway to handle the ~0UL case on 64 bit.
Ah, I vaguely remember this patch. The ~0ULL mask didn't happen here (nobody uses it) so the possibility was false. IMHO, if we use this code on 32bit architectures, the mask should be u64 and the overflow should be handled explicitly. But as you pointed out, looks like that this patch takes account of the overflow.
Something like this?
Ian.
---swiotlb: explicitly handle segment boundary mask overflow.
When swiotlb is used on 32 bit we can overflow mask + 1 in the common case where mask is
0xffffffffUL. This overflow was previously caught by the case which attempts to handle a mask of ~0UL on 64 bit.Signed-off-by Ian Campbell <[email protected]>diff -r
5fa30e5284ddlib/swiotlb.clib/swiotlb.c +1
-1@@ -303,7 +303,7 @@unsigned int nslots, stride, index, wrap;int i;unsigned long start_dma_addr; -unsigned long mask; +u64 mask;unsigned long offset_slots;unsigned long max_slots;
lib/swiotlb.c line 317
Quoted 4 lines
lib/swiotlb.c +1
-0@@ -314,6 +314,7 @@ max_slots = mask +1 ?ALIGN (mask +1 ,1 <<IO_TLB_SHIFT ) >>IO_TLB_SHIFT :1UL << (BITS_PER_LONG -IO_TLB_SHIFT ); +BUG_ON (max_slots >1UL << (BITS_PER_LONG -IO_TLB_SHIFT ));
How can this BUG_ON happen? Using u64 for the mask is fine though.
Thanks,
Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
Quoted 4 lines · FUJITA Tomonori
Quoted 2 lines
+
BUG_ON(max_slots > 1UL << (BITS_PER_LONG - IO_TLB_SHIFT));How can this
BUG_ONhappen? Using u64 for the mask is fine though.
It covers the cases where the previous code would have overflowed. It can't happen right now because although mask is 64 bits the value assigned to it is currently sizeof(unsigned long). If someone changes the type of that field then we would start seeing unexpected values.
Ian.
Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
Quoted 10 lines · On Wed, 26 Nov 2008 09:36:49 +0000 Ian Campbell <[email protected]> wrote:
Quoted 4 lines · FUJITA Tomonori
Quoted 2 lines
+
BUG_ON(max_slots > 1UL << (BITS_PER_LONG - IO_TLB_SHIFT));How can this
BUG_ONhappen? Using u64 for the mask is fine though.It covers the cases where the previous code would have overflowed. It can't happen right now because although mask is 64 bits the value assigned to it is currently sizeof(unsigned long). If someone changes the type of that field then we would start seeing unexpected values.
If someone changes dma_get_seg_boundary to return a u64 value instead of unsigned long, this BUG_ON could happen on 32bit architectures. But you don't need to trigger BUG_ON for it. max_slots > 1UL << (BITS_PER_LONG - IO_TLB_SHIFT) should be fine for iommu_is_span_boundary().
Anyway, this is minor but would it be nice to make sure that anyone can easily understand the code without digging into the git log?
a) dropping this patch and adding some comments how the code works (especially about the overflow on 32bit architectures).
b) removing the BUG_ON in this patch and adding some comments.
?
Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
Quoted 20 lines · FUJITA Tomonori
Quoted 10 lines · On Wed, 26 Nov 2008 09:36:49 +0000 Ian Campbell <[email protected]> wrote:
Quoted 4 lines · FUJITA Tomonori
Quoted 2 lines
+
BUG_ON(max_slots > 1UL << (BITS_PER_LONG - IO_TLB_SHIFT));How can this
BUG_ONhappen? Using u64 for the mask is fine though.It covers the cases where the previous code would have overflowed. It can't happen right now because although mask is 64 bits the value assigned to it is currently sizeof(unsigned long). If someone changes the type of that field then we would start seeing unexpected values.
If someone changes
dma_get_seg_boundaryto return a u64 value instead of unsigned long, thisBUG_ONcould happen on 32bit architectures. But you don't need to triggerBUG_ONfor it.max_slots> 1UL << (BITS_PER_LONG-IO_TLB_SHIFT) should be fine foriommu_is_span_boundary().
Quoted 7 lines
Anyway, this is minor but would it be nice to make sure that anyone can easily understand the code without digging into the git log?
a) dropping this patch and adding some comments how the code works (especially about the overflow on 32bit architectures).
b) removing the
BUG_ONin this patch and adding some comments.
Yes, I think adding a comment to the existing code (option a) would be best. I actually have a small queue of other fixes which make swiotlb work properly for x86 PAE and HighMem but they are not particularly well baked at the moment. I'll include a patch to add a comment in that series.
Ian.
Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
Quoted 24 lines · On Thu, 27 Nov 2008 17:14:47 +0000 Ian Campbell <[email protected]> wrote:
Quoted 23 lines · FUJITA Tomonori
Quoted 10 lines · On Wed, 26 Nov 2008 09:36:49 +0000 Ian Campbell <[email protected]> wrote:
Quoted 4 lines · FUJITA Tomonori
Quoted 2 lines
+
BUG_ON(max_slots > 1UL << (BITS_PER_LONG - IO_TLB_SHIFT));How can this
BUG_ONhappen? Using u64 for the mask is fine though.It covers the cases where the previous code would have overflowed. It can't happen right now because although mask is 64 bits the value assigned to it is currently sizeof(unsigned long). If someone changes the type of that field then we would start seeing unexpected values.
If someone changes
dma_get_seg_boundaryto return a u64 value instead of unsigned long, thisBUG_ONcould happen on 32bit architectures. But you don't need to triggerBUG_ONfor it.max_slots> 1UL << (BITS_PER_LONG-IO_TLB_SHIFT) should be fine foriommu_is_span_boundary().Anyway, this is minor but would it be nice to make sure that anyone can easily understand the code without digging into the git log?
Quoted 7 lines
Quoted 4 lines
a) dropping this patch and adding some comments how the code works (especially about the overflow on 32bit architectures).
b) removing the
BUG_ONin this patch and adding some comments.Yes, I think adding a comment to the existing code (option a) would be best.
Yeah, that should be fine.
Quoted 4 lines
I actually have a small queue of other fixes which make swiotlb work properly for x86 PAE and HighMem but they are not particularly well baked at the moment. I'll include a patch to add a comment in that series.
I see.
Thanks,
PATCH 1/1 x86: add arch specific version of the swiotlb virt<->bus<->virt functions
In reply to Jeremy FitzhardingeFrom: Ian Campbell <[email protected]>
These are to be used later and contain the default implementation.
--- arch/x86/kernel/pci-swiotlb_64.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
arch/x86/kernel/pci-swiotlb_64.c +10 -0
@@ -23,6 +23,16 @@return (void *)__get_free_pages (GFP_DMA | __GFP_NOWARN, order); } +unsigned long swiotlb_virt_to_bus (volatile void *address) +{ +return virt_to_bus (address); +} + +void *swiotlb_bus_to_virt (unsigned long address) +{ +return bus_to_virt (address); +} +static dma_addr_t swiotlb_map_single_phys (struct device *hwdev,phys_addr_t paddr,size_t size,int direction)
PATCH 1/1 xen dom0: Make hvc_xen console work for dom0
In reply to Jeremy FitzhardingeUse the console hypercalls for dom0 console.
--- drivers/char/hvc_xen.c | 100 +++++++++++++++++++++++++++++++----------------- drivers/xen/events.c | 2 include/xen/events.h | 2 3 files changed, 69 insertions(+), 35 deletions(-)
drivers/char/hvc_xen.c +66 -34
@@ -55,7 +55,7 @@notify_remote_via_evtchn (xen_start_info->console.domU.evtchn); } -static int write_console (uint32_t vtermno,const char *data,int len) +static int domU_write_console (uint32_t vtermno,const char *data,int len) {struct xencons_interface *intf =xencons_interface ();XENCONS_RING_IDX cons, prod;
@@ -76,7 +76,7 @@return sent; } -static int read_console (uint32_t vtermno,char *buf,int len) +static int domU_read_console (uint32_t vtermno,char *buf,int len) {struct xencons_interface *intf =xencons_interface ();XENCONS_RING_IDX cons, prod;
@@ -97,28 +97,62 @@return recv; } -static struct hv_ops hvc_ops = { - .get_chars = read_console, - .put_chars = write_console, +static struct hv_ops domU_hvc_ops = { + .get_chars = domU_read_console, + .put_chars = domU_write_console, .notifier_add = notifier_add_irq, .notifier_del = notifier_del_irq, .notifier_hangup = notifier_hangup_irq, }; -static int __init xen_init (void ) +static int dom0_read_console (uint32_t vtermno,char *buf,int len) +{ +return HYPERVISOR_console_io (CONSOLEIO_read, len, buf); +} + +/* +* Either for a dom0 to write to the system console, or a domU with a +* debug version of Xen +*/ +static int dom0_write_console (uint32_t vtermno,const char *str,int len) +{ +int rc =HYPERVISOR_console_io (CONSOLEIO_write, len, (char *)str); +if (rc <0 ) +return 0 ; + +return len; +} + +static struct hv_ops dom0_hvc_ops = { + .get_chars = dom0_read_console, + .put_chars = dom0_write_console, + .notifier_add = notifier_add_irq, + .notifier_del = notifier_del_irq, +}; + +static int __init xen_hvc_init (void ) {struct hvc_struct *hp; +struct hv_ops *ops; -if (!xen_pv_domain () || -xen_initial_domain () || - !xen_start_info->console.domU.evtchn) -return -ENODEV ; +if (!xen_pv_domain ()) +return -ENODEV ; - xencons_irq =bind_evtchn_to_irq (xen_start_info->console.domU.evtchn); +if (xen_initial_domain ()) { + ops = &dom0_hvc_ops; + xencons_irq =bind_virq_to_irq (VIRQ_CONSOLE ,0 ); + }else { +if (!xen_start_info->console.domU.evtchn) +return -ENODEV ; + + ops = &domU_hvc_ops; + xencons_irq =bind_evtchn_to_irq (xen_start_info->console.domU.evtchn); + } +if (xencons_irq <0 ) xencons_irq =0 ;/* NO_IRQ */ - hp =hvc_alloc (HVC_COOKIE , xencons_irq, &hvc_ops,256 ); + hp =hvc_alloc (HVC_COOKIE , xencons_irq, ops,256 );if (IS_ERR (hp))return PTR_ERR (hp);
@@ -135,7 +169,7 @@rebind_evtchn_irq (xen_start_info->console.domU.evtchn, xencons_irq); } -static void __exit xen_fini(void) +static void __exit xen_hvc_fini(void) {if (hvc)hvc_remove (hvc);
@@ -143,29 +177,24 @@static int xen_cons_init (void ) { +struct hv_ops *ops; +if (!xen_pv_domain ())return 0 ; -hvc_instantiate (HVC_COOKIE ,0 , &hvc_ops); + ops = &domU_hvc_ops; +if (xen_initial_domain ()) + ops = &dom0_hvc_ops; + +hvc_instantiate (HVC_COOKIE ,0 , ops); +return 0 ; } -module_init (xen_init); -module_exit (xen_fini); +module_init (xen_hvc_init); +module_exit (xen_hvc_fini);console_initcall (xen_cons_init); -static void raw_console_write (const char *str,int len) -{ -while (len >0 ) { -int rc =HYPERVISOR_console_io (CONSOLEIO_write, len, (char *)str); -if (rc <=0 ) -break ; - - str += rc; - len -= rc; - } -} -#ifdef CONFIG_EARLY_PRINTK static void xenboot_write_console (struct console *console,const char *string,unsigned len)
@@ -173,19 +202,22 @@unsigned int linelen, off =0 ;const char *pos; -raw_console_write (string, len); +dom0_write_console (0 , string, len); -write_console (0 ,"(early) " ,8 ); +if (xen_initial_domain ()) +return ; + +domU_write_console (0 ,"(early) " ,8 );while (off < len &&NULL != (pos =strchr (string+off,'\n' ))) { linelen = pos-string+off;if (off + linelen > len)break ; -write_console (0 , string+off, linelen); -write_console (0 ,"\r\n" ,2 ); +domU_write_console (0 , string+off, linelen); +domU_write_console (0 ,"\r\n" ,2 ); off += linelen +1 ; }if (off < len) -write_console (0 , string+off, len-off); +domU_write_console (0 , string+off, len-off); }struct console xenboot_console = {
@@ -197,7 +229,7 @@void xen_raw_console_write (const char *str) { -raw_console_write (str,strlen (str)); +dom0_write_console (0 , str,strlen (str)); }void xen_raw_printk (const char *fmt, ...)
drivers/xen/events.c +1 -1
@@ -307,7 +307,7 @@ } -static int bind_virq_to_irq (unsigned int virq,unsigned int cpu) +int bind_virq_to_irq (unsigned int virq,unsigned int cpu) {struct evtchn_bind_virq bind_virq;int evtchn, irq;
include/xen/events.h +2 -0
@@ -12,6 +12,8 @@irq_handler_t handler, unsigned long irqflags, const char *devname, void *dev_id); +int bind_virq_to_irq (unsigned int virq,unsigned int cpu); + int bind_virq_to_irqhandler(unsigned int virq, unsigned int cpu, irq_handler_t handler, unsigned long irqflags, const char *devname,
PATCH 1/1 xen dom0: Initialize xenbus for dom0
In reply to Jeremy FitzhardingeFrom: Juan Quintela <[email protected]>
Do initial xenbus/xenstore setup in dom0. In dom0 we need to actually allocate the xenstore resources, rather than being given them from outside.
--- drivers/xen/xenbus/xenbus_probe.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)
drivers/xen/xenbus/xenbus_probe.c +29 -1
@@ -810,6 +810,7 @@static int __init xenbus_probe_init (void ) {int err =0 ; +unsigned long page =0 ;DPRINTK ("" );
@@ -830,7 +831,31 @@* Domain0 doesn't have a store_evtchn or store_mfn yet. */ if (xen_initial_domain ()) { -/* dom0 not yet supported */ +struct evtchn_alloc_unbound alloc_unbound; + +/* Allocate Xenstore page */ + page =get_zeroed_page (GFP_KERNEL ); +if (!page) +return -ENOMEM ; + + xen_store_mfn = xen_start_info->store_mfn = +pfn_to_mfn (virt_to_phys ((void *)page) >> +PAGE_SHIFT ); + +/* Next allocate a local port which xenstored can bind to */ + alloc_unbound.dom =DOMID_SELF ; + alloc_unbound.remote_dom =0 ; + + err =HYPERVISOR_event_channel_op (EVTCHNOP_alloc_unbound, + &alloc_unbound); +if (err == -ENOSYS ) + goto out_unreg_front; + +BUG_ON (err); + xen_store_evtchn = xen_start_info->store_evtchn = + alloc_unbound.port; + + xen_store_interface =mfn_to_virt (xen_store_mfn); }else { xenstored_ready =1 ; xen_store_evtchn = xen_start_info->store_evtchn;
@@ -858,6 +883,9 @@bus_unregister (&xenbus_frontend.bus); out_error: +if (page !=0 ) +free_page (page); +return err; }
PATCH 1/1 xen dom0: Set up basic IO permissions for dom0
In reply to Jeremy FitzhardingeFrom: Juan Quintela <[email protected]>
Add the direct mapping area for ISA bus access, and enable IO space access for the guest when running as dom0.
--- arch/x86/xen/enlighten.c | 32 ++++++++++++++++++++++++++++++++ arch/x86/xen/setup.c | 6 +++++- 2 files changed, 37 insertions(+), 1 deletion(-)
arch/x86/xen/enlighten.c +32 -0
@@ -1437,6 +1437,7 @@return __ka (m2p (maddr)); } +/* Set the page permissions on an identity-mapped pages */ static void set_page_prot (void *addr,pgprot_t prot) {unsigned long pfn =__pa (addr) >>PAGE_SHIFT ;
@@ -1492,6 +1493,29 @@set_page_prot (pmd,PAGE_KERNEL_RO ); } +static __init void xen_ident_map_ISA (void ) +{ +unsigned long pa; + +/* +* If we're dom0, then linear map the ISA machine addresses into +* the kernel's address space. +*/ +if (!xen_initial_domain ()) +return ; + +xen_raw_printk ("Xen: setup ISA identity maps\n" ); + +for (pa =ISA_START_ADDRESS ; pa <ISA_END_ADDRESS ; pa +=PAGE_SIZE ) { +pte_t pte =mfn_pte (PFN_DOWN (pa),PAGE_KERNEL_IO ); + +if (HYPERVISOR_update_va_mapping (PAGE_OFFSET + pa, pte,0 )) +BUG (); + } + +xen_flush_tlb (); +} +#ifdef CONFIG_X86_64 static void convert_pfn_mfn (void *v) {
@@ -1674,6 +1698,7 @@xen_raw_console_write ("mapping kernel into physical memory\n" ); pgd =xen_setup_kernel_pagetable (pgd, xen_start_info->nr_pages); +xen_ident_map_ISA (); init_mm.pgd = pgd;
@@ -1683,6 +1708,13 @@if (xen_feature (XENFEAT_supervisor_mode_kernel)) pv_info.kernel_rpl =0 ; +if (xen_initial_domain ()) { +struct physdev_set_iopl set_iopl; + set_iopl.iopl =1 ; +if (HYPERVISOR_physdev_op (PHYSDEVOP_set_iopl, &set_iopl) ==-1 ) +BUG (); + } +/* set the limit of our address space */ xen_reserve_top ();
arch/x86/xen/setup.c +5 -1
@@ -51,6 +51,9 @@* Even though this is normal, usable memory under Xen, reserve * ISA memory anyway because too many things think they can poke * about in there. +* +* In a dom0 kernel, this region is identity mapped with the +* hardware ISA area, so it really is out of bounds. */ e820_add_region (ISA_START_ADDRESS ,ISA_END_ADDRESS -ISA_START_ADDRESS ,E820_RESERVED );
@@ -188,7 +191,8 @@ pm_idle = xen_idle; -paravirt_disable_iospace (); +if (!xen_initial_domain ()) +paravirt_disable_iospace ();fiddle_vdso (); }
PATCH 1/1 xen-dom0: only selectively disable cpu features
In reply to Jeremy FitzhardingeDom0 kernels actually want most of the CPU features to be enabled. Some, like MCA/MCE, are still handled by Xen itself.
We leave APIC enabled even though we don't really have a functional local apic so that the ACPI code will parse the corresponding tables properly.
--- arch/x86/xen/enlighten.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
arch/x86/xen/enlighten.c +13 -8
@@ -205,18 +205,23 @@static void xen_cpuid (unsigned int *ax,unsigned int *bx,unsigned int *cx,unsigned int *dx) { -unsigned maskedx = ~0 ; +unsigned maskedx =0 ;/* * Mask out inconvenient features, to try and disable as many * unsupported kernel subsystems as possible. */ -if (*ax ==1 ) - maskedx = ~((1 <<X86_FEATURE_APIC ) |/* disable APIC */ - (1 <<X86_FEATURE_ACPI ) |/* disable ACPI */ - (1 <<X86_FEATURE_MCE ) |/* disable MCE */ - (1 <<X86_FEATURE_MCA ) |/* disable MCA */ - (1 <<X86_FEATURE_ACC ));/* thermal monitoring */ +if (*ax ==1 ) { + maskedx = + (1 <<X86_FEATURE_MCE ) |/* disable MCE */ + (1 <<X86_FEATURE_MCA ) |/* disable MCA */ + (1 <<X86_FEATURE_ACC );/* thermal monitoring */ + +if (!xen_initial_domain ()) + maskedx |= + (1 <<X86_FEATURE_APIC ) |/* disable local APIC */ + (1 <<X86_FEATURE_ACPI );/* disable ACPI */ + } asm(XEN_EMULATE_PREFIX "cpuid" : "=a" (*ax),
@@ -224,7 +229,7 @@ "=c" (*cx), "=d" (*dx) : "0" (*ax), "2" (*cx)); - *dx &= maskedx; + *dx &= ~maskedx; }static void xen_set_debugreg (int reg,unsigned long val)
PATCH 1/1 xen dom0: Add support for the platform_ops hypercall
In reply to Jeremy FitzhardingeFrom: Stephen Tweedie <[email protected]>
Minimal changes to get platform ops (renamed dom0_ops on pv_ops) working on pv_ops builds. Pulls in upstream linux-2.6.18-xen.hg's platform.h
--- arch/x86/include/asm/xen/hypercall.h | 8 + include/xen/interface/platform.h | 232 ++++++++++++++++++++++++++++++++++ include/xen/interface/xen.h | 2 3 files changed, 242 insertions(+)
arch/x86/include/asm/xen/hypercall.h +8 -0
@@ -45,6 +45,7 @@#include <xen/interface/xen.h> #include <xen/interface/sched.h> #include <xen/interface/physdev.h> +#include <xen/interface/platform.h> /* * The hypercall asms have to meet several constraints:
@@ -282,6 +283,13 @@ }static inline int +HYPERVISOR_dom0_op (struct xen_platform_op *platform_op) +{ + platform_op->interface_version =XENPF_INTERFACE_VERSION ; +return _hypercall1 (int, dom0_op, platform_op); +} + +static inline int HYPERVISOR_set_debugreg (int reg,unsigned long value) {return _hypercall2 (int, set_debugreg, reg, value);
include/xen/interface/platform.h new file +232 -0
232 lines not on this page · raw message
include/xen/interface/xen.h +2 -0
@@ -461,6 +461,8 @@#define __mk_unsigned_long (x) x ## UL#define mk_unsigned_long (x) __mk_unsigned_long(x) +DEFINE_GUEST_HANDLE (uint64_t); +#else /* __ASSEMBLY__ */ /* In assembly code we cannot use C numeric constant suffixes. */
PATCH 1/1 xen mtrr: Add mtrr_ops support for Xen mtrr
In reply to Jeremy FitzhardingeFrom: Stephen Tweedie <[email protected]>
Add a Xen mtrr type, and reorganise mtrr initialisation slightly to allow the mtrr driver to set up num_var_ranges (Xen needs to do this by querying the hypervisor itself.)
Only the boot path is handled for now: we set up a xen-specific mtrr_if and set up the mtrr tables based on hypervisor information, but we don't yet handle mtrr entry add/delete.
--- arch/x86/kernel/cpu/mtrr/Makefile | 1 arch/x86/kernel/cpu/mtrr/amd.c | 1 arch/x86/kernel/cpu/mtrr/centaur.c | 1 arch/x86/kernel/cpu/mtrr/cyrix.c | 1 arch/x86/kernel/cpu/mtrr/generic.c | 1 arch/x86/kernel/cpu/mtrr/main.c | 11 ++++-- arch/x86/kernel/cpu/mtrr/mtrr.h | 5 +++ arch/x86/kernel/cpu/mtrr/xen.c | 59 ++++++++++++++++++++++++++++++++++++ 8 files changed, 77 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/mtrr/Makefile +1 -0
@@ -1,3 +1,4 @@ obj-y := main.o if.o generic.o state.o obj-$(CONFIG_X86_32) += amd.o cyrix.o centaur.o +obj-$(CONFIG_XEN_DOM0) += xen.o
arch/x86/kernel/cpu/mtrr/amd.c +1 -0
@@ -108,6 +108,7 @@ .get_free_region = generic_get_free_region, .validate_add_page = amd_validate_add_page, .have_wrcomb = positive_have_wrcomb, + .num_var_ranges = common_num_var_ranges, };int __init amd_init_mtrr (void )
arch/x86/kernel/cpu/mtrr/centaur.c +1 -0
@@ -213,6 +213,7 @@ .get_free_region = centaur_get_free_region, .validate_add_page = centaur_validate_add_page, .have_wrcomb = positive_have_wrcomb, + .num_var_ranges = common_num_var_ranges, };int __init centaur_init_mtrr (void )
arch/x86/kernel/cpu/mtrr/cyrix.c +1 -0
@@ -263,6 +263,7 @@ .get_free_region = cyrix_get_free_region, .validate_add_page = generic_validate_add_page, .have_wrcomb = positive_have_wrcomb, + .num_var_ranges = common_num_var_ranges, };int __init cyrix_init_mtrr (void )
arch/x86/kernel/cpu/mtrr/generic.c +1 -0
@@ -667,4 +667,5 @@ .set = generic_set_mtrr, .validate_add_page = generic_validate_add_page, .have_wrcomb = generic_have_wrcomb, + .num_var_ranges = common_num_var_ranges, };
arch/x86/kernel/cpu/mtrr/main.c +8 -3
@@ -99,7 +99,7 @@ }/* This function returns the number of variable MTRRs */ -static void __init set_num_var_ranges (void ) +int __init common_num_var_ranges (void ) {unsigned long config =0 , dummy;
@@ -109,7 +109,7 @@ config =2 ; else if (is_cpu(CYRIX) || is_cpu(CENTAUR)) config =8 ; - num_var_ranges = config &0xff ; +return config &0xff ; }static void __init init_table (void )
@@ -1676,12 +1676,17 @@void __init mtrr_bp_init (void ) {u32 phys_addr; +init_ifs (); phys_addr =32 ;if (cpu_has_mtrr) { mtrr_if = &generic_mtrr_ops; +#ifdef CONFIG_XEN_DOM0 +xen_init_mtrr (); +#endif + size_or_mask =0xff000000 ;/* 36 bits */ size_and_mask =0x00f00000 ; phys_addr =36 ;
@@ -1739,7 +1744,7 @@ }if (mtrr_if) { -set_num_var_ranges (); + num_var_ranges = mtrr_if->num_var_ranges ();init_table ();if (use_intel ()) {get_mtrr_state ();
arch/x86/kernel/cpu/mtrr/mtrr.h +5 -0
@@ -50,6 +50,8 @@int (*validate_add_page)(unsigned long base,unsigned long size,unsigned int type);int (*have_wrcomb)(void ); + +int (*num_var_ranges)(void ); }; extern int generic_get_free_region(unsigned long base, unsigned long size,
@@ -61,6 +63,8 @@extern int positive_have_wrcomb (void ); +extern int __init common_num_var_ranges (void ); +/* library functions for processor-specific routines */ struct set_mtrr_context {unsigned long flags;
@@ -104,3 +108,4 @@int amd_init_mtrr (void );int cyrix_init_mtrr (void );int centaur_init_mtrr (void ); +void xen_init_mtrr (void );
arch/x86/kernel/cpu/mtrr/xen.c new file +59 -0
59 lines not on this page · raw message
Re: [PATCH 25 of 38] xen mtrr: Add mtrr_ops support for Xen mtrr
Quoted 5 lines · Jeremy Fitzhardinge
arch/x86/kernel/cpu/mtrr/xen.c new file +0
-0@@ -0,0 +1,59 @@
...
arch/x86/kernel/cpu/mtrr/xen.c line 29
Quoted 13 lines
arch/x86/kernel/cpu/mtrr/xen.c new file +13
-0+ +/* DOM0 TODO: Need to fill in the remaining mtrr methods to have full +* working userland mtrr support. */ +static struct mtrr_ops xen_mtrr_ops = { + .vendor =X86_VENDOR_UNKNOWN , +// .set = xen_set_mtrr, +// .get = xen_get_mtrr, + .get_free_region = generic_get_free_region, +// .validate_add_page = xen_validate_add_page, + .have_wrcomb = positive_have_wrcomb, + .use_intel_if =0 , + .num_var_ranges = xen_num_var_ranges, +};
...
I'm vague on the details now, but looking back at the dom0 patch set here:
I see we did have a bunch more mtrr work e.g. fixing the TODO above:
Cheers,
Mark.
Re: [PATCH 25 of 38] xen mtrr: Add mtrr_ops support for Xen mtrr
Quoted 28 lines · Mark McLoughlin
Quoted 6 lines · Jeremy Fitzhardinge
arch/x86/kernel/cpu/mtrr/xen.c new file +0
-0@@ -0,0 +1,59 @@...
Quoted 14 lines
arch/x86/kernel/cpu/mtrr/xen.c new file +13
-0+ +/* DOM0 TODO: Need to fill in the remaining mtrr methods to have full +* working userland mtrr support. */ +static struct mtrr_ops xen_mtrr_ops = { + .vendor =X86_VENDOR_UNKNOWN , +// .set = xen_set_mtrr, +// .get = xen_get_mtrr, + .get_free_region = generic_get_free_region, +// .validate_add_page = xen_validate_add_page, + .have_wrcomb = positive_have_wrcomb, + .use_intel_if =0 , + .num_var_ranges = xen_num_var_ranges, +};...
Quoted 9 lines
I'm vague on the details now, but looking back at the dom0 patch set here:
I see we did have a bunch more mtrr work e.g. fixing the TODO above:
Yes, the mtrr changes are incomplete. I started on them as much as necessary to get things booting, and then left the rest to revisit. It's not a particularly pretty part of the kernel, and so I was hoping some magic beautification fairy would visit it before I needed to touch it more...
J
PATCH 1/1 xen: forcibly disable PAT support
In reply to Jeremy FitzhardingeFrom: Ian Campbell <[email protected]>
Xen imposes a particular PAT layout on all paravirtual guests which does not match the layout Linux would like to use.
Force PAT to be disabled until this is resolved.
--- arch/x86/include/asm/pat.h | 4 ++-- arch/x86/xen/enlighten.c | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-)
arch/x86/include/asm/pat.h +2 -2
@@ -6,9 +6,11 @@#ifdef CONFIG_X86_PAT extern int pat_enabled;extern void validate_pat_support (struct cpuinfo_x86 *c); +extern void pat_disable (char *reason);#else static const int pat_enabled;static inline void validate_pat_support (struct cpuinfo_x86 *c) { } +static inline void pat_disable (char *reason) { }#endif extern void pat_init (void );
@@ -17,6 +19,4 @@ unsigned long req_type, unsigned long *ret_type);extern int free_memtype (u64 start,u64 end); -extern void pat_disable (char *reason); -#endif /* _ASM_X86_PAT_H */
arch/x86/xen/enlighten.c +3 -0
@@ -48,6 +48,7 @@#include <asm/pgtable.h> #include <asm/tlbflush.h> #include <asm/reboot.h> +#include <asm/pat.h> #include "xen-ops.h" #include "mmu.h"
@@ -1743,6 +1744,8 @@add_preferred_console ("hvc" ,0 ,NULL ); } +pat_disable ("PAT disabled on Xen" ); +xen_raw_console_write ("about to get started...\n" );/* Start the world */
PATCH 1/1 xen/dom0: use _PAGE_IOMAP in ioremap to do machine mappings
In reply to Jeremy FitzhardingeIn a Xen domain, ioremap operates on machine addresses, not pseudo-physical addresses. We use _PAGE_IOMAP to determine whether a mapping is intended for machine addresses.
--- arch/x86/include/asm/xen/page.h | 8 +---- arch/x86/xen/enlighten.c | 14 +++++++-- arch/x86/xen/mmu.c | 60 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 73 insertions(+), 9 deletions(-)
arch/x86/include/asm/xen/page.h +2 -6
@@ -112,13 +112,9 @@*/ static inline unsigned long mfn_to_local_pfn (unsigned long mfn) { -extern unsigned long max_mapnr;unsigned long pfn =mfn_to_pfn (mfn); -if ((pfn < max_mapnr) - && !xen_feature (XENFEAT_auto_translated_physmap) - && (get_phys_to_machine (pfn) != mfn)) -return max_mapnr;/* force !pfn_valid() */ -/* XXX fixme; not true with sparsemem */ +if (get_phys_to_machine (pfn) != mfn) +return -1 ;/* force !pfn_valid() */ return pfn; }
arch/x86/xen/enlighten.c +12 -2
@@ -1162,11 +1162,19 @@#ifdef CONFIG_X86_LOCAL_APIC case FIX_APIC_BASE :/* maps dummy local APIC */ #endif +/* All local page mappings */ pte =pfn_pte (phys, prot);break ; +case FIX_PARAVIRT_BOOTMAP : +/* This is an MFN, but it isn't an IO mapping from the +IO domain */ + pte =mfn_pte (phys, prot); +break ; +default : - pte =mfn_pte (phys, prot); +/* By default, set_fixmap is used for hardware mappings */ + pte =mfn_pte (phys,__pgprot (pgprot_val (prot) | _PAGE_IOMAP));break ; }
@@ -1695,7 +1703,9 @@/* Prevent unwanted bits from being set in PTEs. */ __supported_pte_mask &= ~_PAGE_GLOBAL; -if (!xen_initial_domain ()) +if (xen_initial_domain ()) + __supported_pte_mask |= _PAGE_IOMAP; +else __supported_pte_mask &= ~(_PAGE_PWT | _PAGE_PCD);/* Don't do the full vcpu_info placement stuff until we have a
arch/x86/xen/mmu.c +59 -1
98 lines not on this page · raw message
PATCH 1/1 paravirt/xen: add pvop for page_is_ram
In reply to Jeremy FitzhardingeA guest domain may have external pages mapped into its address space, in order to share memory with other domains. These shared pages are more akin to io mappings than real RAM, and should not pass the page_is_ram test. Add a paravirt op for this so that a hypervisor backend can validate whether a page should be considered ram or not.
--- arch/x86/include/asm/page.h | 9 ++++++++- arch/x86/include/asm/paravirt.h | 7 +++++++ arch/x86/kernel/paravirt.c | 1 + arch/x86/mm/ioremap.c | 2 +- arch/x86/xen/enlighten.c | 11 +++++++++++ 5 files changed, 28 insertions(+), 2 deletions(-)
arch/x86/include/asm/page.h +8 -1
@@ -56,7 +56,14 @@typedef struct {pgdval_t pgd; }pgd_t ;typedef struct {pgprotval_t pgprot; }pgprot_t ; -extern int page_is_ram (unsigned long pagenr); +extern int native_page_is_ram (unsigned long pagenr); +#ifndef CONFIG_PARAVIRT +static inline int page_is_ram (unsigned long pagenr) +{ +return native_page_is_ram (pagenr); +} +#endif +extern int pagerange_is_ram (unsigned long start,unsigned long end);extern int devmem_is_allowed (unsigned long pagenr); extern void map_devmem(unsigned long pfn, unsigned long size,
arch/x86/include/asm/paravirt.h +7 -0
@@ -321,6 +321,8 @@an mfn. We can tell which is which from the index. */ void (*set_fixmap)(unsigned /* enum fixed_addresses */ idx,unsigned long phys,pgprot_t flags); + +int (*page_is_ram)(unsigned long pfn); };struct raw_spinlock ;
@@ -1386,6 +1388,11 @@ pv_mmu_ops.set_fixmap (idx, phys, flags); } +static inline int page_is_ram (unsigned long pfn) +{ +return PVOP_CALL1 (int, pv_mmu_ops.page_is_ram, pfn); +} +void _paravirt_nop (void );#define paravirt_nop ((void *)_paravirt_nop)
arch/x86/kernel/paravirt.c +1 -0
@@ -451,6 +451,7 @@ }, .set_fixmap = native_set_fixmap, + .page_is_ram = native_page_is_ram, };EXPORT_SYMBOL_GPL (pv_time_ops);
arch/x86/mm/ioremap.c +1 -1
@@ -97,7 +97,7 @@#endif -int page_is_ram (unsigned long pagenr) +int native_page_is_ram (unsigned long pagenr) {resource_size_t addr, end;int i;
arch/x86/xen/enlighten.c +11 -0
@@ -1190,6 +1190,16 @@#endif } +static int xen_page_is_ram (unsigned long pfn) +{ +/* Granted pages are not RAM. They will not have a proper +identity pfn<->mfn translation. */ +if (mfn_to_local_pfn (pfn_to_mfn (pfn)) != pfn) +return 0 ; + +return native_page_is_ram (pfn); +} +static const struct pv_info xen_info__initdata = { .paravirt_enabled =1 , .shared_kernel_pmd =0 ,
@@ -1364,6 +1374,7 @@ }, .set_fixmap = xen_set_fixmap, + .page_is_ram = xen_page_is_ram, };static void xen_reboot (int reason)
PATCH 1/1 xen: create dummy ioapic mapping
In reply to Jeremy FitzhardingeWe don't allow direct access to the IO apic, so make sure that any request to map it just "maps" non-present pages. We should see any attempts at direct access explode nicely.
--- arch/x86/xen/enlighten.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
arch/x86/xen/enlighten.c +10 -0
@@ -1166,6 +1166,16 @@ pte =pfn_pte (phys, prot);break ; +#ifdef CONFIG_X86_IO_APIC + case FIX_IO_APIC_BASE_0 ... FIX_IO_APIC_BASE_END: +/* +* We just don't map the IO APIC - all access is via +* hypercalls. Keep the address in the pte for reference. +*/ + pte =pfn_pte (phys,PAGE_NONE ); +break ; +#endif +case FIX_PARAVIRT_BOOTMAP :/* This is an MFN, but it isn't an IO mapping from the IO domain */
PATCH 1/1 xen: implement io_apic_ops
In reply to Jeremy FitzhardingeWrites to the IO APIC are paravirtualized via hypercalls, so implement the appropriate operations.
--- arch/x86/xen/Makefile | 3 +- arch/x86/xen/apic.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ arch/x86/xen/enlighten.c | 2 + arch/x86/xen/xen-ops.h | 2 + 4 files changed, 72 insertions(+), 1 deletion(-)
arch/x86/xen/Makefile +2 -1
@@ -9,4 +9,5 @@ time.o xen-asm_$(BITS).o grant-table.o suspend.o obj-$(CONFIG_SMP) += smp.o spinlock.o -obj-$(CONFIG_XEN_DEBUG_FS) += debugfs.o +obj-$(CONFIG_XEN_DEBUG_FS) += debugfs.o +obj-$(CONFIG_XEN_DOM0) += apic.o
arch/x86/xen/apic.c new file +66 -0
66 lines not on this page · raw message
arch/x86/xen/enlighten.c +2 -0
@@ -1750,6 +1750,8 @@ set_iopl.iopl =1 ;if (HYPERVISOR_physdev_op (PHYSDEVOP_set_iopl, &set_iopl) ==-1 )BUG (); + +xen_init_apic (); }/* set the limit of our address space */
arch/x86/xen/xen-ops.h +2 -0
@@ -64,6 +64,8 @@#endif +void xen_init_apic (void ); +/* Declare an asm function, along with symbols needed to make it inlineable */ #define DECL_ASM (ret, name, ...) \
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Hi,
it seems that if CONFIG_XEN is set by CONFIG_XEN_DOM0 is not set, then the call to xen_init_apic() in xen_start_kernel() causes the build to fail.
One possible soluion to this is to provide a dummy version of xen_init_apic() in the !CONFIG_XEN_DOM0 case.
Another possible solution would be to add #ifdef CONFIG_XEN_DOM0 inside xen_start_kernel()
#make gcc --version
gcc (Debian 4.3.2-1) 4.3.2
Copyright (C) 2008 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
# make [snip] UPD include/linux/compile.h CC init/version.o LD init/built-in.o LD .tmp_vmlinux1 arch/x86/xen/built-in.o: In function `xen_start_kernel': (.init.text+0x8ef): undefined reference to `xen_init_apic'
arch/x86/xen/xen-ops.h +4 -0
@@ -64,7 +64,11 @@ static inline void xen_smp_init(void) {}#endif +#ifdef CONFIG_XEN_DOM0 void xen_init_apic (void ); +#else +static inline void xen_init_apic (void ) { ; } +#endif /* Declare an asm function, along with symbols needed to make it inlineable */
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Quoted 6 lines · Simon Horman
Hi,
it seems that if
CONFIG_XENis set byCONFIG_XEN_DOM0is not set, then the call toxen_init_apic()inxen_start_kernel()causes the build to fail.
Quoted 6 lines
One possible soluion to this is to provide a dummy version of
xen_init_apic()in the !CONFIG_XEN_DOM0 case.Another possible solution would be to add #ifdef
CONFIG_XEN_DOM0insidexen_start_kernel()
Ah, OK, will fix that up.
J
Re: [PATCH 30 of 38] xen: implement io_apic_ops
In reply to Jeremy FitzhardingeQuoted 10 lines · Jeremy Fitzhardinge
Writes to the IO APIC are paravirtualized via hypercalls, so implement the appropriate operations.
Signed-off-by Jeremy Fitzhardinge <[email protected]>--- arch/x86/xen/Makefile | 3 +- arch/x86/xen/apic.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ arch/x86/xen/enlighten.c | 2 + arch/x86/xen/xen-ops.h | 2 + 4 files changed, 72 insertions(+), 1 deletion(-)
hm, why is the ioapic used as the API here, and not an irqchip?
Ingo
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Quoted 15 lines · Ingo Molnar
Quoted 11 lines · Jeremy Fitzhardinge
Writes to the IO APIC are paravirtualized via hypercalls, so implement the appropriate operations.
Signed-off-by Jeremy Fitzhardinge <[email protected]>--- arch/x86/xen/Makefile | 3 +- arch/x86/xen/apic.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ arch/x86/xen/enlighten.c | 2 + arch/x86/xen/xen-ops.h | 2 + 4 files changed, 72 insertions(+), 1 deletion(-)
Quoted 2 lines
hm, why is the ioapic used as the API here, and not an irqchip?
In essence, the purpose of the series is to break the 1:1 relationship between Linux irqs and hardware GSIs. This allows me to have my own irq allocator, which in turn allows me to intermix "physical" irqs (ie, a Linux irq number bound to a real hardware interrupt source) with the various software/virtual irqs the Xen system needs.
Once a physical irq has been mapped onto a gsi interrupt source, the mechanisms for handing the ioapic side of things are more or less the same. There's the same procedure of finding the ioapic/pin for a gsi and programming the appropriate vector.
(Presumably once I implement MSI support, all references to "gsi" will become "gsi/msi/etc".)
So, there's an awkward tradeoff. I could just completely duplicate the whole irq/vector/ioapic management code and hide it under my own irqchip, but it would end up duplicating a lot of the existing code. My alternative was to try to open out the existing code into something like a thin ioapic library, which I can call into as needed. The only low-level difference is that the Xen ioapics need to be programmed via a hypercall rather than register writes.
If the x86 interrupt layer in general decouples irqs from GSIs, then I can probably make use of that to clean things up. A general irq allocator along with some way of attaching interrupt-source-specific information to each irq would get me a long way, I think. I'd still need hooks to paravirtualize the actual ioapic writes, but at least I wouldn't need to have quite so much delicate hooking.
Thanks,
J
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Quoted 42 lines · Jeremy Fitzhardinge
Quoted 15 lines · Ingo Molnar
Quoted 11 lines · Jeremy Fitzhardinge
Writes to the IO APIC are paravirtualized via hypercalls, so implement the appropriate operations.
Signed-off-by Jeremy Fitzhardinge <[email protected]>--- arch/x86/xen/Makefile | 3 +- arch/x86/xen/apic.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ arch/x86/xen/enlighten.c | 2 + arch/x86/xen/xen-ops.h | 2 + 4 files changed, 72 insertions(+), 1 deletion(-)Quoted 2 lines
hm, why is the ioapic used as the API here, and not an irqchip?
In essence, the purpose of the series is to break the 1:1 relationship between Linux irqs and hardware GSIs. This allows me to have my own irq allocator, which in turn allows me to intermix "physical" irqs (ie, a Linux irq number bound to a real hardware interrupt source) with the various software/virtual irqs the Xen system needs.
Once a physical irq has been mapped onto a gsi interrupt source, the mechanisms for handing the ioapic side of things are more or less the same. There's the same procedure of finding the ioapic/pin for a gsi and programming the appropriate vector.
(Presumably once I implement MSI support, all references to "gsi" will become "gsi/msi/etc".)
So, there's an awkward tradeoff. I could just completely duplicate the whole irq/vector/ioapic management code and hide it under my own irqchip, but it would end up duplicating a lot of the existing code. My alternative was to try to open out the existing code into something like a thin ioapic library, which I can call into as needed. The only low-level difference is that the Xen ioapics need to be programmed via a hypercall rather than register writes.
Quoted 6 lines
If the x86 interrupt layer in general decouples irqs from GSIs, then I can probably make use of that to clean things up. A general irq allocator along with some way of attaching interrupt-source-specific information to each irq would get me a long way, I think. I'd still need hooks to paravirtualize the actual ioapic writes, but at least I wouldn't need to have quite so much delicate hooking.
it certainly looks thin enough to me although i'm really not sure we want to virtualize at the IO-APIC level. Peter, what's your opinion/preference?
Ingo
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Quoted 5 lines · Ingo Molnar
it certainly looks thin enough to me although i'm really not sure we want to virtualize at the IO-APIC level. Peter, what's your opinion/preference?
relative fixed mapping, always make it simple and tracked easily. like irq nr == gsi nr...
YH
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Quoted 9 lines · Yinghai Lu
Quoted 3 lines · Ingo Molnar
it certainly looks thin enough to me although i'm really not sure we want to virtualize at the IO-APIC level. Peter, what's your opinion/preference?
relative fixed mapping, always make it simple and tracked easily. like irq nr == gsi nr...
That was a bit terse for me to understand your point. The code already has irq==gsi. Are you proposing that it stay that way, or something else?
J
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Quoted 10 lines · Jeremy Fitzhardinge
Quoted 9 lines · Yinghai Lu
Quoted 5 lines · Ingo Molnar
it certainly looks thin enough to me although i'm really not sure we want to virtualize at the IO-APIC level. Peter, what's your opinion/preference?
Quoted 5 lines
Quoted 2 lines
relative fixed mapping, always make it simple and tracked easily. like irq nr == gsi nr...
That was a bit terse for me to understand your point. The code already has irq==gsi. Are you proposing that it stay that way
yes
YH
Re: [PATCH 30 of 38] xen: implement io_apic_ops
In reply to Ingo MolnarQuoted 2 lines · Ingo Molnar
it certainly looks thin enough to me although i'm really not sure we want to virtualize at the IO-APIC level. Peter, what's your opinion/preference?
Given that Xen's requirements here are pretty Xen-specific (I don't imagine that any other virtualization system would work in the same way), I didn't bother trying to come up with a general "virtualization" layer at this level - that's why I was pretty blunt about putting xen_* calls in without any indirection. But the code could certainly be restructured in a way which would make it simpler to hook Xen as a side-effect, so long as it was achieving some other goal as well (general cleanup, big iron architecture support, better msi handling, whatever...).
J
H. Peter Anvin
reviewerRe: [PATCH 30 of 38] xen: implement io_apic_ops
In reply to Ingo MolnarQuoted 5 lines · Ingo Molnar
it certainly looks thin enough to me although i'm really not sure we want to virtualize at the IO-APIC level. Peter, what's your opinion/preference?
Not having studied the Xen code in detail, but my assumption would be that we should allocate this at the IRQ chip level rather than violating the IO-APIC code. I also (as previously discussed) really want to see a dynamic allocator for IRQ numbers like PowerPC already has.
-hpa
Re: [PATCH 30 of 38] xen: implement io_apic_ops
In reply to Jeremy FitzhardingeQuoted 16 lines · Jeremy Fitzhardinge
Quoted 15 lines · Ingo Molnar
Quoted 11 lines · Jeremy Fitzhardinge
Writes to the IO APIC are paravirtualized via hypercalls, so implement the appropriate operations.
Signed-off-by Jeremy Fitzhardinge <[email protected]>--- arch/x86/xen/Makefile | 3 +- arch/x86/xen/apic.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ arch/x86/xen/enlighten.c | 2 + arch/x86/xen/xen-ops.h | 2 + 4 files changed, 72 insertions(+), 1 deletion(-)
Quoted 5 lines
Quoted 2 lines
hm, why is the ioapic used as the API here, and not an irqchip?
In essence, the purpose of the series is to break the 1:1 relationship between Linux irqs and hardware GSIs.
Bad idea (I think). We have a 1:1 relationship between the linux irq number and the GSI because it makes the code dramatically simpler, and it took significant work to get there. The concept of an intermediate mapping layer sounds nasty. But I haven't yet read the patch.
Quoted 6 lines
If the x86 interrupt layer in general decouples irqs from GSIs, then I can probably make use of that to clean things up. A general irq allocator along with some way of attaching interrupt-source-specific information to each irq would get me a long way, I think. I'd still need hooks to paravirtualize the actual ioapic writes, but at least I wouldn't need to have quite so much delicate hooking.
I know that is where I thought the sparse irq code was going.
Eric
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Quoted 24 lines · Eric W. Biederman
Quoted 21 lines · Jeremy Fitzhardinge
Quoted 19 lines · Ingo Molnar
Quoted 12 lines · Jeremy Fitzhardinge
Writes to the IO APIC are paravirtualized via hypercalls, so implement the appropriate operations.
Signed-off-by Jeremy Fitzhardinge <[email protected]>--- arch/x86/xen/Makefile | 3 +- arch/x86/xen/apic.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ arch/x86/xen/enlighten.c | 2 + arch/x86/xen/xen-ops.h | 2 + 4 files changed, 72 insertions(+), 1 deletion(-)hm, why is the ioapic used as the API here, and not an irqchip?
Quoted 9 lines
Quoted 3 lines
In essence, the purpose of the series is to break the 1:1 relationship between Linux irqs and hardware GSIs.
Bad idea (I think). We have a 1:1 relationship between the linux irq number and the GSI because it makes the code dramatically simpler, and it took significant work to get there. The concept of an intermediate mapping layer sounds nasty. But I haven't yet read the patch.
The changes are spread over a number of patches, but the meat of it is in "xen: route hardware irqs via Xen". It turns out fairly simply, but perhaps its because I've made a number of simplifying assumptions: interrupts are always IOAPIC based, only using ACPI for routing, no MSI support yet.
But it seems to me that the only time you really care that the irq isn't a gsi is when programming a vector into the ioapics - you need to do a irq -> ioapic/pin mapping anyway, so adding a irq -> gsi -> ioapic/pin map isn't all that complex. And conversely, when probing devices you need to map gsi->irq to see whether the interrupt is shared, though you could do that on a pure gsi level anyway.
And of course the current code isn't purely irq == gsi anyway, since msis are allocated irqs as well, and there's no underlying gsi. In a sense you can think of the other Xen interrupt sources as being a bit like MSI, at least in as much as they're not sourced from a GSI (but they go further and are not sourced from an IOAPIC at all).
J
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Quoted 9 lines · Jeremy Fitzhardinge
The changes are spread over a number of patches, but the meat of it is in "xen:
route hardware irqs via Xen". It turns out fairly simply, but perhaps its because I've made a number of simplifying assumptions: interrupts are always IOAPIC based, only using ACPI for routing, no MSI support yet.But it seems to me that the only time you really care that the irq isn't a gsi is when programming a vector into the ioapics - you need to do a irq -> ioapic/pin mapping anyway, so adding a irq -> gsi -> ioapic/pin map isn't all that complex.
It is hideous. Been there and ripped out hundreds of lines of useless and problem causing code to get here. It is especially bad when you do not identity map the first 16 gsi to linux irqs (the legacy isa irqs).
It is very useful to keep the linux irq number and the gsi irq number the same. It facilitates debugging and keeps the code simple.
Quoted 3 lines
And conversely, when probing devices you need to map
gsi->irqto see whether the interrupt is shared, though you could do that on a pure gsi level anyway.
What I care about is important is that the GSI number == the linux irq number.
Quoted 2 lines
And of course the current code isn't purely irq == gsi anyway, since msis are allocated irqs as well, and there's no underlying gsi.
Yep. And but the numbers we you should be beyond the range of the gsi's so there is no conflict. Think of it an extension of how we identitly make the low 16 linux irqs.
Quoted 4 lines
In a sense you can think
of the other Xen interrupt sources as being a bit like MSI, at least in as much as they're not sourced from a GSI (but they go further and are not sourced from an IOAPIC at all).
MSI isn't sourced from an IOAPIC either.
The difference is that the xen sources are not delivered using vectors. The cpu vector numbers we do hide and treat as an implementation detail. And I am totally happy not going through the vector allocation path.
My gut feel says that you just want to use a different set of irq operations when
doing Xen native and working with hardware interrupts. I haven't seen the code so
I don't know how you interact there. Except in dom0 this is not a consideration so I don't how it is handled.
Eric
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Quoted 15 lines · Eric W. Biederman
Quoted 10 lines · Jeremy Fitzhardinge
The changes are spread over a number of patches, but the meat of it is in "xen:
route hardware irqs via Xen". It turns out fairly simply, but perhaps its because I've made a number of simplifying assumptions: interrupts are always IOAPIC based, only using ACPI for routing, no MSI support yet.But it seems to me that the only time you really care that the irq isn't a gsi is when programming a vector into the ioapics - you need to do a irq -> ioapic/pin mapping anyway, so adding a irq -> gsi -> ioapic/pin map isn't all that complex.
Quoted 4 lines
It is hideous. Been there and ripped out hundreds of lines of useless and problem causing code to get here. It is especially bad when you do not identity map the first 16 gsi to linux irqs (the legacy isa irqs).
Yes. I made that concession too, and just reserved them as identity mapped legacy irqs.
Quoted 4 lines
Yep. And but the numbers we you should be beyond the range of the gsi's so there is no conflict. Think of it an extension of how we identitly make the low 16 linux irqs.
Yes, I suppose we can statically partition the irq space. In fact the original 2.6.18-xen dom0 kernel does precisely that, but runs into limitations because of the compile-time limit on NR_IRQS in that kernel. If we move to a purely dynamically allocated irq space, then having a sparse allocation if irqs becomes reasonable again, for msis and vectorless Xen interrupts.
Quoted 8 lines
Quoted 5 lines
In a sense you can think
of the other Xen interrupt sources as being a bit like MSI, at least in as much as they're not sourced from a GSI (but they go further and are not sourced from an IOAPIC at all).MSI isn't sourced from an IOAPIC either.
Right.
Quoted 4 lines
The difference is that the xen sources are not delivered using vectors. The cpu vector numbers we do hide and treat as an implementation detail. And I am totally happy not going through the vector allocation path.
Right. And in the physical irq event channel case, the vector space is managed by Xen, so we need to use Xen to allocate the vector, then program that into the appropriate place in the ioapic.
Quoted 5 lines
My gut feel says that you just want to use a different set of irq operations when
doing Xen native and working with hardware interrupts. I haven't seen the code so
I don't know how you interact there. Except in dom0 this is not a consideration so I don't how it is handled.
Yeah. In the domU case, where there's no physical interrupts, the Xen code completely avoids the ioapic/vector stuff, and directly converts an event channel into an irq. Indeed, physical irq delivery is handled the same way; its just that the setup requires touching the ioapics to program the appropriate vector and bind it to an event channel.
J
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Quoted 7 lines · Jeremy Fitzhardinge
Yes, I suppose we can statically partition the irq space. In fact the original 2.6.18-xen dom0 kernel does precisely that, but runs into limitations because of the compile-time limit on
NR_IRQSin that kernel. If we move to a purely dynamically allocated irq space, then having a sparse allocation if irqs becomes reasonable again, for msis and vectorless Xen interrupts.
Quoted 8 lines
Quoted 4 lines
The difference is that the xen sources are not delivered using vectors. The cpu vector numbers we do hide and treat as an implementation detail. And I am totally happy not going through the vector allocation path.
Right. And in the physical irq event channel case, the vector space is managed by Xen, so we need to use Xen to allocate the vector, then program that into the appropriate place in the ioapic.
We should be able to share code with iommu for irqs handling, at first glance you are describing a pretty similar problem. Now I don't know think the interrupt remapping code is any kind of beauty but that seems to be roughly what you are doing with Xen domU. I certainly think with some careful factoring we can share the ioapic munging code. And the code to pick how we program the ioapics.
Quoted 11 lines
Quoted 5 lines
My gut feel says that you just want to use a different set of irq operations when doing Xen native and working with hardware interrupts. I haven't seen the code so I don't know how you interact there. Except in dom0 this is not a consideration so I don't how it is handled.
Yeah. In the domU case, where there's no physical interrupts, the Xen code completely avoids the ioapic/vector stuff, and directly converts an event channel into an irq. Indeed, physical irq delivery is handled the same way; its just that the setup requires touching the ioapics to program the appropriate vector and bind it to an event channel.
Reasonable. A lot like the intel interrupt remapping code in that respect. The message we program in has little to do with the vector the interrupt arrives on.
So I don't quite know where to hook it but if we are careful we should be able to get a good interrupt mapping.
Eric
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Quoted 20 lines · Eric W. Biederman
Quoted 7 lines · Jeremy Fitzhardinge
Yes, I suppose we can statically partition the irq space. In fact the original 2.6.18-xen dom0 kernel does precisely that, but runs into limitations because of the compile-time limit on
NR_IRQSin that kernel. If we move to a purely dynamically allocated irq space, then having a sparse allocation if irqs becomes reasonable again, for msis and vectorless Xen interrupts.Quoted 9 lines
Quoted 5 lines
The difference is that the xen sources are not delivered using vectors. The cpu vector numbers we do hide and treat as an implementation detail. And I am totally happy not going through the vector allocation path.
Right. And in the physical irq event channel case, the vector space is managed by Xen, so we need to use Xen to allocate the vector, then program that into the appropriate place in the ioapic.
Quoted 7 lines
We should be able to share code with iommu for irqs handling, at first glance you are describing a pretty similar problem. Now I don't know think the interrupt remapping code is any kind of beauty but that seems to be roughly what you are doing with Xen domU. I certainly think with some careful factoring we can share the ioapic munging code. And the code to pick how we program the ioapics.
Notwithstanding the possibility that there'll be general changes to x86 interrupt handing in the future, do you have any objection to my patches as they stand? Ingo would like to see your and/or hpa's ack before accepting them.
Should I repost them?
Thanks,
J
Re: [PATCH 30 of 38] xen: implement io_apic_ops
Quoted 5 lines · Jeremy Fitzhardinge
Notwithstanding the possibility that there'll be general changes to x86 interrupt handing in the future, do you have any objection to my patches as they stand? Ingo would like to see your and/or hpa's ack before accepting them.
Should I repost them?
I couldn't track down the first set. So if you could repost them with me on the cc I can review them, and tell you if I have specific objections. My apologies for the trouble.
Eric
PATCH 1/1 xen: set irq_chip disable
In reply to Jeremy FitzhardingeBy default, the irq_chip.disable operation is a no-op. Explicitly set it to disable the Xen event channel.
--- drivers/xen/events.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/xen/events.c +3 -0
@@ -806,8 +806,11 @@static struct irq_chip xen_dynamic_chip__read_mostly = { .name ="xen-dyn" , + + .disable = disable_dynirq, .mask = disable_dynirq, .unmask = enable_dynirq, + .ack = ack_dynirq, .set_affinity = set_affinity_irq, .retrigger = retrigger_dynirq,
PATCH 1/1 xen: use our own eventchannel->irq path
In reply to Jeremy FitzhardingeRather than overloading vectors for event channels, take full responsibility for mapping an event channel to irq directly. With this patch Xen has its own irq allocator.
When the kernel gets an event channel upcall, it maps the event channel number to an irq and injects it into the normal interrupt path.
--- arch/x86/include/asm/xen/events.h | 6 ------ arch/x86/xen/irq.c | 17 +---------------- drivers/xen/events.c | 22 ++++++++++++++++++++-- 3 files changed, 21 insertions(+), 24 deletions(-)
arch/x86/include/asm/xen/events.h +0 -6
@@ -15,10 +15,4 @@ return raw_irqs_disabled_flags(regs->flags); } -static inline void xen_do_IRQ (int irq,struct pt_regs *regs) -{ - regs->orig_ax = ~irq; -do_IRQ (regs); -} -#endif /* _ASM_X86_XEN_EVENTS_H */
arch/x86/xen/irq.c +1 -16
@@ -19,21 +19,6 @@ (void )HYPERVISOR_xen_version (0 ,NULL ); } -static void __init __xen_init_IRQ (void ) -{ -int i; - -/* Create identity vector->irq map */ -for (i =0 ; i <NR_VECTORS ; i++) { -int cpu; - -for_each_possible_cpu (cpu) -per_cpu (vector_irq, cpu)[i] = i; - } - -xen_init_IRQ (); -} -static unsigned long xen_save_fl (void ) {struct vcpu_info *vcpu;
@@ -123,7 +108,7 @@ }static const struct pv_irq_ops xen_irq_ops__initdata = { - .init_IRQ = __xen_init_IRQ, + .init_IRQ = xen_init_IRQ, .save_fl = xen_save_fl, .restore_fl = xen_restore_fl, .irq_disable = xen_irq_disable,
drivers/xen/events.c +20 -2
@@ -29,6 +29,7 @@#include <asm/ptrace.h> #include <asm/irq.h> +#include <asm/idle.h> #include <asm/sync_bitops.h> #include <asm/xen/hypercall.h> #include <asm/xen/hypervisor.h>
@@ -503,6 +504,24 @@ } +static void xen_do_irq (unsigned irq,struct pt_regs *regs) +{ +struct pt_regs *old_regs =set_irq_regs (regs); + +if (WARN_ON (irq ==-1 )) +return ; + +exit_idle (); +irq_enter (); + +//printk("cpu %d handling irq %d\n", smp_processor_id(), info->irq); +handle_irq (irq, regs); + +irq_exit (); + +set_irq_regs (old_regs); +} +/* * Search the CPUs pending events bitmasks. For each one found, map * the event number to an irq, and feed it into do_IRQ() for
@@ -543,8 +562,7 @@int port = (word_idx *BITS_PER_LONG ) + bit_idx;int irq = evtchn_to_irq[port]; -if (irq !=-1 ) -xen_do_IRQ (irq, regs); +xen_do_irq (irq, regs); } }
PATCH 1/1 xen: pack all irq-related info together
In reply to Jeremy FitzhardingePut all irq info into one struct. Also, use a union to keep event channel type-specific information, rather than overloading the index field.
--- drivers/xen/events.c | 184 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 135 insertions(+), 49 deletions(-)
drivers/xen/events.c +135 -49
@@ -51,18 +51,8 @@/* IRQ <-> IPI mapping */ static DEFINE_PER_CPU(int, ipi_to_irq[XEN_NR_IPIS]) = {[0 ... XEN_NR_IPIS-1] = -1}; -/* Packed IRQ information: binding type, sub-type index, and event channel. */ -struct packed_irq -{ -unsigned short evtchn; -unsigned char index; -unsigned char type; -}; - -static struct packed_irq irq_info[NR_IRQS ]; - -/* Binding types. */ -enum { +/* Interrupt types. */ +enum xen_irq_type {IRQT_UNBOUND ,IRQT_PIRQ ,IRQT_VIRQ ,
@@ -70,14 +60,39 @@ IRQT_EVTCHN }; -/* Convenient shorthand for packed representation of an unbound IRQ. */ -#define IRQ_UNBOUND mk_irq_info(IRQT_UNBOUND, 0, 0) +/* +* Packed IRQ information: +* type - enum xen_irq_type +* event channel - irq->event channel mapping +* cpu - cpu this event channel is bound to +* index - type-specific information: +* PIRQ - vector, with MSB being "needs EIO" +* VIRQ - virq number +* IPI - IPI vector +* EVTCHN - +*/ +struct irq_info +{ +enum xen_irq_type type;/* type */ +unsigned short evtchn;/* event channel */ +unsigned short cpu;/* cpu bound */ + +union { +unsigned short virq; +enum ipi_vector ipi; +struct { +unsigned short gsi; +unsigned short vector; + } pirq; + } u; +}; + +static struct irq_info irq_info[NR_IRQS ];static int evtchn_to_irq[NR_EVENT_CHANNELS ] = { [0 ...NR_EVENT_CHANNELS -1 ] =-1 };static unsigned long cpu_evtchn_mask[NR_CPUS ][NR_EVENT_CHANNELS /BITS_PER_LONG ]; -static u8 cpu_evtchn[NR_EVENT_CHANNELS ];/* Reference counts for bindings to IRQs. */ static int irq_bindcount[NR_IRQS ];
@@ -88,27 +103,107 @@static struct irq_chip xen_dynamic_chip;/* Constructor for packed IRQ information. */ -static inline struct packed_irq mk_irq_info (u32 type,u32 index,u32 evtchn) +static struct irq_info mk_unbound_info (void ) { -return (struct packed_irq ) { evtchn, index, type }; +return (struct irq_info ) { .type =IRQT_UNBOUND }; +} + +static struct irq_info mk_evtchn_info (unsigned short evtchn) +{ +return (struct irq_info ) { .type =IRQT_EVTCHN , .evtchn = evtchn }; +} + +static struct irq_info mk_ipi_info (unsigned short evtchn,enum ipi_vector ipi) +{ +return (struct irq_info ) { .type =IRQT_IPI , .evtchn = evtchn, + .u.ipi = ipi }; +} + +static struct irq_info mk_virq_info (unsigned short evtchn,unsigned short virq) +{ +return (struct irq_info ) { .type =IRQT_VIRQ , .evtchn = evtchn, + .u.virq = virq }; +} + +static struct irq_info mk_pirq_info (unsigned short evtchn, +unsigned short gsi,unsigned short vector) +{ +return (struct irq_info ) { .type =IRQT_PIRQ , .evtchn = evtchn, + .u.pirq = { .gsi = gsi, .vector = vector } }; }/* * Accessors for packed IRQ information. */ -static inline unsigned int evtchn_from_irq (int irq) +static struct irq_info *info_for_irq (unsigned irq) { -return irq_info[irq].evtchn; +return &irq_info[irq]; } -static inline unsigned int index_from_irq (int irq) +static unsigned int evtchn_from_irq (unsigned irq) { -return irq_info[irq].index; +return info_for_irq (irq)->evtchn; } -static inline unsigned int type_from_irq (int irq) +static enum ipi_vector ipi_from_irq (unsigned irq) { -return irq_info[irq].type; +struct irq_info *info =info_for_irq (irq); + +BUG_ON (info ==NULL ); +BUG_ON (info->type !=IRQT_IPI ); + +return info->u.ipi; +} + +static unsigned virq_from_irq (unsigned irq) +{ +struct irq_info *info =info_for_irq (irq); + +BUG_ON (info ==NULL ); +BUG_ON (info->type !=IRQT_VIRQ ); + +return info->u.virq; +} + +static unsigned gsi_from_irq (unsigned irq) +{ +struct irq_info *info =info_for_irq (irq); + +BUG_ON (info ==NULL ); +BUG_ON (info->type !=IRQT_PIRQ ); + +return info->u.pirq.gsi; +} + +static unsigned vector_from_irq (unsigned irq) +{ +struct irq_info *info =info_for_irq (irq); + +BUG_ON (info ==NULL ); +BUG_ON (info->type !=IRQT_PIRQ ); + +return info->u.pirq.vector; +} + +static enum xen_irq_type type_from_irq (unsigned irq) +{ +return info_for_irq (irq)->type; +} + +static unsigned cpu_from_irq (unsigned irq) +{ +return info_for_irq (irq)->cpu; +} + +static unsigned int cpu_from_evtchn (unsigned int evtchn) +{ +int irq = evtchn_to_irq[evtchn]; +unsigned ret =0 ; + +if (irq !=-1 ) + ret =cpu_from_irq (irq); + +return ret; } static inline unsigned long active_evtchns(unsigned int cpu,
@@ -129,10 +224,10 @@irq_to_desc (irq)->affinity =cpumask_of_cpu (cpu);#endif -__clear_bit (chn, cpu_evtchn_mask[cpu_evtchn[chn]]); +__clear_bit (chn, cpu_evtchn_mask[cpu_from_irq (irq)]);__set_bit (chn, cpu_evtchn_mask[cpu]); - cpu_evtchn[chn] = cpu; + irq_info[irq].cpu = cpu; }static void init_evtchn_cpu_bindings (void )
@@ -146,15 +241,9 @@ desc->affinity =cpumask_of_cpu (0 );#endif -memset (cpu_evtchn,0 ,sizeof (cpu_evtchn));memset (cpu_evtchn_mask[0 ], ~0 ,sizeof (cpu_evtchn_mask[0 ])); } -static inline unsigned int cpu_from_evtchn (unsigned int evtchn) -{ -return cpu_evtchn[evtchn]; -} -static inline void clear_evtchn (int port) {struct shared_info *s = HYPERVISOR_shared_info;
@@ -239,6 +328,8 @@if (irq == nr_irqs)panic ("No available IRQ to bind to: increase nr_irqs!\n" ); +dynamic_irq_init (irq); +return irq; }
@@ -253,12 +344,11 @@if (irq ==-1 ) { irq =find_unbound_irq (); -dynamic_irq_init (irq);set_irq_chip_and_handler_name (irq, &xen_dynamic_chip, handle_level_irq,"event" ); evtchn_to_irq[evtchn] = irq; - irq_info[irq] =mk_irq_info (IRQT_EVTCHN ,0 , evtchn); + irq_info[irq] =mk_evtchn_info (evtchn); } irq_bindcount[irq]++;
@@ -282,7 +372,6 @@if (irq <0 ) goto out; -dynamic_irq_init (irq);set_irq_chip_and_handler_name (irq, &xen_dynamic_chip, handle_level_irq,"ipi" );
@@ -293,7 +382,7 @@ evtchn = bind_ipi.port; evtchn_to_irq[evtchn] = irq; - irq_info[irq] =mk_irq_info (IRQT_IPI , ipi, evtchn); + irq_info[irq] =mk_ipi_info (evtchn, ipi);per_cpu (ipi_to_irq, cpu)[ipi] = irq;
@@ -327,12 +416,11 @@ irq =find_unbound_irq (); -dynamic_irq_init (irq);set_irq_chip_and_handler_name (irq, &xen_dynamic_chip, handle_level_irq,"virq" ); evtchn_to_irq[evtchn] = irq; - irq_info[irq] =mk_irq_info (IRQT_VIRQ , virq, evtchn); + irq_info[irq] =mk_virq_info (evtchn, virq);per_cpu (virq_to_irq, cpu)[virq] = irq;
@@ -361,11 +449,11 @@switch (type_from_irq (irq)) {case IRQT_VIRQ :per_cpu (virq_to_irq,cpu_from_evtchn (evtchn)) - [index_from_irq (irq)] =-1 ; + [virq_from_irq (irq)] =-1 ;break ;case IRQT_IPI :per_cpu (ipi_to_irq,cpu_from_evtchn (evtchn)) - [index_from_irq (irq)] =-1 ; + [ipi_from_irq (irq)] =-1 ;break ;default :break ;
@@ -375,7 +463,7 @@bind_evtchn_to_cpu (evtchn,0 ); evtchn_to_irq[evtchn] =-1 ; - irq_info[irq] =IRQ_UNBOUND ; + irq_info[irq] =mk_unbound_info ();dynamic_irq_cleanup (irq); }
@@ -493,8 +581,8 @@for (i =0 ; i <NR_EVENT_CHANNELS ; i++) {if (sync_test_bit (i, sh->evtchn_pending)) {printk (" %d: event %d -> irq %d\n" , - cpu_evtchn[i], i, - evtchn_to_irq[i]); +cpu_from_evtchn (i), i, + evtchn_to_irq[i]); } }
@@ -592,7 +680,7 @@BUG_ON (irq_bindcount[irq] ==0 ); evtchn_to_irq[evtchn] = irq; - irq_info[irq] =mk_irq_info (IRQT_EVTCHN ,0 , evtchn); + irq_info[irq] =mk_evtchn_info (evtchn);spin_unlock (&irq_mapping_update_lock);
@@ -702,8 +790,7 @@if ((irq =per_cpu (virq_to_irq, cpu)[virq]) ==-1 )continue ; -BUG_ON (irq_info[irq].type !=IRQT_VIRQ ); -BUG_ON (irq_info[irq].index != virq); +BUG_ON (virq_from_irq (irq) != virq);/* Get a new binding from Xen. */ bind_virq.virq = virq;
@@ -715,7 +802,7 @@/* Record the new mapping. */ evtchn_to_irq[evtchn] = irq; - irq_info[irq] =mk_irq_info (IRQT_VIRQ , virq, evtchn); + irq_info[irq] =mk_virq_info (evtchn, virq);bind_evtchn_to_cpu (evtchn, cpu);/* Ready for use. */
@@ -732,8 +819,7 @@if ((irq =per_cpu (ipi_to_irq, cpu)[ipi]) ==-1 )continue ; -BUG_ON (irq_info[irq].type !=IRQT_IPI ); -BUG_ON (irq_info[irq].index != ipi); +BUG_ON (ipi_from_irq (irq) != ipi);/* Get a new binding from Xen. */ bind_ipi.vcpu = cpu;
@@ -744,7 +830,7 @@/* Record the new mapping. */ evtchn_to_irq[evtchn] = irq; - irq_info[irq] =mk_irq_info (IRQT_IPI , ipi, evtchn); + irq_info[irq] =mk_ipi_info (evtchn, ipi);bind_evtchn_to_cpu (evtchn, cpu);/* Ready for use. */
PATCH 1/1 xen: remove irq bindcount
In reply to Jeremy FitzhardingeThere should be no need for us to maintain our own bind count for irqs, since the surrounding irq system should keep track of shared irqs for us.
--- drivers/xen/events.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-)
drivers/xen/events.c +7 -19
@@ -53,7 +53,7 @@/* Interrupt types. */ enum xen_irq_type { -IRQT_UNBOUND , +IRQT_UNBOUND =0 ,IRQT_PIRQ ,IRQT_VIRQ ,IRQT_IPI ,
@@ -94,9 +94,6 @@ };static unsigned long cpu_evtchn_mask[NR_CPUS ][NR_EVENT_CHANNELS /BITS_PER_LONG ]; -/* Reference counts for bindings to IRQs. */ -static int irq_bindcount[NR_IRQS ]; -/* Xen will never allocate port zero for any purpose. */ #define VALID_EVTCHN (chn) ((chn) != 0)
@@ -320,9 +317,8 @@ {int irq; -/* Only allocate from dynirq range */ for_each_irq_nr (irq) -if (irq_bindcount[irq] ==0 ) +if (irq_info[irq].type ==IRQT_UNBOUND )break ;if (irq == nr_irqs)
@@ -351,8 +347,6 @@ irq_info[irq] =mk_evtchn_info (evtchn); } - irq_bindcount[irq]++; -spin_unlock (&irq_mapping_update_lock);return irq;
@@ -389,8 +383,6 @@bind_evtchn_to_cpu (evtchn, cpu); } - irq_bindcount[irq]++; - out:spin_unlock (&irq_mapping_update_lock);return irq;
@@ -427,8 +419,6 @@bind_evtchn_to_cpu (evtchn, cpu); } - irq_bindcount[irq]++; -spin_unlock (&irq_mapping_update_lock);return irq;
@@ -441,7 +431,7 @@spin_lock (&irq_mapping_update_lock); -if ((--irq_bindcount[irq] ==0 ) &&VALID_EVTCHN (evtchn)) { +if (VALID_EVTCHN (evtchn)) { close.port = evtchn;if (HYPERVISOR_event_channel_op (EVTCHNOP_close, &close) !=0 )BUG ();
@@ -667,6 +657,8 @@/* Rebind a new event channel to an existing irq. */ void rebind_evtchn_irq (int evtchn,int irq) { +struct irq_info *info =info_for_irq (irq); +/* Make sure the irq is masked, since the new event channel will also be masked. */ disable_irq (irq);
@@ -676,8 +668,8 @@/* After resume the irq<->evtchn mappings are all cleared out */ BUG_ON (evtchn_to_irq[evtchn] !=-1 );/* Expect irq to have been bound before, -so the bindcount should be non-0 */ -BUG_ON (irq_bindcount[irq] ==0 ); +so there should be a proper type */ +BUG_ON (info->type !=IRQT_UNBOUND ); evtchn_to_irq[evtchn] = irq; irq_info[irq] =mk_evtchn_info (evtchn);
@@ -930,9 +922,5 @@for (i =0 ; i <NR_EVENT_CHANNELS ; i++)mask_evtchn (i); -/* Dynamic IRQ space is currently unbound. Zero the refcnts. */ -for_each_irq_nr (i) - irq_bindcount[i] =0 ; -irq_ctx_init (smp_processor_id ()); }
PATCH 1/1 xen: implement pirq type event channels
In reply to Jeremy FitzhardingeA privileged PV Xen domain can get direct access to hardware. In order for this to be useful, it must be able to get hardware interrupts.
Being a PV Xen domain, all interrupts are delivered as event channels. PIRQ event channels are bound to a pirq number and an interrupt vector. When a IO APIC raises a hardware interrupt on that vector, it is delivered as an event channel, which we can deliver to the appropriate device driver(s).
This patch simply implements the infrastructure for dealing with pirq event channels.
--- drivers/xen/events.c | 250 +++++++++++++++++++++++++++++++++++++++++++++++++- include/xen/events.h | 11 ++ 2 files changed, 258 insertions(+), 3 deletions(-)
drivers/xen/events.c +247 -3
@@ -16,7 +16,7 @@* (typically dom0). * 2. VIRQs, typically used for timers. These are per-cpu events. * 3. IPIs. -* 4. Hardware interrupts. Not supported at present. +* 4. PIRQs - Hardware interrupts. * * Jeremy Fitzhardinge <[email protected]>, XenSource Inc, 2007 */
@@ -39,6 +39,9 @@#include <xen/interface/xen.h> #include <xen/interface/event_channel.h> +/* Leave low irqs free for identity mapping */ +#define LEGACY_IRQS 16 +/* * This lock protects updates to the following mapping and reference-count * arrays. The lock does not need to be acquired to read the mapping tables.
@@ -82,10 +85,12 @@enum ipi_vector ipi;struct {unsigned short gsi; -unsigned short vector; +unsigned char vector; +unsigned char flags; } pirq; } u; }; +#define PIRQ_NEEDS_EOI (1 << 0)static struct irq_info irq_info[NR_IRQS ];
@@ -98,6 +103,7 @@#define VALID_EVTCHN (chn) ((chn) != 0)static struct irq_chip xen_dynamic_chip; +static struct irq_chip xen_pirq_chip;/* Constructor for packed IRQ information. */ static struct irq_info mk_unbound_info (void )
@@ -203,6 +209,15 @@return ret; } +static bool pirq_needs_eoi (unsigned irq) +{ +struct irq_info *info =info_for_irq (irq); + +BUG_ON (info->type !=IRQT_PIRQ ); + +return info->u.pirq.flags &PIRQ_NEEDS_EOI ; +} +static inline unsigned long active_evtchns (unsigned int cpu,struct shared_info *sh,unsigned int idx)
@@ -317,9 +332,12 @@ {int irq; -for_each_irq_nr (irq) +for_each_irq_nr (irq) { +if (irq <LEGACY_IRQS ) +continue ;if (irq_info[irq].type ==IRQT_UNBOUND )break ; + }if (irq == nr_irqs)panic ("No available IRQ to bind to: increase nr_irqs!\n" );
@@ -329,6 +347,212 @@return irq; } +static bool identity_mapped_irq (unsigned irq) +{ +/* only identity map legacy irqs */ +return irq <LEGACY_IRQS ; +} + +static void pirq_unmask_notify (int irq) +{ +struct physdev_eoi eoi = { .irq = irq }; + +if (unlikely (pirq_needs_eoi (irq))) { +int rc =HYPERVISOR_physdev_op (PHYSDEVOP_eoi, &eoi); +WARN_ON (rc); + } +} + +static void pirq_query_unmask (int irq) +{ +struct physdev_irq_status_query irq_status; +struct irq_info *info =info_for_irq (irq); + +BUG_ON (info->type !=IRQT_PIRQ ); + + irq_status.irq = irq; +if (HYPERVISOR_physdev_op (PHYSDEVOP_irq_status_query, &irq_status)) + irq_status.flags =0 ; + + info->u.pirq.flags &= ~PIRQ_NEEDS_EOI ; +if (irq_status.flags & XENIRQSTAT_needs_eoi) + info->u.pirq.flags |=PIRQ_NEEDS_EOI ; +} + +static bool probing_irq (int irq) +{ +struct irq_desc *desc =irq_to_desc (irq); + +return desc && desc->action ==NULL ; +} + +static unsigned int startup_pirq (unsigned int irq) +{ +struct evtchn_bind_pirq bind_pirq; +struct irq_info *info =info_for_irq (irq); +int evtchn =evtchn_from_irq (irq); + +BUG_ON (info->type !=IRQT_PIRQ ); + +if (VALID_EVTCHN (evtchn)) + goto out; + + bind_pirq.pirq = irq; +/* NB. We are happy to share unless we are probing. */ + bind_pirq.flags =probing_irq (irq) ?0 :BIND_PIRQ__WILL_SHARE ; +if (HYPERVISOR_event_channel_op (EVTCHNOP_bind_pirq, &bind_pirq) !=0 ) { +if (!probing_irq (irq)) +printk (KERN_INFO "Failed to obtain physical IRQ %d\n" , + irq); +return 0 ; + } + evtchn = bind_pirq.port; + +pirq_query_unmask (irq); + + evtchn_to_irq[evtchn] = irq; +bind_evtchn_to_cpu (evtchn,0 ); + info->evtchn = evtchn; + + out: +unmask_evtchn (evtchn); +pirq_unmask_notify (irq); + +return 0 ; +} + +static void shutdown_pirq (unsigned int irq) +{ +struct evtchn_close close; +struct irq_info *info =info_for_irq (irq); +int evtchn =evtchn_from_irq (irq); + +BUG_ON (info->type !=IRQT_PIRQ ); + +if (!VALID_EVTCHN (evtchn)) +return ; + +mask_evtchn (evtchn); + + close.port = evtchn; +if (HYPERVISOR_event_channel_op (EVTCHNOP_close, &close) !=0 ) +BUG (); + +bind_evtchn_to_cpu (evtchn,0 ); + evtchn_to_irq[evtchn] =-1 ; + info->evtchn =0 ; +} + +static void enable_pirq (unsigned int irq) +{ +startup_pirq (irq); +} + +static void disable_pirq (unsigned int irq) +{ +} + +static void ack_pirq (unsigned int irq) +{ +int evtchn =evtchn_from_irq (irq); + +move_native_irq (irq); + +if (VALID_EVTCHN (evtchn)) { +mask_evtchn (evtchn); +clear_evtchn (evtchn); + } +} + +static void end_pirq (unsigned int irq) +{ +int evtchn =evtchn_from_irq (irq); +struct irq_desc *desc =irq_to_desc (irq); + +if (WARN_ON (!desc)) +return ; + +if ((desc->status & (IRQ_DISABLED |IRQ_PENDING )) == + (IRQ_DISABLED |IRQ_PENDING )) { +shutdown_pirq (irq); + }else if (VALID_EVTCHN (evtchn)) { +unmask_evtchn (evtchn); +pirq_unmask_notify (irq); + } +} + +static int find_irq_by_gsi (unsigned gsi) +{ +int irq; + +for (irq =0 ; irq <NR_IRQS ; irq++) { +struct irq_info *info =info_for_irq (irq); + +if (info ==NULL || info->type !=IRQT_PIRQ ) +continue ; + +if (gsi_from_irq (irq) == gsi) +return irq; + } + +return -1 ; +} + +/* +* Allocate a physical irq, along with a vector. We don't assign an +* event channel until the irq actually started up. Return an +* existing irq if we've already got one for the gsi. +*/ +int xen_allocate_pirq (unsigned gsi) +{ +int irq; +struct physdev_irq irq_op; + +spin_lock (&irq_mapping_update_lock); + + irq =find_irq_by_gsi (gsi); +if (irq !=-1 ) { +printk (KERN_INFO "xen_allocate_pirq: returning irq %d for gsi %u\n" , + irq, gsi); + goto out;/* XXX need refcount? */ + } + +if (identity_mapped_irq (gsi)) { + irq = gsi; +dynamic_irq_init (irq); + }else + irq =find_unbound_irq (); + +spin_unlock (&irq_mapping_update_lock); + +set_irq_chip_and_handler_name (irq, &xen_pirq_chip, + handle_level_irq,"pirq" ); + + irq_op.irq = irq; +if (HYPERVISOR_physdev_op (PHYSDEVOP_alloc_irq_vector, &irq_op)) { +dynamic_irq_cleanup (irq); + irq = -ENOSPC ; + goto out; + } + + irq_info[irq] =mk_pirq_info (0 , gsi, irq_op.vector); + +out: +spin_unlock (&irq_mapping_update_lock); + +return irq; +} + +int xen_vector_from_irq (unsigned irq) +{ +return vector_from_irq (irq); +} + +int xen_gsi_from_irq (unsigned irq) +{ +return gsi_from_irq (irq); +} +int bind_evtchn_to_irq (unsigned int evtchn) {int irq;
@@ -912,6 +1136,26 @@ .retrigger = retrigger_dynirq, }; +static struct irq_chip xen_pirq_chip__read_mostly = { + .name ="xen-pirq" , + + .startup = startup_pirq, + .shutdown = shutdown_pirq, + + .enable = enable_pirq, + .unmask = enable_pirq, + + .disable = disable_pirq, + .mask = disable_pirq, + + .ack = ack_pirq, + .end = end_pirq, + + .set_affinity = set_affinity_irq, + + .retrigger = retrigger_dynirq, +}; +void __init xen_init_IRQ (void ) {int i;
include/xen/events.h +11 -0
@@ -55,4 +55,15 @@irq will be disabled so it won't deliver an interrupt. */ void xen_poll_irq (int irq); +/* Allocate an irq for a physical interrupt, given a gsi. "Legacy" +GSIs are identity mapped; others are dynamically allocated as +usual. */ +int xen_allocate_pirq (unsigned gsi); + +/* Return vector allocated to pirq */ +int xen_vector_from_irq (unsigned pirq); + +/* Return gsi allocated to pirq */ +int xen_gsi_from_irq (unsigned pirq); +#endif /* _XEN_EVENTS_H */
PATCH 1/1 xen: route hardware irqs via Xen
In reply to Jeremy FitzhardingeThis patch puts the hooks into place so that when the interrupt subsystem registers an irq, it gets routed via Xen (if we're running under Xen).
The first step is to get a gsi for a particular device+pin. We use the normal acpi interrupt routing to do the mapping.
Normally the gsi number is used directly as the irq number. We can't do that since we also have irqs for non-hardware event channels, and so we must share the irq space between them. A given gsi is only allocated a single irq, so re-registering a gsi will simply return the same irq.
We therefore allocate an irq for a given gsi, and return that. As a special case, we reserve the first 16 irqs for identity-mapping legacy irqs, since there's a fair amount of code which assumes that.
Having allocated an irq, we ask Xen to allocate a vector, and then bind that pirq/vector to an event channel. When the hardware raises an interrupt on a vector, Xen signals us on the corresponding event channel, which gets routed to the irq and delivered to the appropriate device driver.
This patch does everything except set up the IO APIC pin routing to the vector.
--- arch/x86/kernel/acpi/boot.c | 8 +++ arch/x86/pci/legacy.c | 4 + arch/x86/xen/Makefile | 1 arch/x86/xen/pci.c | 98 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/xen/xen-ops.h | 1 drivers/xen/events.c | 9 ++- include/asm-x86/xen/pci.h | 7 +++ include/xen/events.h | 8 +++ 8 files changed, 132 insertions(+), 4 deletions(-)
arch/x86/kernel/acpi/boot.c +8 -0
@@ -42,6 +42,8 @@#include <asm/mpspec.h> #include <asm/smp.h> +#include <asm/xen/pci.h> +#include <asm/xen/hypervisor.h> #ifdef CONFIG_X86_LOCAL_APIC
@@ -501,6 +503,12 @@unsigned int irq;unsigned int plat_gsi = gsi; +#ifdef CONFIG_XEN_PCI + irq =xen_register_gsi (gsi, triggering, polarity); +if ((int )irq >=0 ) +return irq; +#endif +#ifdef CONFIG_PCI /* * Make sure all (legacy) PCI IRQs are set as level-triggered.
arch/x86/pci/legacy.c +4 -0
@@ -3,6 +3,7 @@*/ #include <linux/init.h> #include <linux/pci.h> +#include <asm/xen/pci.h> #include "pci.h" /*
@@ -66,6 +67,9 @@#ifdef CONFIG_X86_VISWS pci_visws_init ();#endif +#ifdef CONFIG_XEN_PCI +xen_pci_init (); +#endif pci_legacy_init ();pcibios_irq_init ();pcibios_init ();
arch/x86/xen/Makefile +1 -0
@@ -11,3 +11,4 @@ obj-$(CONFIG_SMP) += smp.o spinlock.o obj-$(CONFIG_XEN_DEBUG_FS) += debugfs.o obj-$(CONFIG_XEN_DOM0) += apic.o +obj-$(CONFIG_XEN_PCI) += pci.o
arch/x86/xen/pci.c new file +98 -0
98 lines not on this page · raw message
arch/x86/xen/xen-ops.h +0 -1
@@ -63,7 +63,6 @@static inline void xen_smp_init (void ) {}#endif -void xen_init_apic (void );/* Declare an asm function, along with symbols needed to make it
drivers/xen/events.c +6 -3
@@ -391,6 +391,7 @@struct evtchn_bind_pirq bind_pirq;struct irq_info *info =info_for_irq (irq);int evtchn =evtchn_from_irq (irq); +int rc;BUG_ON (info->type !=IRQT_PIRQ );
@@ -400,10 +401,12 @@ bind_pirq.pirq = irq;/* NB. We are happy to share unless we are probing. */ bind_pirq.flags =probing_irq (irq) ?0 :BIND_PIRQ__WILL_SHARE ; -if (HYPERVISOR_event_channel_op (EVTCHNOP_bind_pirq, &bind_pirq) !=0 ) { + rc =HYPERVISOR_event_channel_op (EVTCHNOP_bind_pirq, &bind_pirq); +if (rc !=0 ) {if (!probing_irq (irq))printk (KERN_INFO "Failed to obtain physical IRQ %d\n" , irq); +dump_stack ();return 0 ; } evtchn = bind_pirq.port;
@@ -523,8 +526,6 @@ }else irq =find_unbound_irq (); -spin_unlock (&irq_mapping_update_lock); -set_irq_chip_and_handler_name (irq, &xen_pirq_chip, handle_level_irq,"pirq" );
@@ -1167,4 +1168,6 @@mask_evtchn (i);irq_ctx_init (smp_processor_id ()); + +xen_setup_pirqs (); }
include/asm-x86/xen/pci.h new file +7 -0
@@ -0,0 +1,7 @@ +#ifndef _ASM_X86_XEN_PCI_H +#define _ASM_X86_XEN_PCI_H + +void xen_pci_init (void ); +int xen_register_gsi (u32 gsi,int triggering,int polarity); + +#endif /* _ASM_X86_XEN_PCI_H */
include/xen/events.h +8 -0
@@ -66,4 +66,12 @@/* Return gsi allocated to pirq */ int xen_gsi_from_irq (unsigned pirq); +#ifdef CONFIG_XEN_PCI +void xen_setup_pirqs (void ); +#else +static inline void xen_setup_pirqs (void ) +{ +} +#endif +#endif /* _XEN_EVENTS_H */
Re: [PATCH 36 of 38] xen: route hardware irqs via Xen
arch/x86/kernel/acpi/boot.c line 510
Quoted 6 lines
arch/x86/kernel/acpi/boot.c +5
-0+#ifdef CONFIG_XEN_PCI + irq =xen_register_gsi (gsi, triggering, polarity); +if ((int )irq >=0 ) +return irq; +#endif
why not change irq to 'int' and avoid the cast?
also, please eliminate the #ifdef by turning xen_register_gsi() into a 'return -1' inline on !CONFIG_XEN_PCI.
arch/x86/pci/legacy.c line 72
Quoted 3 lines
arch/x86/pci/legacy.c +3
-0+#ifdef CONFIG_XEN_PCI +xen_pci_init (); +#endif
hide the #ifdef in a header please. (like you already properly do for xen_setup_pirqs())
drivers/xen/events.c line 409
Quoted 5 lines
drivers/xen/events.c +2
-0+if (rc !=0 ) {if (!probing_irq (irq))printk (KERN_INFO "Failed to obtain physical IRQ %d\n" , irq); +dump_stack ();
generally it's better to use WARN() or WARN_ONCE() to get good debug feedback and stackdumps. (they also document the reason for the dump)
drivers/xen/events.c line 530
Quoted 8 lines
drivers/xen/events.c +0
-2@@ -523,8 +526,6 @@ }else irq =find_unbound_irq (); -spin_unlock (&irq_mapping_update_lock); -set_irq_chip_and_handler_name (irq, &xen_pirq_chip, handle_level_irq,"pirq" );
hm, looks like a stray bugfix?
Ingo
Re: [PATCH 36 of 38] xen: route hardware irqs via Xen
Quoted 9 lines · Ingo Molnar
arch/x86/kernel/acpi/boot.c line 510
Quoted 6 lines
arch/x86/kernel/acpi/boot.c +5
-0+#ifdef CONFIG_XEN_PCI + irq =xen_register_gsi (gsi, triggering, polarity); +if ((int )irq >=0 ) +return irq; +#endif why not change irq to 'int' and avoid the cast?
Yeah, OK.
Quoted 2 lines
also, please eliminate the #ifdef by turning
xen_register_gsi()into a 'return -1' inline on !CONFIG_XEN_PCI.
OK.
Quoted 8 lines
arch/x86/pci/legacy.c line 72
Quoted 4 lines
arch/x86/pci/legacy.c +3
-0+#ifdef CONFIG_XEN_PCI +xen_pci_init (); +#endif hide the #ifdef in a header please. (like you already properly do for
xen_setup_pirqs())
OK.
Quoted 9 lines
drivers/xen/events.c line 409
Quoted 6 lines
drivers/xen/events.c +2
-0+if (rc !=0 ) {if (!probing_irq (irq))printk (KERN_INFO "Failed to obtain physical IRQ %d\n" , irq); +dump_stack ();generally it's better to use
WARN()orWARN_ONCE()to get good debug feedback and stackdumps. (they also document the reason for the dump)
That was really just a temp debug hack; it shouldn't be dumping stack for probes anyway.
Quoted 4 lines
Quoted 4 lines
drivers/xen/events.c +0
-0@@ -523,8 +526,6 @@ }else irq =find_unbound_irq ();
Quoted 8 lines
drivers/xen/events.c line 531
Quoted 5 lines
drivers/xen/events.c +0
-2-spin_unlock (&irq_mapping_update_lock); -set_irq_chip_and_handler_name (irq, &xen_pirq_chip, handle_level_irq,"pirq" );hm, looks like a stray bugfix?
Erm, yep.
J
Re: [PATCH 36 of 38] xen: route hardware irqs via Xen
In reply to Jeremy FitzhardingeQuoted 38 lines · Jeremy Fitzhardinge
This patch puts the hooks into place so that when the interrupt subsystem registers an irq, it gets routed via Xen (if we're running under Xen).
The first step is to get a gsi for a particular device+pin. We use the normal acpi interrupt routing to do the mapping.
Normally the gsi number is used directly as the irq number. We can't do that since we also have irqs for non-hardware event channels, and so we must share the irq space between them. A given gsi is only allocated a single irq, so re-registering a gsi will simply return the same irq.
We therefore allocate an irq for a given gsi, and return that. As a special case, we reserve the first 16 irqs for identity-mapping legacy irqs, since there's a fair amount of code which assumes that.
Having allocated an irq, we ask Xen to allocate a vector, and then bind that pirq/vector to an event channel. When the hardware raises an interrupt on a vector, Xen signals us on the corresponding event channel, which gets routed to the irq and delivered to the appropriate device driver.
This patch does everything except set up the IO APIC pin routing to the vector.
Signed-off-by Jeremy Fitzhardinge <[email protected]>--- arch/x86/kernel/acpi/boot.c | 8 +++ arch/x86/pci/legacy.c | 4 + arch/x86/xen/Makefile | 1 arch/x86/xen/pci.c | 98 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/xen/xen-ops.h | 1 drivers/xen/events.c | 9 ++- include/asm-x86/xen/pci.h | 7 +++ include/xen/events.h | 8 +++ 8 files changed, 132 insertions(+), 4 deletions(-)
[snip]
arch/x86/xen/xen-ops.h line 68
Quoted 11 lines
arch/x86/xen/xen-ops.h +0
-1@@ -63,7 +63,6 @@static inline void xen_smp_init (void ) {}#endif -void xen_init_apic (void );/* Declare an asm function, along with symbols needed to make it
Hi Jeremy,
This seems like a spurious whitespace change that could be merged into "[PATCH 30 of 38] xen: implement io_apic_ops"
[snip]
Signature · 3 lines
Simon Horman VA Linux Systems Japan K.K., Sydney, Australia Satellite Office H: www.vergenet.net/~horms/ W: www.valinux.co.jp/en
PATCH 1/1 xen: bind pirq to vector and event channel
In reply to Jeremy FitzhardingeHaving converting a dev+pin to a gsi, and that gsi to an irq, and allocated a vector for the irq, we must program the IO APIC to deliver an interrupt on a pin to the vector, so Xen can deliver it as an event channel.
Given the pirq, we can get the gsi and vector. We map the gsi to a specific IO APIC's pin, and set the routing entry.
--- arch/x86/xen/apic.c | 4 ++-- arch/x86/xen/pci.c | 34 +++++++++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 5 deletions(-)
arch/x86/xen/apic.c +2 -2
@@ -4,6 +4,7 @@#include <asm/io_apic.h> #include <asm/acpi.h> +#include <asm/hw_irq.h> #include <asm/xen/hypervisor.h> #include <asm/xen/hypercall.h>
@@ -13,8 +14,7 @@static void __init xen_io_apic_init (void ) { -printk ("xen apic init\n" ); -dump_stack (); +enable_IO_APIC (); }static unsigned int xen_io_apic_read (unsigned apic,unsigned reg)
arch/x86/xen/pci.c +31 -3
@@ -2,8 +2,10 @@#include <linux/acpi.h> #include <linux/pci.h> +#include <asm/mpspec.h> +#include <asm/io_apic.h> +#include <asm/xen/hypervisor.h> -#include <xen/interface/xen.h> #include <xen/events.h>
@@ -11,6 +13,31 @@#include "xen-ops.h" +static void xen_set_io_apic_routing (int irq,int trigger,int polarity) +{ +int ioapic, ioapic_pin; +int vector, gsi; +struct IO_APIC_route_entry entry; + + gsi =xen_gsi_from_irq (irq); + vector =xen_vector_from_irq (irq); + + ioapic =mp_find_ioapic (gsi); +if (ioapic ==-1 ) { +printk (KERN_WARNING "xen_set_ioapic_routing: irq %d gsi %d ioapic %d\n" , + irq, gsi, ioapic); +return ; + } + + ioapic_pin =mp_find_ioapic_pin (ioapic, gsi); + +printk (KERN_INFO "xen_set_ioapic_routing: irq %d gsi %d vector %d ioapic %d pin %d triggering %d polarity %d\n" , + irq, gsi, vector, ioapic, ioapic_pin, trigger, polarity); + +setup_ioapic_entry (ioapic,-1 , &entry, ~0 , trigger, polarity, vector); +ioapic_write_entry (ioapic, ioapic_pin, entry); +} +int xen_register_gsi (u32 gsi,int triggering,int polarity) {int irq;
@@ -25,6 +52,9 @@printk (KERN_DEBUG "xen: --> irq=%d\n" , irq); +if (irq >0 ) +xen_set_io_apic_routing (irq, triggering, polarity); +return irq; }
@@ -45,8 +75,6 @@printk (KERN_INFO "xen: PCI device %s pin %d -> irq %d\n" ,pci_name (dev), dev->pin, irq); - -/* install vector in ioapic? */ }else printk (KERN_INFO "xen: irq enable for %s failed: rc=%d pin=%d irq=%d\n" ,pci_name (dev), rc, dev->pin, dev->irq);
PATCH 1/1 xen: make sure that softirqs get handled at the end of event processing
In reply to Jeremy FitzhardingeMake sure that irq_enter()/irq_exit() wrap the entire event processing loop, rather than each individual event invokation. This make sure that softirq processing is deferred until the of event processing, rather than in the middle with interrupts disabled.
--- drivers/xen/events.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-)
drivers/xen/events.c +9 -20
@@ -806,25 +806,6 @@return IRQ_HANDLED ; } - -static void xen_do_irq (unsigned irq,struct pt_regs *regs) -{ -struct pt_regs *old_regs =set_irq_regs (regs); - -if (WARN_ON (irq ==-1 )) -return ; - -exit_idle (); -irq_enter (); - -//printk("cpu %d handling irq %d\n", smp_processor_id(), info->irq); -handle_irq (irq, regs); - -irq_exit (); - -set_irq_regs (old_regs); -} -/* * Search the CPUs pending events bitmasks. For each one found, map * the event number to an irq, and feed it into do_IRQ() for
@@ -837,11 +818,15 @@void xen_evtchn_do_upcall (struct pt_regs *regs) {int cpu =get_cpu (); +struct pt_regs *old_regs =set_irq_regs (regs);struct shared_info *s = HYPERVISOR_shared_info;struct vcpu_info *vcpu_info =__get_cpu_var (xen_vcpu); static DEFINE_PER_CPU(unsigned, nesting_count);unsigned count; +exit_idle (); +irq_enter (); + do { unsigned long pending_words;
@@ -865,7 +850,8 @@int port = (word_idx *BITS_PER_LONG ) + bit_idx;int irq = evtchn_to_irq[port]; -xen_do_irq (irq, regs); +if (irq !=-1 ) +handle_irq (irq, regs); } }
@@ -876,6 +862,9 @@ }while (count !=1 ); out: +irq_exit (); +set_irq_regs (old_regs); +put_cpu (); }
We are working on the MSI support now. I'd get some guide on how the pirq is defined in pv dom0, can I assume it is something like xen interrupt controller's gsi?
Thanks
Yunhong Jiang
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Jeremy Fitzhardinge
Sent: 2008年11月14日 3:10
To: Ingo Molnar
Cc: the arch/x86 maintainers; Xen-devel; [email protected]; Ian Campbell
Subject: [Xen-devel] [PATCH 00 of 38] xen: add more Xen dom0 support
TODO: work out how to fit MSI into all this.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Quoted 1 line · Yunhong
We are working on the MSI support now. I'd get some guide on how the pirq is defined in pv dom0, can I assume it is something like xen interrupt controller's gsi?
The pirq we pass to Xen is the Linux irq we allocate for the interrupt source. For compatibility with native, we maintain an irq == gsi identity, so the Xen pirq will also equal the gsi. However, there's no problem in allocating irqs above the highest gsi for msi use; you could either adapt xen_allocate_pirq or add an msi variant. I think the first is preferred if possible; you'd just use "gsi" to refer to msis too, if they have easily distinguished identifiers.
Are you thinking of putting a Xen hook in setup_msi_irq?
Thanks,
J
Quoted 16 lines
-----Original Message-----
From: Jeremy Fitzhardinge [mailto:[email protected]]
Sent: 2009年3月24日 0:28
To: Jiang, Yunhong
Cc: Xen-devel
Subject: Re: [Xen-devel] [PATCH 00 of 38] xen: add more Xen dom0 supportQuoted 1 line · Yunhong
We are working on the MSI support now. I'd get some guide on
how the pirq is defined in pv dom0, can I assume it is something like xen interrupt controller's gsi?
Quoted 7 lines
The pirq we pass to Xen is the Linux irq we allocate for the interrupt source. For compatibility with native, we maintain an irq == gsi identity, so the Xen pirq will also equal the gsi. However, there's no problem in allocating irqs above the highest gsi for msi use; you could either adapt
xen_allocate_pirqor add an msi variant. I think the first is preferred if possible; you'd just use "gsi" to refer to msis too, if they have easily distinguished identifiers.
I think both should be ok for dom0. Should the driver domain support be considered also now?
Quoted 2 lines
Are you thinking of putting a Xen hook in
setup_msi_irq?
Yes, exactly, I assume it is acceptable for changes under arch/x86/kernel/, like what changes for IOAPIC, am I right?
Thanks
Yunhong Jiang
Quoted 4 lines
Thanks,
J
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Quoted 2 lines · Yunhong
I think both should be ok for dom0. Should the driver domain support be considered also now?
I've only given them secondary consideration; keep them in mind, don't do anything with precludes them from working, but no explicit effort to make it work. PCI Passthrough is probably more important to keep in mind (though there'd be a fair amount of overlap, I'd assume).
Quoted 5 lines
Quoted 2 lines
Are you thinking of putting a Xen hook in
setup_msi_irq?Yes, exactly, I assume it is acceptable for changes under
arch/x86/kernel/, like what changes for IOAPIC, am I right?
Yes, it should be OK.
J