Le Jeudi 02 Février 2006 19:16, Alex Williamson a écrit :
> Hi Tristan,
>
> I took another quick skim through and have a few more comments. It's
> looking good, thanks for all your work here!
>
> Alex
>
> On Thu, 2006-02-02 at 15:01 +0100, Tristan Gingold wrote:
> > - *pval = ia64_getreg(_IA64_REG_CR_LID);
> > + *pval = vcpu->vcpu_id * 0x01010000;
>
> A comment describing this magic multiplier might be nice.
Yes.
Currently I put vcpu_id in eid and id.
I will document this for SMP dom0/domU.
> > +#ifdef CONFIG_XEN
> > + xen_iosapic_write (rte, IOSAPIC_RTE_HIGH (rte_index), high32);
> > + xen_iosapic_write (rte, IOSAPIC_RTE_LOW (rte_index), low32);
> > +#else
> > + iosapic_write(addr, IOSAPIC_RTE_HIGH(rte_index), high32);
> > + iosapic_write(addr, IOSAPIC_RTE_LOW(rte_index), low32);
> > +#endif
>
> Should we make these runtime checks (ie. if (running_on_xen))? We
> may end up with other things that prevent transparent
> paravirtualization, but this probably shouldn't.
Transparent paravirtualization is kept in running_on_xen, which does the check
on running_on_xen.
> Another random idea; it might also be cleaner for eventual upstream
> inclusion if the xen code were pushed up into the iosapic_read/write/eoi
> functions. Maybe modify the functions to take an rte instead of the
> addr. Then the running_on_xen or CONFIG_XEN checks could be fairly
> consolidated.
I agree on this. That's the reason why I made the check in
xen_iosapic_write/read. Maybe the functions should be renamed later ?
> > + else {
> > + /* Interrupt is unmasked. */
> > + if (intr->low32 & IOSAPIC_MASK) {
> > + /* And was masked. */
> > + rte->vcpu = current;
> > + rte->vcpu_vec = val & 0xff;
> > + spin_unlock_irqrestore(&iosapic_lock, flags);
> > + unmask_vec (rte->xen_vec);
> > + }
> > + else {
> > + /* Was already unmasked. */
> > + if (rte->vcpu->domain != current->domain) {
> > + spin_unlock_irqrestore(&iosapic_lock,
> > flags);
> > + printf ("Oops: GSI %d is reenabled by
> > another"
> > + " domain\n", gsi);
> > + }
> > + }
> > + }
>
> Is the domain check before unmasking still missing here? Seems like
> this block should be almost a mirror image of the masking block.
Well I saw your comment about this.
Only privilegied domain can do a physdev. So currently only dom0 can handle
interrupts. For me this check is correct and enough.
With drivers domain (which are not here), I think any drivers domain can
enable any interrupt. We may add checks but for me this is future work.
Do you have a specific check in mind ?
Thank you for your comments,
Tristan.
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|
Home