From: Paul Mackerras <paulus@samba.org>
To: Alexander Graf <agraf@suse.de>
Cc: kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH 04/11] KVM: PPC: Book3S HV: Add GET/SET_ONE_REG interface for LPCR
Date: Sat, 14 Sep 2013 02:21:09 +0000 [thread overview]
Message-ID: <20130914022109.GC5872@iris.ozlabs.ibm.com> (raw)
In-Reply-To: <2614E750-19D5-4A30-A1AC-B63DDC327721@suse.de>
On Fri, Sep 13, 2013 at 01:36:25PM -0500, Alexander Graf wrote:
>
> On 05.09.2013, at 22:21, Paul Mackerras wrote:
>
> > This adds the ability for userspace to read and write the LPCR
> > (Logical Partitioning Control Register) value relating to a guest
> > via the GET/SET_ONE_REG interface. There is only one LPCR value
> > for the guest, which can be accessed through any vcpu. Userspace
> > can only modify the following fields of the LPCR value:
> >
> > DPFD Default prefetch depth
> > ILE Interrupt little-endian
> > TC Translation control (secondary HPT hash group search disable)
> >
> > Signed-off-by: Paul Mackerras <paulus@samba.org>
>
> There are 3 things I dislike about this patch :)
>
> 1) A vcpu one_reg should only change the state of the vcpu it's targeting. You want a vm wide thing.
In hardware there is in fact an LPCR per hardware CPU thread, though
the architecture says that on threaded processors many of the fields
have to be the same on each thread, including the three fields
mentioned here.
> 2) If anyone gets crazy enough to implement HV emulation in PR KVM this would overlap with the guest's guest LPCR, so we need to name it differently. It's really VM configuration, not a register. Maybe ENABLE_CAP is a better fit?
If we were doing HV emulation in PR KVM then there would still only be
one LPCR per guest vcpu. If there was a hypervisor running as the PR
guest, it would be its job to context switch the LPCR between its
guests. So I don't see why there would be any need for the top-level
KVM to know about the guest's guest LPCR.
In that situation we would need a one_reg to get and set the guest's
LPCR, which would be per vcpu, and there would be no restriction on
which bits could be set by the host.
It sounds to me like the best option would be to keep an LPCR per vcpu
and use the one_reg interface to let the host get and set it. The
actual LPCR applied when the guest runs would use some bits from the
per-vcpu value plus some bits set by the KVM host code (for the host's
protection). How does that sound?
Alternatively I could define a new per-VM ioctl. ENABLE_CAP doesn't
really help since it also is per-vcpu, not per-VM.
> 3) Checkpatch fails:
>
> WARNING: please, no space before tabs
> #59: FILE: arch/powerpc/include/asm/reg.h:295:
> +#define LPCR_TC ^I0x00000200^I/* Translation control */$
Oops, will fix.
Paul.
WARNING: multiple messages have this Message-ID (diff)
From: Paul Mackerras <paulus@samba.org>
To: Alexander Graf <agraf@suse.de>
Cc: kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH 04/11] KVM: PPC: Book3S HV: Add GET/SET_ONE_REG interface for LPCR
Date: Sat, 14 Sep 2013 12:21:09 +1000 [thread overview]
Message-ID: <20130914022109.GC5872@iris.ozlabs.ibm.com> (raw)
In-Reply-To: <2614E750-19D5-4A30-A1AC-B63DDC327721@suse.de>
On Fri, Sep 13, 2013 at 01:36:25PM -0500, Alexander Graf wrote:
>
> On 05.09.2013, at 22:21, Paul Mackerras wrote:
>
> > This adds the ability for userspace to read and write the LPCR
> > (Logical Partitioning Control Register) value relating to a guest
> > via the GET/SET_ONE_REG interface. There is only one LPCR value
> > for the guest, which can be accessed through any vcpu. Userspace
> > can only modify the following fields of the LPCR value:
> >
> > DPFD Default prefetch depth
> > ILE Interrupt little-endian
> > TC Translation control (secondary HPT hash group search disable)
> >
> > Signed-off-by: Paul Mackerras <paulus@samba.org>
>
> There are 3 things I dislike about this patch :)
>
> 1) A vcpu one_reg should only change the state of the vcpu it's targeting. You want a vm wide thing.
In hardware there is in fact an LPCR per hardware CPU thread, though
the architecture says that on threaded processors many of the fields
have to be the same on each thread, including the three fields
mentioned here.
> 2) If anyone gets crazy enough to implement HV emulation in PR KVM this would overlap with the guest's guest LPCR, so we need to name it differently. It's really VM configuration, not a register. Maybe ENABLE_CAP is a better fit?
If we were doing HV emulation in PR KVM then there would still only be
one LPCR per guest vcpu. If there was a hypervisor running as the PR
guest, it would be its job to context switch the LPCR between its
guests. So I don't see why there would be any need for the top-level
KVM to know about the guest's guest LPCR.
In that situation we would need a one_reg to get and set the guest's
LPCR, which would be per vcpu, and there would be no restriction on
which bits could be set by the host.
It sounds to me like the best option would be to keep an LPCR per vcpu
and use the one_reg interface to let the host get and set it. The
actual LPCR applied when the guest runs would use some bits from the
per-vcpu value plus some bits set by the KVM host code (for the host's
protection). How does that sound?
Alternatively I could define a new per-VM ioctl. ENABLE_CAP doesn't
really help since it also is per-vcpu, not per-VM.
> 3) Checkpatch fails:
>
> WARNING: please, no space before tabs
> #59: FILE: arch/powerpc/include/asm/reg.h:295:
> +#define LPCR_TC ^I0x00000200^I/* Translation control */$
Oops, will fix.
Paul.
next prev parent reply other threads:[~2013-09-14 2:21 UTC|newest]
Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-06 3:10 [PATCH 00/11] HV KVM improvements in preparation for POWER8 support Paul Mackerras
2013-09-06 3:10 ` Paul Mackerras
2013-09-06 3:11 ` [PATCH 01/11] KVM: PPC: Book3S HV: Save/restore SIAR and SDAR along with other PMU registers Paul Mackerras
2013-09-06 3:11 ` Paul Mackerras
2013-09-13 21:51 ` Alexander Graf
2013-09-13 21:51 ` Alexander Graf
2013-09-06 3:17 ` [PATCH 02/11] KVM: PPC: Book3S HV: Implement timebase offset for guests Paul Mackerras
2013-09-06 3:17 ` Paul Mackerras
2013-09-13 21:51 ` Alexander Graf
2013-09-13 21:51 ` Alexander Graf
2013-09-06 3:18 ` [PATCH 03/11] KVM: PPC: Book3S: Add GET/SET_ONE_REG interface for VRSAVE Paul Mackerras
2013-09-06 3:18 ` Paul Mackerras
2013-09-13 21:51 ` Alexander Graf
2013-09-13 21:51 ` Alexander Graf
2013-09-14 2:07 ` Paul Mackerras
2013-09-14 2:07 ` Paul Mackerras
2013-09-06 3:21 ` [PATCH 04/11] KVM: PPC: Book3S HV: Add GET/SET_ONE_REG interface for LPCR Paul Mackerras
2013-09-06 3:21 ` Paul Mackerras
2013-09-13 18:36 ` Alexander Graf
2013-09-13 18:36 ` Alexander Graf
2013-09-14 2:21 ` Paul Mackerras [this message]
2013-09-14 2:21 ` Paul Mackerras
2013-09-14 5:12 ` Alexander Graf
2013-09-14 5:12 ` Alexander Graf
2013-09-14 5:58 ` Paul Mackerras
2013-09-14 5:58 ` Paul Mackerras
2013-09-14 11:38 ` Alexander Graf
2013-09-14 11:38 ` Alexander Graf
2013-09-06 3:22 ` [PATCH 05/11] KVM: PPC: Book3S HV: Add support for guest Program Priority Register Paul Mackerras
2013-09-06 3:22 ` Paul Mackerras
2013-09-13 21:51 ` Alexander Graf
2013-09-13 21:51 ` Alexander Graf
2013-09-17 3:29 ` Benjamin Herrenschmidt
2013-09-17 3:29 ` Benjamin Herrenschmidt
2013-09-20 3:39 ` Alexander Graf
2013-09-20 3:39 ` Alexander Graf
2013-09-06 3:22 ` [PATCH 06/11] KVM: PPC: Book3S HV: Support POWER6 compatibility mode on POWER7 Paul Mackerras
2013-09-06 3:22 ` Paul Mackerras
2013-09-06 5:28 ` Aneesh Kumar K.V
2013-09-06 5:40 ` Aneesh Kumar K.V
2013-09-06 6:38 ` Paul Mackerras
2013-09-06 6:38 ` Paul Mackerras
2013-09-13 19:58 ` Alexander Graf
2013-09-13 19:58 ` Alexander Graf
2013-09-14 2:03 ` Paul Mackerras
2013-09-14 2:03 ` Paul Mackerras
2013-09-06 3:23 ` [PATCH 07/11] KVM: PPC: Book3S HV: Implement H_CONFER Paul Mackerras
2013-09-06 3:23 ` Paul Mackerras
2013-09-13 21:51 ` Alexander Graf
2013-09-13 21:51 ` Alexander Graf
2013-09-06 3:23 ` [PATCH 08/11] KVM: PPC: Book3S HV: Restructure kvmppc_hv_entry to be a subroutine Paul Mackerras
2013-09-06 3:23 ` Paul Mackerras
2013-09-13 21:51 ` Alexander Graf
2013-09-13 21:51 ` Alexander Graf
2013-09-06 3:24 ` [PATCH 09/11] KVM: PPC: Book3S HV: Pull out interrupt-reading code into " Paul Mackerras
2013-09-06 3:24 ` Paul Mackerras
2013-09-13 21:51 ` Alexander Graf
2013-09-13 21:51 ` Alexander Graf
2013-09-06 3:24 ` [PATCH 10/11] KVM: PPC: Book3S HV: Avoid unbalanced increments of VPA yield count Paul Mackerras
2013-09-06 3:24 ` Paul Mackerras
2013-09-13 21:51 ` Alexander Graf
2013-09-13 21:51 ` Alexander Graf
2013-09-06 3:25 ` [PATCH 11/11] KVM: PPC: Book3S HV: Return -EINVAL rather than BUG'ing Paul Mackerras
2013-09-06 3:25 ` Paul Mackerras
2013-09-13 21:51 ` Alexander Graf
2013-09-13 21:51 ` Alexander Graf
2013-09-11 9:11 ` [PATCH 00/11] HV KVM improvements in preparation for POWER8 support Paul Mackerras
2013-09-11 9:11 ` Paul Mackerras
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=20130914022109.GC5872@iris.ozlabs.ibm.com \
--to=paulus@samba.org \
--cc=agraf@suse.de \
--cc=kvm-ppc@vger.kernel.org \
--cc=kvm@vger.kernel.org \
/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.