| Message ID | 20170511175340.8448-12-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andre, On 11/05/17 18:53, Andre Przywara wrote: > The target CPU for an LPI is encoded in the interrupt translation table > entry, so can't be easily derived from just an LPI number (short of > walking *all* tables and find the matching LPI). > To avoid this in case we need to know the VCPU (for the INVALL command, > for instance), put the VCPU ID in the struct pending_irq, so that it is > easily accessible. > We use the remaining 8 bits of padding space for that to avoid enlarging > the size of struct pending_irq. The number of VCPUs is limited to 127 > at the moment anyway, which we also confirm with a BUILD_BUG_ON. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/vgic.c | 3 +++ > xen/include/asm-arm/vgic.h | 1 + > 2 files changed, 4 insertions(+) > > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > index 27d6b51..97a2cf2 100644 > --- a/xen/arch/arm/vgic.c > +++ b/xen/arch/arm/vgic.c > @@ -63,6 +63,9 @@ struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned int irq) > > void vgic_init_pending_irq(struct pending_irq *p, unsigned int virq) > { > + /* The lpi_vcpu_id field must be big enough to hold a VCPU ID. */ > + BUILD_BUG_ON(BIT(sizeof(p->lpi_vcpu_id) * 8) < MAX_VIRT_CPUS); > + > INIT_LIST_HEAD(&p->inflight); > INIT_LIST_HEAD(&p->lr_queue); > p->irq = virq; > diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h > index e2111a5..02732db 100644 > --- a/xen/include/asm-arm/vgic.h > +++ b/xen/include/asm-arm/vgic.h > @@ -73,6 +73,7 @@ struct pending_irq > uint8_t lr; > uint8_t priority; > uint8_t lpi_priority; /* Caches the priority if this is an LPI. */ > + uint8_t lpi_vcpu_id; /* The VCPU for an LPI. */ Based on the previous patch (#10), I was expecting to see this new field initialized in vgic_init_pending_irq. > /* inflight is used to append instances of pending_irq to > * vgic.inflight_irqs */ > struct list_head inflight; > Cheers,
On Tue, 16 May 2017, Julien Grall wrote: > Hi Andre, > > On 11/05/17 18:53, Andre Przywara wrote: > > The target CPU for an LPI is encoded in the interrupt translation table > > entry, so can't be easily derived from just an LPI number (short of > > walking *all* tables and find the matching LPI). > > To avoid this in case we need to know the VCPU (for the INVALL command, > > for instance), put the VCPU ID in the struct pending_irq, so that it is > > easily accessible. > > We use the remaining 8 bits of padding space for that to avoid enlarging > > the size of struct pending_irq. The number of VCPUs is limited to 127 > > at the moment anyway, which we also confirm with a BUILD_BUG_ON. > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > --- > > xen/arch/arm/vgic.c | 3 +++ > > xen/include/asm-arm/vgic.h | 1 + > > 2 files changed, 4 insertions(+) > > > > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > > index 27d6b51..97a2cf2 100644 > > --- a/xen/arch/arm/vgic.c > > +++ b/xen/arch/arm/vgic.c > > @@ -63,6 +63,9 @@ struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, > > unsigned int irq) > > > > void vgic_init_pending_irq(struct pending_irq *p, unsigned int virq) > > { > > + /* The lpi_vcpu_id field must be big enough to hold a VCPU ID. */ > > + BUILD_BUG_ON(BIT(sizeof(p->lpi_vcpu_id) * 8) < MAX_VIRT_CPUS); > > + > > INIT_LIST_HEAD(&p->inflight); > > INIT_LIST_HEAD(&p->lr_queue); > > p->irq = virq; > > diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h > > index e2111a5..02732db 100644 > > --- a/xen/include/asm-arm/vgic.h > > +++ b/xen/include/asm-arm/vgic.h > > @@ -73,6 +73,7 @@ struct pending_irq > > uint8_t lr; > > uint8_t priority; > > uint8_t lpi_priority; /* Caches the priority if this is an LPI. > > */ > > + uint8_t lpi_vcpu_id; /* The VCPU for an LPI. */ > > Based on the previous patch (#10), I was expecting to see this new field > initialized in vgic_init_pending_irq. right, it should be initialized to INVALID_VCPU_ID > > /* inflight is used to append instances of pending_irq to > > * vgic.inflight_irqs */ > > struct list_head inflight; > > > > Cheers, > > -- > Julien Grall >
Hi, On 22/05/17 23:15, Stefano Stabellini wrote: > On Tue, 16 May 2017, Julien Grall wrote: >> Hi Andre, >> >> On 11/05/17 18:53, Andre Przywara wrote: >>> The target CPU for an LPI is encoded in the interrupt translation table >>> entry, so can't be easily derived from just an LPI number (short of >>> walking *all* tables and find the matching LPI). >>> To avoid this in case we need to know the VCPU (for the INVALL command, >>> for instance), put the VCPU ID in the struct pending_irq, so that it is >>> easily accessible. >>> We use the remaining 8 bits of padding space for that to avoid enlarging >>> the size of struct pending_irq. The number of VCPUs is limited to 127 >>> at the moment anyway, which we also confirm with a BUILD_BUG_ON. >>> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >>> --- >>> xen/arch/arm/vgic.c | 3 +++ >>> xen/include/asm-arm/vgic.h | 1 + >>> 2 files changed, 4 insertions(+) >>> >>> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c >>> index 27d6b51..97a2cf2 100644 >>> --- a/xen/arch/arm/vgic.c >>> +++ b/xen/arch/arm/vgic.c >>> @@ -63,6 +63,9 @@ struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, >>> unsigned int irq) >>> >>> void vgic_init_pending_irq(struct pending_irq *p, unsigned int virq) >>> { >>> + /* The lpi_vcpu_id field must be big enough to hold a VCPU ID. */ >>> + BUILD_BUG_ON(BIT(sizeof(p->lpi_vcpu_id) * 8) < MAX_VIRT_CPUS); >>> + >>> INIT_LIST_HEAD(&p->inflight); >>> INIT_LIST_HEAD(&p->lr_queue); >>> p->irq = virq; >>> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h >>> index e2111a5..02732db 100644 >>> --- a/xen/include/asm-arm/vgic.h >>> +++ b/xen/include/asm-arm/vgic.h >>> @@ -73,6 +73,7 @@ struct pending_irq >>> uint8_t lr; >>> uint8_t priority; >>> uint8_t lpi_priority; /* Caches the priority if this is an LPI. >>> */ >>> + uint8_t lpi_vcpu_id; /* The VCPU for an LPI. */ >> >> Based on the previous patch (#10), I was expecting to see this new field >> initialized in vgic_init_pending_irq. > > right, it should be initialized to INVALID_VCPU_ID That's right, I fixed that now. Cheers, Andre. >>> /* inflight is used to append instances of pending_irq to >>> * vgic.inflight_irqs */ >>> struct list_head inflight; >>> >> >> Cheers, >> >> -- >> Julien Grall >>
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index 27d6b51..97a2cf2 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -63,6 +63,9 @@ struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned int irq) void vgic_init_pending_irq(struct pending_irq *p, unsigned int virq) { + /* The lpi_vcpu_id field must be big enough to hold a VCPU ID. */ + BUILD_BUG_ON(BIT(sizeof(p->lpi_vcpu_id) * 8) < MAX_VIRT_CPUS); + INIT_LIST_HEAD(&p->inflight); INIT_LIST_HEAD(&p->lr_queue); p->irq = virq; diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h index e2111a5..02732db 100644 --- a/xen/include/asm-arm/vgic.h +++ b/xen/include/asm-arm/vgic.h @@ -73,6 +73,7 @@ struct pending_irq uint8_t lr; uint8_t priority; uint8_t lpi_priority; /* Caches the priority if this is an LPI. */ + uint8_t lpi_vcpu_id; /* The VCPU for an LPI. */ /* inflight is used to append instances of pending_irq to * vgic.inflight_irqs */ struct list_head inflight;
The target CPU for an LPI is encoded in the interrupt translation table entry, so can't be easily derived from just an LPI number (short of walking *all* tables and find the matching LPI). To avoid this in case we need to know the VCPU (for the INVALL command, for instance), put the VCPU ID in the struct pending_irq, so that it is easily accessible. We use the remaining 8 bits of padding space for that to avoid enlarging the size of struct pending_irq. The number of VCPUs is limited to 127 at the moment anyway, which we also confirm with a BUILD_BUG_ON. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- xen/arch/arm/vgic.c | 3 +++ xen/include/asm-arm/vgic.h | 1 + 2 files changed, 4 insertions(+)