| Message ID | 20170316112030.20419-21-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andre, On 03/16/2017 11:20 AM, Andre Przywara wrote: > The MAPTI commands associates a DeviceID/EventID pair with a LPI/CPU > pair and actually instantiates LPI interrupts. > We connect the already allocated host LPI to this virtual LPI, so that > any triggering IRQ on the host can be quickly forwarded to a guest. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/gic-v3-its.c | 63 ++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/gic-v3-lpi.c | 18 ++++++++++++ > xen/arch/arm/vgic-v3-its.c | 27 +++++++++++++++-- > xen/include/asm-arm/gic_v3_its.h | 6 ++++ > 4 files changed, 112 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c > index 5a2dbec..e2fcf50 100644 > --- a/xen/arch/arm/gic-v3-its.c > +++ b/xen/arch/arm/gic-v3-its.c > @@ -724,6 +724,69 @@ restart: > spin_unlock(&d->arch.vgic.its_devices_lock); > } > > +/* > + * Translates an event for a given guest device ID into the associated host > + * LPI number. This can be used to look up the mapped guest LPI. > + */ > +static uint32_t translate_event(struct domain *d, paddr_t doorbell, > + uint32_t devid, uint32_t eventid) > +{ > + struct rb_node *node; > + struct its_devices *dev; > + uint32_t host_lpi = 0; > + int cmp; > + > + spin_lock(&d->arch.vgic.its_devices_lock); > + node = d->arch.vgic.its_devices.rb_node; > + while (node) > + { > + dev = rb_entry(node, struct its_devices, rbnode); > + cmp = compare_its_guest_devices(dev, doorbell, devid); > + > + if ( !cmp ) > + { > + if ( eventid >= dev->eventids ) > + goto out; > + > + host_lpi = dev->host_lpis[eventid / LPI_BLOCK] + > + (eventid % LPI_BLOCK); > + if ( !is_lpi(host_lpi) ) Hmmm, I don't understand this check. host_lpi should always be an LPI. No? > + host_lpi = 0; > + goto out; > + } > + > + if ( cmp > 0 ) > + node = node->rb_left; > + else > + node = node->rb_right; > + } > + > +out: > + spin_unlock(&d->arch.vgic.its_devices_lock); > + > + return host_lpi; > +} > + > +/* > + * Connects the event ID for an already assigned device to the given VCPU/vLPI > + * pair. The corresponding physical LPI is already mapped on the host side > + * (when assigning the physical device to the guest), so we just connect the > + * target VCPU/vLPI pair to that interrupt to inject it properly if it fires. > + */ > +int gicv3_assign_guest_event(struct domain *d, paddr_t doorbell_address, > + uint32_t devid, uint32_t eventid, It looks like to me that devid is the virtual deviceID. If so, please prefix with 'v', otherwise 'p'. > + struct vcpu *v, uint32_t virt_lpi) > +{ > + uint32_t host_lpi = translate_event(d, doorbell_address, devid, eventid); > + > + if ( !host_lpi ) > + return -ENOENT; > + > + gicv3_lpi_update_host_entry(host_lpi, d->domain_id, v->vcpu_id, virt_lpi); > + > + return 0; > +} > + > /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */ > void gicv3_its_dt_init(const struct dt_device_node *node) > { > diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c > index 994698e..c110ec9 100644 > --- a/xen/arch/arm/gic-v3-lpi.c > +++ b/xen/arch/arm/gic-v3-lpi.c > @@ -153,6 +153,24 @@ void do_LPI(unsigned int lpi) > vgic_vcpu_inject_irq(vcpu, hlpi.virt_lpi); > } > > +int gicv3_lpi_update_host_entry(uint32_t host_lpi, int domain_id, > + unsigned int vcpu_id, uint32_t virt_lpi) > +{ > + union host_lpi *hlpip, hlpi; > + > + host_lpi -= LPI_OFFSET; I would add an ASSERT(host_lpi > LPI_OFFSET); > + > + hlpip = &lpi_data.host_lpis[host_lpi / HOST_LPIS_PER_PAGE][host_lpi % HOST_LPIS_PER_PAGE]; > + > + hlpi.virt_lpi = virt_lpi; > + hlpi.dom_id = domain_id; > + hlpi.vcpu_id = vcpu_id; > + > + write_u64_atomic(&hlpip->data, hlpi.data); > + > + return 0; > +} > + > static int gicv3_lpi_allocate_pendtable(uint64_t *reg) > { > uint64_t val; > diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c > index c26d5d4..600ff69 100644 > --- a/xen/arch/arm/vgic-v3-its.c > +++ b/xen/arch/arm/vgic-v3-its.c > @@ -167,8 +167,8 @@ static bool read_itte(struct virt_its *its, uint32_t devid, uint32_t evid, > } > > #define SKIP_LPI_UPDATE 1 > -bool write_itte(struct virt_its *its, uint32_t devid, uint32_t evid, > - uint32_t collid, uint32_t vlpi, struct vcpu **vcpu) > +static bool write_itte(struct virt_its *its, uint32_t devid, uint32_t evid, > + uint32_t collid, uint32_t vlpi, struct vcpu **vcpu) Please explain in the commit message why you move to static. > { > struct vits_itte *itte; > > @@ -332,6 +332,25 @@ static int its_handle_mapd(struct virt_its *its, uint64_t *cmdptr) > return 0; > } > > +static int its_handle_mapti(struct virt_its *its, uint64_t *cmdptr) > +{ > + uint32_t devid = its_cmd_get_deviceid(cmdptr); > + uint32_t eventid = its_cmd_get_id(cmdptr); > + uint32_t intid = its_cmd_get_physical_id(cmdptr); > + int collid = its_cmd_get_collection(cmdptr); This should be unsigned. > + struct vcpu *vcpu; > + > + if ( its_cmd_get_command(cmdptr) == GITS_CMD_MAPI ) > + intid = eventid; > + > + if ( !write_itte(its, devid, eventid, collid, intid, &vcpu) ) > + return -1; > + > + gicv3_assign_guest_event(its->d, its->doorbell_address, devid, eventid, vcpu, intid); If you have a function returning an error, then you should check it and not ignoring it. Cheers,
Hi, On 24/03/17 14:54, Julien Grall wrote: > Hi Andre, > > On 03/16/2017 11:20 AM, Andre Przywara wrote: >> The MAPTI commands associates a DeviceID/EventID pair with a LPI/CPU >> pair and actually instantiates LPI interrupts. >> We connect the already allocated host LPI to this virtual LPI, so that >> any triggering IRQ on the host can be quickly forwarded to a guest. >> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> --- >> xen/arch/arm/gic-v3-its.c | 63 >> ++++++++++++++++++++++++++++++++++++++++ >> xen/arch/arm/gic-v3-lpi.c | 18 ++++++++++++ >> xen/arch/arm/vgic-v3-its.c | 27 +++++++++++++++-- >> xen/include/asm-arm/gic_v3_its.h | 6 ++++ >> 4 files changed, 112 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c >> index 5a2dbec..e2fcf50 100644 >> --- a/xen/arch/arm/gic-v3-its.c >> +++ b/xen/arch/arm/gic-v3-its.c >> @@ -724,6 +724,69 @@ restart: >> spin_unlock(&d->arch.vgic.its_devices_lock); >> } >> >> +/* >> + * Translates an event for a given guest device ID into the >> associated host >> + * LPI number. This can be used to look up the mapped guest LPI. >> + */ >> +static uint32_t translate_event(struct domain *d, paddr_t doorbell, >> + uint32_t devid, uint32_t eventid) >> +{ >> + struct rb_node *node; >> + struct its_devices *dev; >> + uint32_t host_lpi = 0; >> + int cmp; >> + >> + spin_lock(&d->arch.vgic.its_devices_lock); >> + node = d->arch.vgic.its_devices.rb_node; >> + while (node) >> + { >> + dev = rb_entry(node, struct its_devices, rbnode); >> + cmp = compare_its_guest_devices(dev, doorbell, devid); >> + >> + if ( !cmp ) >> + { >> + if ( eventid >= dev->eventids ) >> + goto out; >> + >> + host_lpi = dev->host_lpis[eventid / LPI_BLOCK] + >> + (eventid % LPI_BLOCK); >> + if ( !is_lpi(host_lpi) ) > > Hmmm, I don't understand this check. host_lpi should always be an LPI. No? Looks like. Dropped in v4. >> + host_lpi = 0; >> + goto out; >> + } >> + >> + if ( cmp > 0 ) >> + node = node->rb_left; >> + else >> + node = node->rb_right; >> + } >> + >> +out: >> + spin_unlock(&d->arch.vgic.its_devices_lock); >> + >> + return host_lpi; >> +} >> + >> +/* >> + * Connects the event ID for an already assigned device to the given >> VCPU/vLPI >> + * pair. The corresponding physical LPI is already mapped on the host >> side >> + * (when assigning the physical device to the guest), so we just >> connect the >> + * target VCPU/vLPI pair to that interrupt to inject it properly if >> it fires. >> + */ >> +int gicv3_assign_guest_event(struct domain *d, paddr_t doorbell_address, >> + uint32_t devid, uint32_t eventid, > > It looks like to me that devid is the virtual deviceID. If so, please > prefix with 'v', otherwise 'p'. Fixed in v4. >> + struct vcpu *v, uint32_t virt_lpi) >> +{ >> + uint32_t host_lpi = translate_event(d, doorbell_address, devid, >> eventid); >> + >> + if ( !host_lpi ) >> + return -ENOENT; >> + >> + gicv3_lpi_update_host_entry(host_lpi, d->domain_id, v->vcpu_id, >> virt_lpi); >> + >> + return 0; >> +} >> + >> /* Scan the DT for any ITS nodes and create a list of host ITSes out >> of it. */ >> void gicv3_its_dt_init(const struct dt_device_node *node) >> { >> diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c >> index 994698e..c110ec9 100644 >> --- a/xen/arch/arm/gic-v3-lpi.c >> +++ b/xen/arch/arm/gic-v3-lpi.c >> @@ -153,6 +153,24 @@ void do_LPI(unsigned int lpi) >> vgic_vcpu_inject_irq(vcpu, hlpi.virt_lpi); >> } >> >> +int gicv3_lpi_update_host_entry(uint32_t host_lpi, int domain_id, >> + unsigned int vcpu_id, uint32_t virt_lpi) >> +{ >> + union host_lpi *hlpip, hlpi; >> + >> + host_lpi -= LPI_OFFSET; > > I would add an ASSERT(host_lpi > LPI_OFFSET); Fixed in v4. >> + >> + hlpip = &lpi_data.host_lpis[host_lpi / >> HOST_LPIS_PER_PAGE][host_lpi % HOST_LPIS_PER_PAGE]; >> + >> + hlpi.virt_lpi = virt_lpi; >> + hlpi.dom_id = domain_id; >> + hlpi.vcpu_id = vcpu_id; >> + >> + write_u64_atomic(&hlpip->data, hlpi.data); >> + >> + return 0; >> +} >> + >> static int gicv3_lpi_allocate_pendtable(uint64_t *reg) >> { >> uint64_t val; >> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c >> index c26d5d4..600ff69 100644 >> --- a/xen/arch/arm/vgic-v3-its.c >> +++ b/xen/arch/arm/vgic-v3-its.c >> @@ -167,8 +167,8 @@ static bool read_itte(struct virt_its *its, >> uint32_t devid, uint32_t evid, >> } >> >> #define SKIP_LPI_UPDATE 1 >> -bool write_itte(struct virt_its *its, uint32_t devid, uint32_t evid, >> - uint32_t collid, uint32_t vlpi, struct vcpu **vcpu) >> +static bool write_itte(struct virt_its *its, uint32_t devid, uint32_t >> evid, >> + uint32_t collid, uint32_t vlpi, struct vcpu >> **vcpu) > > Please explain in the commit message why you move to static. Fixed in v4. > >> { >> struct vits_itte *itte; >> >> @@ -332,6 +332,25 @@ static int its_handle_mapd(struct virt_its *its, >> uint64_t *cmdptr) >> return 0; >> } >> >> +static int its_handle_mapti(struct virt_its *its, uint64_t *cmdptr) >> +{ >> + uint32_t devid = its_cmd_get_deviceid(cmdptr); >> + uint32_t eventid = its_cmd_get_id(cmdptr); >> + uint32_t intid = its_cmd_get_physical_id(cmdptr); >> + int collid = its_cmd_get_collection(cmdptr); > > This should be unsigned. Fixed in v4. >> + struct vcpu *vcpu; >> + >> + if ( its_cmd_get_command(cmdptr) == GITS_CMD_MAPI ) >> + intid = eventid; >> + >> + if ( !write_itte(its, devid, eventid, collid, intid, &vcpu) ) >> + return -1; >> + >> + gicv3_assign_guest_event(its->d, its->doorbell_address, devid, >> eventid, vcpu, intid); > > If you have a function returning an error, then you should check it and > not ignoring it. Fixed in v3. Cheers, Andre.
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c index 5a2dbec..e2fcf50 100644 --- a/xen/arch/arm/gic-v3-its.c +++ b/xen/arch/arm/gic-v3-its.c @@ -724,6 +724,69 @@ restart: spin_unlock(&d->arch.vgic.its_devices_lock); } +/* + * Translates an event for a given guest device ID into the associated host + * LPI number. This can be used to look up the mapped guest LPI. + */ +static uint32_t translate_event(struct domain *d, paddr_t doorbell, + uint32_t devid, uint32_t eventid) +{ + struct rb_node *node; + struct its_devices *dev; + uint32_t host_lpi = 0; + int cmp; + + spin_lock(&d->arch.vgic.its_devices_lock); + node = d->arch.vgic.its_devices.rb_node; + while (node) + { + dev = rb_entry(node, struct its_devices, rbnode); + cmp = compare_its_guest_devices(dev, doorbell, devid); + + if ( !cmp ) + { + if ( eventid >= dev->eventids ) + goto out; + + host_lpi = dev->host_lpis[eventid / LPI_BLOCK] + + (eventid % LPI_BLOCK); + if ( !is_lpi(host_lpi) ) + host_lpi = 0; + goto out; + } + + if ( cmp > 0 ) + node = node->rb_left; + else + node = node->rb_right; + } + +out: + spin_unlock(&d->arch.vgic.its_devices_lock); + + return host_lpi; +} + +/* + * Connects the event ID for an already assigned device to the given VCPU/vLPI + * pair. The corresponding physical LPI is already mapped on the host side + * (when assigning the physical device to the guest), so we just connect the + * target VCPU/vLPI pair to that interrupt to inject it properly if it fires. + */ +int gicv3_assign_guest_event(struct domain *d, paddr_t doorbell_address, + uint32_t devid, uint32_t eventid, + struct vcpu *v, uint32_t virt_lpi) +{ + uint32_t host_lpi = translate_event(d, doorbell_address, devid, eventid); + + if ( !host_lpi ) + return -ENOENT; + + gicv3_lpi_update_host_entry(host_lpi, d->domain_id, v->vcpu_id, virt_lpi); + + return 0; +} + /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */ void gicv3_its_dt_init(const struct dt_device_node *node) { diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c index 994698e..c110ec9 100644 --- a/xen/arch/arm/gic-v3-lpi.c +++ b/xen/arch/arm/gic-v3-lpi.c @@ -153,6 +153,24 @@ void do_LPI(unsigned int lpi) vgic_vcpu_inject_irq(vcpu, hlpi.virt_lpi); } +int gicv3_lpi_update_host_entry(uint32_t host_lpi, int domain_id, + unsigned int vcpu_id, uint32_t virt_lpi) +{ + union host_lpi *hlpip, hlpi; + + host_lpi -= LPI_OFFSET; + + hlpip = &lpi_data.host_lpis[host_lpi / HOST_LPIS_PER_PAGE][host_lpi % HOST_LPIS_PER_PAGE]; + + hlpi.virt_lpi = virt_lpi; + hlpi.dom_id = domain_id; + hlpi.vcpu_id = vcpu_id; + + write_u64_atomic(&hlpip->data, hlpi.data); + + return 0; +} + static int gicv3_lpi_allocate_pendtable(uint64_t *reg) { uint64_t val; diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c index c26d5d4..600ff69 100644 --- a/xen/arch/arm/vgic-v3-its.c +++ b/xen/arch/arm/vgic-v3-its.c @@ -167,8 +167,8 @@ static bool read_itte(struct virt_its *its, uint32_t devid, uint32_t evid, } #define SKIP_LPI_UPDATE 1 -bool write_itte(struct virt_its *its, uint32_t devid, uint32_t evid, - uint32_t collid, uint32_t vlpi, struct vcpu **vcpu) +static bool write_itte(struct virt_its *its, uint32_t devid, uint32_t evid, + uint32_t collid, uint32_t vlpi, struct vcpu **vcpu) { struct vits_itte *itte; @@ -332,6 +332,25 @@ static int its_handle_mapd(struct virt_its *its, uint64_t *cmdptr) return 0; } +static int its_handle_mapti(struct virt_its *its, uint64_t *cmdptr) +{ + uint32_t devid = its_cmd_get_deviceid(cmdptr); + uint32_t eventid = its_cmd_get_id(cmdptr); + uint32_t intid = its_cmd_get_physical_id(cmdptr); + int collid = its_cmd_get_collection(cmdptr); + struct vcpu *vcpu; + + if ( its_cmd_get_command(cmdptr) == GITS_CMD_MAPI ) + intid = eventid; + + if ( !write_itte(its, devid, eventid, collid, intid, &vcpu) ) + return -1; + + gicv3_assign_guest_event(its->d, its->doorbell_address, devid, eventid, vcpu, intid); + + return 0; +} + #define ITS_CMD_BUFFER_SIZE(baser) ((((baser) & 0xff) + 1) << 12) static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its, @@ -364,6 +383,10 @@ static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its, case GITS_CMD_MAPD: its_handle_mapd(its, cmdptr); break; + case GITS_CMD_MAPI: + case GITS_CMD_MAPTI: + its_handle_mapti(its, cmdptr); + break; case GITS_CMD_SYNC: /* We handle ITS commands synchronously, so we ignore SYNC. */ break; diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h index 2387972..1ea00e9 100644 --- a/xen/include/asm-arm/gic_v3_its.h +++ b/xen/include/asm-arm/gic_v3_its.h @@ -163,6 +163,12 @@ int gicv3_allocate_host_lpi_block(struct host_its *its, struct domain *d, uint32_t host_devid, uint32_t eventid); int gicv3_free_host_lpi_block(struct host_its *its, uint32_t lpi); +int gicv3_assign_guest_event(struct domain *d, paddr_t doorbell, + uint32_t devid, uint32_t eventid, + struct vcpu *v, uint32_t virt_lpi); +int gicv3_lpi_update_host_entry(uint32_t host_lpi, int domain_id, + unsigned int vcpu_id, uint32_t virt_lpi); + #else static LIST_HEAD(host_its_list);
The MAPTI commands associates a DeviceID/EventID pair with a LPI/CPU pair and actually instantiates LPI interrupts. We connect the already allocated host LPI to this virtual LPI, so that any triggering IRQ on the host can be quickly forwarded to a guest. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- xen/arch/arm/gic-v3-its.c | 63 ++++++++++++++++++++++++++++++++++++++++ xen/arch/arm/gic-v3-lpi.c | 18 ++++++++++++ xen/arch/arm/vgic-v3-its.c | 27 +++++++++++++++-- xen/include/asm-arm/gic_v3_its.h | 6 ++++ 4 files changed, 112 insertions(+), 2 deletions(-)