Hi Eric, Sorry for delayed reply. > -----Original Message----- > From: Auger Eric [mailto:eric.auger@xxxxxxxxxx] > Sent: Wednesday, September 6, 2017 12:53 PM > To: Vijaya Kumar K <vkilari@xxxxxxxxxxxxxx>; > kvmarm@xxxxxxxxxxxxxxxxxxxxx; marc.zyngier@xxxxxxx; cdall@xxxxxxxxxx; > andre.przywara@xxxxxxx > Cc: vvenkat@xxxxxxxxxxxxxx; shankerd@xxxxxxxxxxxxxx; > kvm@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH] KVM: arm64: vgic-its: Fix wrong return value check in > vgic_its_restore_device_tables > > Hi Vijaya, > > On 06/09/2017 07:26, Vijaya Kumar K wrote: > > scan_its_table() return 1 on success. > > As mentioned in the kernel-doc comment of scan_its_table, this latter > returns 1 if the last element is not found. Than can happen while scanning an > L2 table but shouldn't happen if we scan an L1 table. > > * Return: < 0 on error, 0 if last element was identified, 1 otherwise > * (the last element may not be found on second level tables) OK. I will fix this comment > > > In the function vgic_its_restore_device_tables() > > the return value of scan_its_table() is checked against success value > > and returns -EINVAL. Hence migration fails for VM with ITS. > > > > With this patch the failure return value is checked while returning > > -EINVAL. > > > > Signed-off-by: Vijaya Kumar K <vkilari@xxxxxxxxxxxxxx> > > > > diff --git a/virt/kvm/arm/vgic/vgic-its.c > > b/virt/kvm/arm/vgic/vgic-its.c index aa6b68d..63f8ac3 100644 > > --- a/virt/kvm/arm/vgic/vgic-its.c > > +++ b/virt/kvm/arm/vgic/vgic-its.c > > @@ -2142,7 +2142,7 @@ static int vgic_its_restore_device_tables(struct > vgic_its *its) > > vgic_its_restore_dte, NULL); > > } > > > > - if (ret > 0) > > + if (ret <= 0) > > ret = -EINVAL; > your modification would return -EINVAL for whatever error encountered > during the scan table or if last element is found. I don't think this is what we > want. IIUC, ret 0 indicates last entry of the table. So in this case return value 0 is also success. with the assumption that table might be smaller than size. So only check for < 0 and return -EINVAL. For all other return values 0 and > 0 return 0. as below. Please correct me if I wrong. If (ret < 0) ret = -EINVAL; else ret = 0;
