All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoffer Dall <cdall@linaro.org>
To: Auger Eric <eric.auger@redhat.com>
Cc: eric.auger.pro@gmail.com, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org, marc.zyngier@arm.com,
	peter.maydell@linaro.org, andre.przywara@arm.com,
	wanghaibin.wang@huawei.com, wu.wubin@huawei.com
Subject: Re: [PATCH v2 03/10] KVM: arm/arm64: vgic-its: Improve error reporting on device table save
Date: Fri, 13 Oct 2017 19:56:53 +0200	[thread overview]
Message-ID: <20171013175653.GD17578@cbox> (raw)
In-Reply-To: <0a5b2332-a88f-060f-6c0c-8c8d64f6a1f9@redhat.com>

On Fri, Oct 13, 2017 at 04:22:25PM +0200, Auger Eric wrote:
> Hi,
> 
> On 13/10/2017 15:16, Christoffer Dall wrote:
> > On Wed, Sep 27, 2017 at 03:28:33PM +0200, Eric Auger wrote:
> >> At the moment the device table save() returns -EINVAL if
> >> vgic_its_check_id() fails to return the gpa of the entry
> >> associated to the device/collection id. Let vgic_its_check_id()
> >> return an int instead of a bool and return a more precised
> >> error value:
> >> - EINVAL in case the id is out of range
> >> - EFAULT if the gpa is not provisionned or is not valid
> >>
> > 
> > This is just to ease debugging, yes?
> 
> I understood user-space should be able to discriminate between bad guest
> programming and values corrupted by the userspace (regs for instance).
> In first case QEMU should not abort. In latter case it should abort.

So what is userspace supposed to do in the first case?

> 
> In vgic_its_check_id we are checking the L1 entry validity bit and in
> case it is invalid we can't compute the GPA of the entry. I was thinking
> we should return -EFAULT in that case. But maybe returning EFAULT in
> case the BASER<n> address is not reachable also is wrong because that
> may be caused by the userspace writing a wrong value. Sigh ...
> 

I think if either userspace or the guest programmed something that
cannot be traversed, then you just don't save/restore the ITS properly,
because it's broken anyway, so I don't think we need to replicate the
*same broken state* at the destination.

Maybe I'm missing part of the picture here.

Thanks,
-Christoffer

  reply	other threads:[~2017-10-13 17:56 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-27 13:28 [PATCH v2 00/10] vITS Migration fixes and reset Eric Auger
2017-09-27 13:28 ` [PATCH v2 01/10] KVM: arm/arm64: vgic-its: fix return value for restore Eric Auger
2017-10-06 14:37   ` Andre Przywara
2017-10-06 15:33     ` Auger Eric
2017-10-13 11:04   ` Christoffer Dall
2017-09-27 13:28 ` [PATCH v2 02/10] KVM: arm/arm64: vgic-its: Always allow clearing GITS_CREADR/CWRITER Eric Auger
2017-10-06 14:37   ` Andre Przywara
2017-10-06 15:29     ` Auger Eric
2017-10-13 11:44       ` Christoffer Dall
2017-10-13 11:54         ` Auger Eric
2017-10-13 17:54           ` Christoffer Dall
2017-10-14  8:53             ` Auger Eric
2017-10-14 15:04               ` Christoffer Dall
2017-09-27 13:28 ` [PATCH v2 03/10] KVM: arm/arm64: vgic-its: Improve error reporting on device table save Eric Auger
2017-10-06 14:38   ` Andre Przywara
2017-10-13 13:16   ` Christoffer Dall
2017-10-13 14:22     ` Auger Eric
2017-10-13 17:56       ` Christoffer Dall [this message]
2017-10-14  8:52         ` Auger Eric
2017-10-14 15:06           ` Christoffer Dall
2017-09-27 13:28 ` [PATCH v2 04/10] KVM: arm/arm64: vgic-its: Check GITS_BASER Valid bit before saving tables Eric Auger
2017-10-06 14:38   ` Andre Przywara
2017-10-13 13:24   ` Christoffer Dall
2017-09-27 13:28 ` [PATCH v2 05/10] KVM: arm/arm64: vgic-its: Check GITS_CBASER validity before processing commands Eric Auger
2017-10-06 14:38   ` Andre Przywara
2017-10-06 15:29     ` Auger Eric
2017-09-27 13:28 ` [PATCH v2 06/10] KVM: arm/arm64: vgic-its: Always attempt to save/restore device and collection tables Eric Auger
2017-10-06 14:38   ` Andre Przywara
2017-10-06 15:29     ` Auger Eric
2017-09-27 13:28 ` [PATCH v2 07/10] KVM: arm/arm64: vgic-its: new helper functions to free the caches Eric Auger
2017-10-13 13:35   ` Christoffer Dall
2017-10-13 14:37     ` Auger Eric
2017-10-21  9:02     ` Auger Eric
2017-10-21 14:34       ` Christoffer Dall
2017-09-27 13:28 ` [PATCH v2 08/10] KVM: arm/arm64: vgic-its: free caches when GITS_BASER Valid bit is cleared Eric Auger
2017-10-13 15:19   ` Christoffer Dall
2017-10-13 15:34     ` Auger Eric
2017-10-13 17:59       ` Christoffer Dall
2017-09-27 13:28 ` [PATCH v2 09/10] KVM: arm/arm64: Document KVM_DEV_ARM_ITS_CTRL_RESET Eric Auger
2017-10-12 10:57   ` Peter Maydell
2017-10-12 11:34     ` Auger Eric
2017-10-13 15:26   ` Christoffer Dall
2017-10-13 15:41     ` Auger Eric
2017-10-13 18:00       ` Christoffer Dall
2017-09-27 13:28 ` [PATCH v2 10/10] KVM: arm/arm64: vgic-its: Implement KVM_DEV_ARM_ITS_CTRL_RESET Eric Auger
2017-10-13 15:40   ` Christoffer Dall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171013175653.GD17578@cbox \
    --to=cdall@linaro.org \
    --cc=andre.przywara@arm.com \
    --cc=eric.auger.pro@gmail.com \
    --cc=eric.auger@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=peter.maydell@linaro.org \
    --cc=wanghaibin.wang@huawei.com \
    --cc=wu.wubin@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.