>>> On 23.02.17 at 12:52, <roger....@citrix.com> wrote:
> +struct hvm_hw_vioapic *gsi_vioapic(struct domain *d, unsigned int gsi,
const struct domain *d ?
> +static unsigned int pin_gsi(struct domain *d, struct hvm_hw_vioapic *vioapic,
const for both?
> + unsigned int pin)
> +{
> + struct hvm_hw_vioapic *tmp;
And here.
Also wouldn't this function more naturally (i.e. more generally useful)
simply return the base GSI, leaving it to the caller to add the pin
number?
> @@ -157,21 +210,22 @@ static void vioapic_write_redirent(
> pent->fields.remote_irr = 0;
> else if ( !ent.fields.mask &&
> !ent.fields.remote_irr &&
> - hvm_irq->gsi_assert_count[idx] )
> + hvm_irq->gsi_assert_count[gsi] )
Neither this nor any earlier patch modified the size of this array, afaics.
> -static void vioapic_write_indirect(struct domain *d, uint32_t val)
> +static void vioapic_write_indirect(struct domain *d, unsigned long addr,
> + uint32_t val)
> {
> - struct hvm_hw_vioapic *vioapic = domain_vioapic(d);
> + struct hvm_hw_vioapic *vioapic = addr_vioapic(d, addr);
I think it would be better for the caller to pass the vioapic it
already established (and ASSERT()ed).
> @@ -430,11 +493,14 @@ void vioapic_update_EOI(struct domain *d, u8 vector)
>
> static int vioapic_save(struct domain *d, hvm_domain_context_t *h)
> {
> - struct hvm_hw_vioapic *vioapic = domain_vioapic(d);
> + struct hvm_hw_vioapic *vioapic = domain_vioapic(d, 0);
>
> if ( !has_vioapic(d) )
> return 0;
>
> + if ( d->arch.hvm_domain.nr_vioapics != 1 )
> + return -ENOSYS;
> +
> if ( vioapic->nr_pins != VIOAPIC_NUM_PINS )
> return -ENOSYS;
Perhaps merge the two if()s? Otherwise -EOPNOTSUPP again.
> int vioapic_init(struct domain *d)
> {
> if ( !has_vioapic(d) )
> + {
> + d->arch.hvm_domain.nr_vioapics = 0;
I don't think this is needed - if anything you may want to again
ASSERT() it being zero.
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -1120,7 +1120,7 @@ static int __vlapic_accept_pic_intr(struct vcpu *v)
> if ( !has_vioapic(d) )
> return 0;
>
> - redir0 = domain_vioapic(d)->redirtbl[0];
> + redir0 = domain_vioapic(d, 0)->redirtbl[0];
What if the first IO-APIC has less than 16 pins?
> --- a/xen/arch/x86/hvm/vpt.c
> +++ b/xen/arch/x86/hvm/vpt.c
> @@ -78,7 +78,8 @@ void hvm_set_guest_time(struct vcpu *v, u64 guest_time)
> static int pt_irq_vector(struct periodic_time *pt, enum hvm_intsrc src)
> {
> struct vcpu *v = pt->vcpu;
> - unsigned int gsi, isa_irq;
> + struct hvm_hw_vioapic *vioapic;
> + unsigned int gsi, isa_irq, pin;
>
> if ( pt->source == PTSRC_lapic )
> return pt->irq;
> @@ -91,13 +92,16 @@ static int pt_irq_vector(struct periodic_time *pt, enum
> hvm_intsrc src)
> + (isa_irq & 7));
>
> ASSERT(src == hvm_intsrc_lapic);
> - return domain_vioapic(v->domain)->redirtbl[gsi].fields.vector;
> + vioapic = gsi_vioapic(v->domain, gsi, &pin);
> +
> + return vioapic->redirtbl[pin].fields.vector;
Please don't chance de-referencing NULL here and below.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
