Re: [Xen-ia64-devel] IOSAPIC virtualisation

[Alex Williamson <alex.williamson@xxxxxx>]

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.

> +#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.

   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.


> +       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.

-- 
Alex Williamson                             HP Linux & Open Source Lab


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