|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 20/27] ARM: vITS: handle MAPTI command
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@xxxxxxx>
>> ---
>> 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.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |