RE: [Xen-devel] [PATCH] Make the hypercall PHYSDEVOP_alloc_irq_vector hypercall dummy.

["Zhang, Xiantao" <xiantao.zhang@xxxxxxxxx>]

Jeremy Fitzhardinge wrote:
> On 08/31/09 19:03, Zhang, Xiantao wrote:

>> pin=%d, irq=%d\n" \ + "%s: new_entry=%08x\n" \ + "%s: " f,
>> __FUNCTION__, apic, pin, irq, \ + __FUNCTION__, *(u32 *)&rte, \
>> __FUNCTION__ , ##a ) int ioapic_guest_write(unsigned long physbase,
>> unsigned int reg, u32 val) { - int apic, pin, old_irq = -1, new_irq
>> = -1; - struct IO_APIC_route_entry old_rte = { 0 }, new_rte = { 0 };
>> + int apic, pin, irq, ret, vector, pirq; + struct
>> IO_APIC_route_entry rte = { 0 };   
> 
> Is this just a gratuitious renaming, or does it serve some other
> purpose? 

Just renaming, since I don't think it needs to care about old_entry, so remove 
it and rename new_rte to rte. 

>> unsigned long flags; + struct irq_cfg *cfg; + struct irq_desc *desc;
>> if ( (apic = ioapic_physbase_to_id(physbase)) < 0 ) return apic; @@
>> -2204,7 +2156,7 @@ int ioapic_guest_write(unsigned long phy pin =
>> (reg 
>> - 0x10) >> 1; /* Write first half from guest; second half is target
>> info. */ - *(u32 *)&new_rte = val; + *(u32 *)&rte = val; /* * What
>> about weird destination types? @@ -2214,7 +2166,7 @@ int
>> ioapic_guest_write(unsigned long phy * ExtINT: Ignore? Linux only
>> asserts this at start of day. * For now, print a message and return
>> an error. We can fix up on demand. */ - if ( new_rte.delivery_mode >
>> dest_LowestPrio ) + if ( rte.delivery_mode > dest_LowestPrio ) {
>> printk("ERROR: Attempt to write weird IOAPIC destination mode!\n");
>> printk(" APIC=%d/%d, lo-reg=%x\n", apic, pin, val); @@ -2225,83
>> +2177,65 @@ int ioapic_guest_write(unsigned long phy * The guest does
>> not know physical APIC arrangement (flat vs. cluster). * Apply
>> genapic conventions for this platform. */ - new_rte.delivery_mode =
>> INT_DELIVERY_MODE; - new_rte.dest_mode = INT_DEST_MODE; +
>> rte.delivery_mode = INT_DELIVERY_MODE; + rte.dest_mode =
>> INT_DEST_MODE; + + irq = apic_pin_2_gsi_irq(apic, pin); + if ( irq <
>> 0 ) + return irq; + + desc = irq_to_desc(irq); + cfg =
>> desc->chip_data; + + /* Since PHYSDEVOP_alloc_irq_vector is dummy,
>> rte.vector is the pirq + which corresponds to this ioapic pin,
>> retrieve it for building + pirq and irq mapping. + */ + pirq =
>> rte.vector; + if(pirq < 0 || pirq >= dom0->nr_pirqs) + return
>> -EINVAL; + + if ( desc->action ) + { + WARN_BOGUS_WRITE("Attempt to
>> modify IO-APIC pin for in-use IRQ!\n"); + return 0; + } + + if (
>> cfg->vector <= 0 || cfg->vector > LAST_DYNAMIC_VECTOR ) { + +
>> printk("allocated vector for irq:%d\n", irq); + + vector =
>> assign_irq_vector(irq); + if ( vector < 0 ) + return vector; + +
>> add_pin_to_irq(irq, apic, pin); 
> 
> 
> If this is replacing or disabling a previously set "vector" will it
> properly clean up the old state?  (I don't fully understand how all
> the datastructures relate pirq/vector/apic/pin to each other.)

IMO, dom0 should only cares about pirq(==GSI) and doesn't need to know the ifno 
about vector and specific apic&pin at all. Since Xen also knows GSI info, so 
dom0 and Xen can build irq mapping(xen's irq and dom0's pirq) through it.  
Maybe we can get rid of ioapic in dom0 eventually. 

>> extra_dom0_irqs, extra_domU_irqs = 8; +static unsigned int
>> extra_dom0_irqs = 256, extra_domU_irqs = 32; static void __init
>> parse_extra_guest_irqs(const char *s) { if ( isdigit(*s) ) @@ -253,9
>> +253,11 @@ struct domain *domain_create( d->is_paused_by_controller =
>> 1; atomic_inc(&d->pause_count); - d->nr_pirqs = (nr_irqs_gsi + -
>> (domid ? extra_domU_irqs : - extra_dom0_irqs ?: nr_irqs_gsi)); + if (
>> domid ) + d->nr_pirqs = nr_irqs_gsi + extra_domU_irqs; + else +
>> d->nr_pirqs = nr_irqs_gsi + extra_dom0_irqs;
> 
> 
> Surely there's a better test for this?  We don't want to be
> hard-coding "privileged domain == dom0" into Xen.  What version is
> this patch against anyway?  I don't see this "extra_dom0_irqs" stuff
> in current Xen-unstable.

I'm using xen-unsable.hg's tip. 

>> + d->pirq_to_evtchn = xmalloc_array(u16, d->nr_pirqs); d->pirq_mask =
>> xmalloc_array( unsigned long, BITS_TO_LONGS(d->nr_pirqs));
> 
> Thanks,
>       J


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