Re: [Xen-devel] [PATCH v9 16/28] ARM: vITS: handle INT command

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

On 17/05/17 17:17, Julien Grall wrote:
> Hi Andre,
> 
> On 11/05/17 18:53, Andre Przywara wrote:
>> The INT command sets a given LPI identified by a DeviceID/EventID pair
>> as pending and thus triggers it to be injected.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
>> ---
>>  xen/arch/arm/vgic-v3-its.c | 21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
>> index 12ec5f1..f9379c9 100644
>> --- a/xen/arch/arm/vgic-v3-its.c
>> +++ b/xen/arch/arm/vgic-v3-its.c
>> @@ -300,6 +300,24 @@ static uint64_t its_cmd_mask_field(uint64_t
>> *its_cmd, unsigned int word,
>>  #define its_cmd_get_validbit(cmd)       its_cmd_mask_field(cmd, 2,
>> 63,  1)
>>  #define its_cmd_get_ittaddr(cmd)        (its_cmd_mask_field(cmd, 2,
>> 8, 44) << 8)
>>
>> +static int its_handle_int(struct virt_its *its, uint64_t *cmdptr)
>> +{
>> +    uint32_t devid = its_cmd_get_deviceid(cmdptr);
>> +    uint32_t eventid = its_cmd_get_id(cmdptr);
>> +    struct vcpu *vcpu;
>> +    uint32_t vlpi;
>> +
>> +    if ( !read_itte(its, devid, eventid, &vcpu, &vlpi) )
>> +        return -1;
> 
> See my comment on patch #13 about crafting the memory.

So read_itte goes through some checks already (valid VCPU IDs, valid
device table pointer, valid event ID, ...). I believe we can't do much
more than this. I added a fat TODO and an ASSERT(is_dom0) in
vgic_v3_verify_its_status() to not forget about this problem.
Ideally it shouldn't matter what the guest writes into the table,
hopefully the per-IRQ locking ensures this.

Cheers,
Andre.

> 
>> +
>> +    if ( vlpi == INVALID_LPI )
>> +        return -1;
>> +
>> +    vgic_vcpu_inject_irq(vcpu, vlpi);
>> +
>> +    return 0;
>> +}
>> +
>>  #define ITS_CMD_BUFFER_SIZE(baser)      ((((baser) & 0xff) + 1) << 12)
>>  #define ITS_CMD_OFFSET(reg)             ((reg) & GENMASK(19, 5))
>>
>> @@ -329,6 +347,9 @@ static int vgic_its_handle_cmds(struct domain *d,
>> struct virt_its *its)
>>
>>          switch ( its_cmd_get_command(command) )
>>          {
>> +        case GITS_CMD_INT:
>> +            ret = its_handle_int(its, command);
>> +            break;
>>          case GITS_CMD_SYNC:
>>              /* We handle ITS commands synchronously, so we ignore
>> SYNC. */
>>              break;
>>
> 
> Cheers,
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel