| Message ID | 1492164934-988-3-git-send-email-eric.auger@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 14 April 2017 at 11:15, Eric Auger <eric.auger@redhat.com> wrote: > Add description for how to save GICV3 LPI pending bit into > guest RAM pending tables. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> > > --- > v5: creation > --- > Documentation/virtual/kvm/devices/arm-vgic-v3.txt | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt > index c1a2461..9293b45 100644 > --- a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt > +++ b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt > @@ -167,11 +167,17 @@ Groups: > KVM_DEV_ARM_VGIC_CTRL_INIT > request the initialization of the VGIC, no additional parameter in > kvm_device_attr.addr. > + KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES > + save all LPI pending bits into guest RAM pending tables. > + > + The first kB of the pending table is not altered by this operation. > Errors: > -ENXIO: VGIC not properly configured as required prior to calling > this attribute > -ENODEV: no online VCPU > -ENOMEM: memory shortage when allocating vgic internal data > + -EFAULT: Invalid guest ram access > + -EBUSY: One or more VCPUS are running > > > KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO > -- > 2.5.5 When does the -EFAULT return happen? (if the guest points GITS_BASER<n> etc at invalid memory, presumably?) How does the QEMU migration code handle this case? Failing migration because the guest has done something silly doesn't seem too palatable, but trying to avoid that could be more effort than an obscure corner case really merits. thanks -- PMM
Hi Peter, On 25/04/2017 12:43, Peter Maydell wrote: > On 14 April 2017 at 11:15, Eric Auger <eric.auger@redhat.com> wrote: >> Add description for how to save GICV3 LPI pending bit into >> guest RAM pending tables. >> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> >> --- >> v5: creation >> --- >> Documentation/virtual/kvm/devices/arm-vgic-v3.txt | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt >> index c1a2461..9293b45 100644 >> --- a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt >> +++ b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt >> @@ -167,11 +167,17 @@ Groups: >> KVM_DEV_ARM_VGIC_CTRL_INIT >> request the initialization of the VGIC, no additional parameter in >> kvm_device_attr.addr. >> + KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES >> + save all LPI pending bits into guest RAM pending tables. >> + >> + The first kB of the pending table is not altered by this operation. >> Errors: >> -ENXIO: VGIC not properly configured as required prior to calling >> this attribute >> -ENODEV: no online VCPU >> -ENOMEM: memory shortage when allocating vgic internal data >> + -EFAULT: Invalid guest ram access >> + -EBUSY: One or more VCPUS are running >> >> >> KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO >> -- >> 2.5.5 > > When does the -EFAULT return happen? (if the guest points GITS_BASER<n> > etc at invalid memory, presumably?) Yes that's correct, when GICR_PENDBASER contains a bad GPA. How does the QEMU migration code > handle this case? Failing migration because the guest has done something > silly doesn't seem too palatable, but trying to avoid that could be > more effort than an obscure corner case really merits. The kvm_device_access will cause an abort() as for other errors returned by kvm_device_ioctl(). Thanks Eric > > thanks > -- PMM > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On 26 April 2017 at 09:26, Auger Eric <eric.auger@redhat.com> wrote: > On 25/04/2017 12:43, Peter Maydell wrote: >> When does the -EFAULT return happen? (if the guest points GITS_BASER<n> >> etc at invalid memory, presumably?) > > Yes that's correct, when GICR_PENDBASER contains a bad GPA. > >> How does the QEMU migration code >> handle this case? Failing migration because the guest has done something >> silly doesn't seem too palatable, but trying to avoid that could be >> more effort than an obscure corner case really merits. > > The kvm_device_access will cause an abort() as for other errors returned > by kvm_device_ioctl(). That's pretty nasty. Guests shouldn't be able to provoke QEMU into abort()ing, ideally. We don't necessarily have to produce a successful migration, but we should at least fail it cleanly. thanks -- PMM
* Peter Maydell (peter.maydell@linaro.org) wrote: > On 26 April 2017 at 09:26, Auger Eric <eric.auger@redhat.com> wrote: > > On 25/04/2017 12:43, Peter Maydell wrote: > >> When does the -EFAULT return happen? (if the guest points GITS_BASER<n> > >> etc at invalid memory, presumably?) > > > > Yes that's correct, when GICR_PENDBASER contains a bad GPA. > > > >> How does the QEMU migration code > >> handle this case? Failing migration because the guest has done something > >> silly doesn't seem too palatable, but trying to avoid that could be > >> more effort than an obscure corner case really merits. > > > > The kvm_device_access will cause an abort() as for other errors returned > > by kvm_device_ioctl(). > > That's pretty nasty. Guests shouldn't be able to provoke QEMU > into abort()ing, ideally. We don't necessarily have to produce > a successful migration, but we should at least fail it cleanly. Yes, no abort()'s during migration due to guest behaviour. They always end up coming back around to being filed as migration bugs and people worry why they've got cores. Ideally log a message into stderr to say that the guest state is inconsistent so that when someone comes to debug it then they can see it's obvious. Dave > thanks > -- PMM -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Hi Peter, Dave, On 26/04/2017 10:48, Dr. David Alan Gilbert wrote: > * Peter Maydell (peter.maydell@linaro.org) wrote: >> On 26 April 2017 at 09:26, Auger Eric <eric.auger@redhat.com> wrote: >>> On 25/04/2017 12:43, Peter Maydell wrote: >>>> When does the -EFAULT return happen? (if the guest points GITS_BASER<n> >>>> etc at invalid memory, presumably?) >>> >>> Yes that's correct, when GICR_PENDBASER contains a bad GPA. >>> >>>> How does the QEMU migration code >>>> handle this case? Failing migration because the guest has done something >>>> silly doesn't seem too palatable, but trying to avoid that could be >>>> more effort than an obscure corner case really merits. >>> >>> The kvm_device_access will cause an abort() as for other errors returned >>> by kvm_device_ioctl(). >> >> That's pretty nasty. Guests shouldn't be able to provoke QEMU >> into abort()ing, ideally. We don't necessarily have to produce >> a successful migration, but we should at least fail it cleanly. > > Yes, no abort()'s during migration due to guest behaviour. > They always end up coming back around to being filed as migration > bugs and people worry why they've got cores. > > Ideally log a message into stderr to say that the guest state > is inconsistent so that when someone comes to debug it then they > can see it's obvious. OK I agree. I will respin the QEMU part accordingly and in that situation I won't abort and will print a message. Thanks Eric > > Dave > >> thanks >> -- PMM > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Wed, Apr 26, 2017 at 11:57:16AM +0200, Auger Eric wrote: > Hi Peter, Dave, > > On 26/04/2017 10:48, Dr. David Alan Gilbert wrote: > > * Peter Maydell (peter.maydell@linaro.org) wrote: > >> On 26 April 2017 at 09:26, Auger Eric <eric.auger@redhat.com> wrote: > >>> On 25/04/2017 12:43, Peter Maydell wrote: > >>>> When does the -EFAULT return happen? (if the guest points GITS_BASER<n> > >>>> etc at invalid memory, presumably?) > >>> > >>> Yes that's correct, when GICR_PENDBASER contains a bad GPA. > >>> > >>>> How does the QEMU migration code > >>>> handle this case? Failing migration because the guest has done something > >>>> silly doesn't seem too palatable, but trying to avoid that could be > >>>> more effort than an obscure corner case really merits. > >>> > >>> The kvm_device_access will cause an abort() as for other errors returned > >>> by kvm_device_ioctl(). > >> > >> That's pretty nasty. Guests shouldn't be able to provoke QEMU > >> into abort()ing, ideally. We don't necessarily have to produce > >> a successful migration, but we should at least fail it cleanly. > > > > Yes, no abort()'s during migration due to guest behaviour. > > They always end up coming back around to being filed as migration > > bugs and people worry why they've got cores. > > > > Ideally log a message into stderr to say that the guest state > > is inconsistent so that when someone comes to debug it then they > > can see it's obvious. > > OK I agree. I will respin the QEMU part accordingly and in that > situation I won't abort and will print a message. > Alternatively we should mark a pending error notification to the guest in KVM, so that when the guest boots it gets something like an SError instead, given that presumably the guest wrote the weird value. Except of course if the problem is caused by QEMU fudging with the register value for the PENDBASER. Just a thought. Thanks, -Christoffer
On 26 April 2017 at 14:00, Christoffer Dall <cdall@linaro.org> wrote: > Alternatively we should mark a pending error notification to the guest > in KVM, so that when the guest boots it gets something like an SError > instead, given that presumably the guest wrote the weird value. Except > of course if the problem is caused by QEMU fudging with the register > value for the PENDBASER. If we have scope for complaining at the guest we should do it at the point where the guest sets PENDBASER in the first place... thanks -- PMM
On Wed, Apr 26, 2017 at 02:01:55PM +0100, Peter Maydell wrote: > On 26 April 2017 at 14:00, Christoffer Dall <cdall@linaro.org> wrote: > > Alternatively we should mark a pending error notification to the guest > > in KVM, so that when the guest boots it gets something like an SError > > instead, given that presumably the guest wrote the weird value. Except > > of course if the problem is caused by QEMU fudging with the register > > value for the PENDBASER. > > If we have scope for complaining at the guest we should do it at > the point where the guest sets PENDBASER in the first place... > Is that what the hardware would have done? Also, userspace could restore a bogus value in the PENDBASER (even though the guest wrote something sane), so maybe we should just keep this as is and handle it nicely in QEMU? Thanks, -Christoffer > thanks > -- PMM
On 26 April 2017 at 14:14, Christoffer Dall <christoffer.dall@linaro.org> wrote: > On Wed, Apr 26, 2017 at 02:01:55PM +0100, Peter Maydell wrote: >> On 26 April 2017 at 14:00, Christoffer Dall <cdall@linaro.org> wrote: >> > Alternatively we should mark a pending error notification to the guest >> > in KVM, so that when the guest boots it gets something like an SError >> > instead, given that presumably the guest wrote the weird value. Except >> > of course if the problem is caused by QEMU fudging with the register >> > value for the PENDBASER. >> >> If we have scope for complaining at the guest we should do it at >> the point where the guest sets PENDBASER in the first place... >> > > Is that what the hardware would have done? I think it's UNPREDICTABLE to enable the GIC with a bogus PENDBASER, but I can't find the bit in the spec that actually says that. I don't know what hardware actually does, but I imagine it will only notice that it's been handed bogus memory at the point where it tries to use it. > Also, userspace could restore a bogus value in the PENDBASER (even > though the guest wrote something sane), so maybe we should just keep > this as is and handle it nicely in QEMU? Yeah, I don't have a strong objection to doing it that way round. thanks -- PMM
Hi Peter, Christoffer, On 26/04/2017 15:26, Peter Maydell wrote: > On 26 April 2017 at 14:14, Christoffer Dall <christoffer.dall@linaro.org> wrote: >> On Wed, Apr 26, 2017 at 02:01:55PM +0100, Peter Maydell wrote: >>> On 26 April 2017 at 14:00, Christoffer Dall <cdall@linaro.org> wrote: >>>> Alternatively we should mark a pending error notification to the guest >>>> in KVM, so that when the guest boots it gets something like an SError >>>> instead, given that presumably the guest wrote the weird value. Except >>>> of course if the problem is caused by QEMU fudging with the register >>>> value for the PENDBASER. >>> >>> If we have scope for complaining at the guest we should do it at >>> the point where the guest sets PENDBASER in the first place... >>> >> >> Is that what the hardware would have done? > > I think it's UNPREDICTABLE to enable the GIC with a bogus PENDBASER, > but I can't find the bit in the spec that actually says that. > I don't know what hardware actually does, but I imagine it will > only notice that it's been handed bogus memory at the point where > it tries to use it. > >> Also, userspace could restore a bogus value in the PENDBASER (even >> though the guest wrote something sane), so maybe we should just keep >> this as is and handle it nicely in QEMU? > > Yeah, I don't have a strong objection to doing it that way round. OK. I will only update the QEMU code then. For info, without talking about save/restore, the GICR_PENDBASER is sync'ed on LPI enable. if the vITS gets an error on kvm_read_guest, we currently abort the sync without reporting any error. GICR_PROPBASER is read on cmd execution (MAPI, INV, INVALL). No error is reported at the moment. My understanding is our implementation chose the 3d alternative of GICV3 arch spec (6.3.2), ie. "the data that generated the error or errors is treated as having a legal value", increment the read cursor and currently we don't report any system error to the guest. Thanks Eric > > thanks > -- PMM >
diff --git a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt index c1a2461..9293b45 100644 --- a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt +++ b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt @@ -167,11 +167,17 @@ Groups: KVM_DEV_ARM_VGIC_CTRL_INIT request the initialization of the VGIC, no additional parameter in kvm_device_attr.addr. + KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES + save all LPI pending bits into guest RAM pending tables. + + The first kB of the pending table is not altered by this operation. Errors: -ENXIO: VGIC not properly configured as required prior to calling this attribute -ENODEV: no online VCPU -ENOMEM: memory shortage when allocating vgic internal data + -EFAULT: Invalid guest ram access + -EBUSY: One or more VCPUS are running KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO
Add description for how to save GICV3 LPI pending bit into guest RAM pending tables. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- v5: creation --- Documentation/virtual/kvm/devices/arm-vgic-v3.txt | 6 ++++++ 1 file changed, 6 insertions(+)