| Message ID | 1417113660-23610-4-git-send-email-christoffer.dall@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 27 November 2014 at 18:40, Christoffer Dall <christoffer.dall@linaro.org> wrote: > It is not clear that this ioctl can be called multiple times for a given > vcpu. Userspace already does this, so clarify the ABI. > > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> > --- > Documentation/virtual/kvm/api.txt | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt > index bb82a90..fc12b4f 100644 > --- a/Documentation/virtual/kvm/api.txt > +++ b/Documentation/virtual/kvm/api.txt > @@ -2453,6 +2453,9 @@ return ENOEXEC for that vcpu. > Note that because some registers reflect machine topology, all vcpus > should be created before this ioctl is invoked. > > +Userspace can call this function multiple times for a given VCPU, which will > +reset the VCPU to its initial states. How about being a little bit more explicit here with something like: "Userspace can call this function multiple times for a given VCPU, including after the VCPU has been run. This will reset the VCPU to its initial state." (I notice that api.txt is inconsistent about using "vcpu" or "VCPU" or "vCPU"... do we have a preference for new text?) > + > Possible features: > - KVM_ARM_VCPU_POWER_OFF: Starts the CPU in a power-off state. > Depends on KVM_CAP_ARM_PSCI. If not set, the CPU will be powered on Do you have to use the same set of feature flags for second and subsequent VCPU_INIT calls, or can they be different each time? thanks -- PMM
On Thu, Nov 27, 2014 at 10:53:50PM +0000, Peter Maydell wrote: > On 27 November 2014 at 18:40, Christoffer Dall > <christoffer.dall@linaro.org> wrote: > > It is not clear that this ioctl can be called multiple times for a given > > vcpu. Userspace already does this, so clarify the ABI. > > > > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> > > --- > > Documentation/virtual/kvm/api.txt | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt > > index bb82a90..fc12b4f 100644 > > --- a/Documentation/virtual/kvm/api.txt > > +++ b/Documentation/virtual/kvm/api.txt > > @@ -2453,6 +2453,9 @@ return ENOEXEC for that vcpu. > > Note that because some registers reflect machine topology, all vcpus > > should be created before this ioctl is invoked. > > > > +Userspace can call this function multiple times for a given VCPU, which will > > +reset the VCPU to its initial states. > > How about being a little bit more explicit here with something like: > > "Userspace can call this function multiple times for a given VCPU, including > after the VCPU has been run. This will reset the VCPU to its initial > state." yeah, better. > > (I notice that api.txt is inconsistent about using "vcpu" or "VCPU" > or "vCPU"... do we have a preference for new text?) > I generally try to match whatever the context is, but I clearly failed here. I don't think there's a preference, no. > > + > > Possible features: > > - KVM_ARM_VCPU_POWER_OFF: Starts the CPU in a power-off state. > > Depends on KVM_CAP_ARM_PSCI. If not set, the CPU will be powered on > > Do you have to use the same set of feature flags for second and > subsequent VCPU_INIT calls, or can they be different each time? > That's a good question. Do you have any opinion on the matter? It seems weird to change the target of a Vcpu from some core to another core, but there is not reason why you shouldn't be able to set a vCpU to be powered off when run, just because it wasn't earlier on, is there? Thanks, -Christoffer
On 2 December 2014 at 14:47, Christoffer Dall <christoffer.dall@linaro.org> wrote: > On Thu, Nov 27, 2014 at 10:53:50PM +0000, Peter Maydell wrote: >> On 27 November 2014 at 18:40, Christoffer Dall >> <christoffer.dall@linaro.org> wrote: >> > Possible features: >> > - KVM_ARM_VCPU_POWER_OFF: Starts the CPU in a power-off state. >> > Depends on KVM_CAP_ARM_PSCI. If not set, the CPU will be powered on >> >> Do you have to use the same set of feature flags for second and >> subsequent VCPU_INIT calls, or can they be different each time? >> > That's a good question. Do you have any opinion on the matter? QEMU always will, so I'd be happy if we said it has to be the same set of flags each time. I guess I'd go for "say they have to match"; we can always relax later if we need to. > It seems weird to change the target of a Vcpu from some core to another > core, but there is not reason why you shouldn't be able to set a vCpU to > be powered off when run, just because it wasn't earlier on, is > there? We need an API for get/set of PSCI power state for migration anyhow, so it's not inherently required to be able to flip this bit on reset. -- PMM
On Tue, Dec 02, 2014 at 03:39:05PM +0000, Peter Maydell wrote: > On 2 December 2014 at 14:47, Christoffer Dall > <christoffer.dall@linaro.org> wrote: > > On Thu, Nov 27, 2014 at 10:53:50PM +0000, Peter Maydell wrote: > >> On 27 November 2014 at 18:40, Christoffer Dall > >> <christoffer.dall@linaro.org> wrote: > >> > Possible features: > >> > - KVM_ARM_VCPU_POWER_OFF: Starts the CPU in a power-off state. > >> > Depends on KVM_CAP_ARM_PSCI. If not set, the CPU will be powered on > >> > >> Do you have to use the same set of feature flags for second and > >> subsequent VCPU_INIT calls, or can they be different each time? > >> > > That's a good question. Do you have any opinion on the matter? > > QEMU always will, so I'd be happy if we said it has to be the same > set of flags each time. I guess I'd go for "say they have to match"; > we can always relax later if we need to. > > > It seems weird to change the target of a Vcpu from some core to another > > core, but there is not reason why you shouldn't be able to set a vCpU to > > be powered off when run, just because it wasn't earlier on, is > > there? > > We need an API for get/set of PSCI power state for migration > anyhow, so it's not inherently required to be able to flip > this bit on reset. > Actually I think the current migration patches rely on being able to call the init ioctl to turn off a vcpu, but I guess you could use the KVM_SET_MP_STATE for that. Alex, any thoughts? -Christoffer
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index bb82a90..fc12b4f 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -2453,6 +2453,9 @@ return ENOEXEC for that vcpu. Note that because some registers reflect machine topology, all vcpus should be created before this ioctl is invoked. +Userspace can call this function multiple times for a given VCPU, which will +reset the VCPU to its initial states. + Possible features: - KVM_ARM_VCPU_POWER_OFF: Starts the CPU in a power-off state. Depends on KVM_CAP_ARM_PSCI. If not set, the CPU will be powered on
It is not clear that this ioctl can be called multiple times for a given vcpu. Userspace already does this, so clarify the ABI. Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> --- Documentation/virtual/kvm/api.txt | 3 +++ 1 file changed, 3 insertions(+)