diff mbox

arm64/kvm: Add generic v8 KVM target

Message ID 1434531646-4873-1-git-send-email-suzuki.poulose@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Suzuki K Poulose June 17, 2015, 9 a.m. UTC
From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

This patch adds a generic ARM v8 KVM target cpu type for use
by the new CPUs which eventualy ends up using the common sys_reg
table. For backward compatibility the existing targets have been
preserved. Any new target CPU that can be covered by generic v8
sys_reg tables should make use of the new generic target.

Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
Acked-by: Marc Zyngier <Marc.Zyngier@arm.com>
---
 arch/arm64/include/uapi/asm/kvm.h    |   10 ++++++++--
 arch/arm64/kvm/guest.c               |    3 ++-
 arch/arm64/kvm/sys_regs_generic_v8.c |    2 ++
 3 files changed, 12 insertions(+), 3 deletions(-)

Comments

Timur Tabi June 19, 2015, 8:31 p.m. UTC | #1
On 06/17/2015 04:00 AM, Suzuki K. Poulose wrote:
>   					  &genericv8_target_table);
>   	kvm_register_target_sys_reg_table(KVM_ARM_TARGET_XGENE_POTENZA,
>   					  &genericv8_target_table);
> +	kvm_register_target_sys_reg_table(KVM_ARM_TARGET_GENERIC_V8,
> +					  &genericv8_target_table);
>

Shouldn't you also remove all of the previous lines that return 
&genericv8_target_table?
Marc Zyngier June 22, 2015, 6:47 a.m. UTC | #2
On Fri, 19 Jun 2015 21:31:27 +0100
Timur Tabi <timur@codeaurora.org> wrote:

> On 06/17/2015 04:00 AM, Suzuki K. Poulose wrote:
> >   					  &genericv8_target_table);
> >   	kvm_register_target_sys_reg_table(KVM_ARM_TARGET_XGENE_POTENZA,
> >   					  &genericv8_target_table);
> > +
> > kvm_register_target_sys_reg_table(KVM_ARM_TARGET_GENERIC_V8,
> > +					  &genericv8_target_table);
> >
> 
> Shouldn't you also remove all of the previous lines that return 
> &genericv8_target_table?

No. KVM_ARM_TARGET_* are part of the uapi, and existing userspace knows
about them. You can't just drop them.

What you *could* do would be to map the existing targets to the generic
one in a way that leaves userspace blissfully unaware of the underlying
change (for example, KVM_ARM_PREFERRED_TARGET should still return
KVM_ARM_TARGET_XGENE_POTENZA on an XGene platform, and
KVM_ARM_VCPU_INIT should still be accept it).

What would be the gain of such a mapping? Added complexity, hardly
anything else. We're much better off considering the generic target as
a new one, both inside the kernel and in the view we export to
userspace.

Thanks,

	M.
Peter Maydell June 22, 2015, 8:44 a.m. UTC | #3
On 17 June 2015 at 10:00, Suzuki K. Poulose <suzuki.poulose@arm.com> wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>
> This patch adds a generic ARM v8 KVM target cpu type for use
> by the new CPUs which eventualy ends up using the common sys_reg
> table. For backward compatibility the existing targets have been
> preserved. Any new target CPU that can be covered by generic v8
> sys_reg tables should make use of the new generic target.

How do you intend this to work for cross-host migration?
Is the idea that the kernel guarantees that "generic" looks
100% the same to the guest regardless of host hardware? I'm
not sure that can be made to work, given impdef differences
in ID register values, bp/wp registers, and so on.

Given that, it seems to me that we still need to provide
KVM_ARM_TARGET_$THISCPU defines so userspace can request
a specific guest CPU flavour; so what does this patch
provide that isn't already provided by just having userspace
query for the "preferred" CPU type as it does already?

thanks
-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
Christoffer Dall June 23, 2015, 12:39 p.m. UTC | #4
On Mon, Jun 22, 2015 at 09:44:48AM +0100, Peter Maydell wrote:
> On 17 June 2015 at 10:00, Suzuki K. Poulose <suzuki.poulose@arm.com> wrote:
> > From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> >
> > This patch adds a generic ARM v8 KVM target cpu type for use
> > by the new CPUs which eventualy ends up using the common sys_reg
> > table. For backward compatibility the existing targets have been
> > preserved. Any new target CPU that can be covered by generic v8
> > sys_reg tables should make use of the new generic target.
> 
> How do you intend this to work for cross-host migration?
> Is the idea that the kernel guarantees that "generic" looks
> 100% the same to the guest regardless of host hardware? I'm
> not sure that can be made to work, given impdef differences
> in ID register values, bp/wp registers, and so on.
> 
> Given that, it seems to me that we still need to provide
> KVM_ARM_TARGET_$THISCPU defines so userspace can request
> a specific guest CPU flavour; so what does this patch
> provide that isn't already provided by just having userspace
> query for the "preferred" CPU type as it does already?
> 
I'm guessing the intention is to avoid having to add code in the kernel
to support KVM on a new CPU where nothing else needs to be done to
support KVM on that system.

Wrt. migration, I was also wondering about this.  Would the differences
in the CPU architecture be detected when feeding back the invariant
sysregs from userspace on VM restore?

-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suzuki K Poulose June 23, 2015, 2:03 p.m. UTC | #5
On 23/06/15 13:39, Christoffer Dall wrote:
> On Mon, Jun 22, 2015 at 09:44:48AM +0100, Peter Maydell wrote:
>> On 17 June 2015 at 10:00, Suzuki K. Poulose <suzuki.poulose@arm.com> wrote:
>>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>>
>>> This patch adds a generic ARM v8 KVM target cpu type for use
>>> by the new CPUs which eventualy ends up using the common sys_reg
>>> table. For backward compatibility the existing targets have been
>>> preserved. Any new target CPU that can be covered by generic v8
>>> sys_reg tables should make use of the new generic target.
>>
>> How do you intend this to work for cross-host migration?
>> Is the idea that the kernel guarantees that "generic" looks
>> 100% the same to the guest regardless of host hardware? I'm
>> not sure that can be made to work, given impdef differences
>> in ID register values, bp/wp registers, and so on.
>>
>> Given that, it seems to me that we still need to provide
>> KVM_ARM_TARGET_$THISCPU defines so userspace can request
>> a specific guest CPU flavour; so what does this patch
>> provide that isn't already provided by just having userspace
>> query for the "preferred" CPU type as it does already?
>>
> I'm guessing the intention is to avoid having to add code in the kernel
> to support KVM on a new CPU where nothing else needs to be done to
> support KVM on that system.
Yes, thats the *only* motivation behind the patch and doesn't address
the migration issue. May be we can create a dummy set of values for
the ID registers, which doesn't provide any 'special functionality'
so that it is safe to be migrated across any host ?

>
> Wrt. migration, I was also wondering about this.  Would the differences
> in the CPU architecture be detected when feeding back the invariant
> sysregs from userspace on VM restore?
>
> -Christoffer
>

Suzuki

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Maydell June 23, 2015, 2:16 p.m. UTC | #6
On 23 June 2015 at 15:03, Suzuki K. Poulose <Suzuki.Poulose@arm.com> wrote:
> On 23/06/15 13:39, Christoffer Dall wrote:
>> On Mon, Jun 22, 2015 at 09:44:48AM +0100, Peter Maydell wrote:
>> I'm guessing the intention is to avoid having to add code in the kernel
>> to support KVM on a new CPU where nothing else needs to be done to
>> support KVM on that system.
>
> Yes, thats the *only* motivation behind the patch

I'm not convinced we really want to change the userspace ABI design
principles just for convenience of internal kernel implementation.
At the moment the approach is:
 (1) if you want a specific guest CPU, you ask for it; the kernel
guarantees that if this succeeds, you can cross-migrate between
this and another kernel that also lets you init that guest CPU
[at the moment this implies no cross-host-type migration, of course]
 (2) if you don't care, you ask the kernel what its preferred
CPU type is, and use it

If we want to change away from this model (which has at least
the advantages of being coherent and sort-of similar to
how x86 does things), what's the model we're moving to?

> May be we can create a dummy set of values for
> the ID registers, which doesn't provide any 'special functionality'
> so that it is safe to be migrated across any host ?

I suspect this problem is larger and requires a more complex
solution than merely providing funky ID register values. If we
can do it successfully that would be really handy, though...

thanks
-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier June 24, 2015, 8:29 a.m. UTC | #7
On 22/06/15 09:44, Peter Maydell wrote:
> On 17 June 2015 at 10:00, Suzuki K. Poulose <suzuki.poulose@arm.com> wrote:
>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>
>> This patch adds a generic ARM v8 KVM target cpu type for use
>> by the new CPUs which eventualy ends up using the common sys_reg
>> table. For backward compatibility the existing targets have been
>> preserved. Any new target CPU that can be covered by generic v8
>> sys_reg tables should make use of the new generic target.
> 
> How do you intend this to work for cross-host migration?

It is not meant to work for cross migration at all.

> Is the idea that the kernel guarantees that "generic" looks
> 100% the same to the guest regardless of host hardware? I'm
> not sure that can be made to work, given impdef differences
> in ID register values, bp/wp registers, and so on.
> 
> Given that, it seems to me that we still need to provide
> KVM_ARM_TARGET_$THISCPU defines so userspace can request
> a specific guest CPU flavour; so what does this patch
> provide that isn't already provided by just having userspace
> query for the "preferred" CPU type as it does already?

The way I see this working is that a "generic" CPU cannot be migrated
(because we don't know anything about it). If it can be identified as a
known (non generic) implementation, then we can migrate it.

	M.
Christoffer Dall June 24, 2015, 8:51 a.m. UTC | #8
On Wed, Jun 24, 2015 at 09:29:56AM +0100, Marc Zyngier wrote:
> On 22/06/15 09:44, Peter Maydell wrote:
> > On 17 June 2015 at 10:00, Suzuki K. Poulose <suzuki.poulose@arm.com> wrote:
> >> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> >>
> >> This patch adds a generic ARM v8 KVM target cpu type for use
> >> by the new CPUs which eventualy ends up using the common sys_reg
> >> table. For backward compatibility the existing targets have been
> >> preserved. Any new target CPU that can be covered by generic v8
> >> sys_reg tables should make use of the new generic target.
> > 
> > How do you intend this to work for cross-host migration?
> 
> It is not meant to work for cross migration at all.
> 
> > Is the idea that the kernel guarantees that "generic" looks
> > 100% the same to the guest regardless of host hardware? I'm
> > not sure that can be made to work, given impdef differences
> > in ID register values, bp/wp registers, and so on.
> > 
> > Given that, it seems to me that we still need to provide
> > KVM_ARM_TARGET_$THISCPU defines so userspace can request
> > a specific guest CPU flavour; so what does this patch
> > provide that isn't already provided by just having userspace
> > query for the "preferred" CPU type as it does already?
> 
> The way I see this working is that a "generic" CPU cannot be migrated
> (because we don't know anything about it). If it can be identified as a
> known (non generic) implementation, then we can migrate it.
> 
Concretely, how should this work?  Be enforced by userspace or should we
deny certain SET_ONE_REG operations from working on this target?

Also, can we imagine any scenario where the generic CPU cannot me
modeled for a VM on a specific piece of hardware (current or future)?

-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier June 24, 2015, 9:32 a.m. UTC | #9
Hi Christoffer,

On 24/06/15 09:51, Christoffer Dall wrote:
> On Wed, Jun 24, 2015 at 09:29:56AM +0100, Marc Zyngier wrote:
>> On 22/06/15 09:44, Peter Maydell wrote:
>>> On 17 June 2015 at 10:00, Suzuki K. Poulose <suzuki.poulose@arm.com> wrote:
>>>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>>>
>>>> This patch adds a generic ARM v8 KVM target cpu type for use
>>>> by the new CPUs which eventualy ends up using the common sys_reg
>>>> table. For backward compatibility the existing targets have been
>>>> preserved. Any new target CPU that can be covered by generic v8
>>>> sys_reg tables should make use of the new generic target.
>>>
>>> How do you intend this to work for cross-host migration?
>>
>> It is not meant to work for cross migration at all.
>>
>>> Is the idea that the kernel guarantees that "generic" looks
>>> 100% the same to the guest regardless of host hardware? I'm
>>> not sure that can be made to work, given impdef differences
>>> in ID register values, bp/wp registers, and so on.
>>>
>>> Given that, it seems to me that we still need to provide
>>> KVM_ARM_TARGET_$THISCPU defines so userspace can request
>>> a specific guest CPU flavour; so what does this patch
>>> provide that isn't already provided by just having userspace
>>> query for the "preferred" CPU type as it does already?
>>
>> The way I see this working is that a "generic" CPU cannot be migrated
>> (because we don't know anything about it). If it can be identified as a
>> known (non generic) implementation, then we can migrate it.
>>
> Concretely, how should this work?  Be enforced by userspace or should we
> deny certain SET_ONE_REG operations from working on this target?

I can definitely see MIDR overriding failing on a generic CPU. Also, you
shouldn't be able to select a generic CPU if we know about the physical CPU.

> 
> Also, can we imagine any scenario where the generic CPU cannot me
> modeled for a VM on a specific piece of hardware (current or future)?

What is the definition of a generic CPU here? In the above, generic
really means "Unknown", so I can't immediately see what it would mean to
model this.

	M.
Christoffer Dall June 25, 2015, 12:30 p.m. UTC | #10
On Wed, Jun 24, 2015 at 10:32:38AM +0100, Marc Zyngier wrote:
> Hi Christoffer,
> 
> On 24/06/15 09:51, Christoffer Dall wrote:
> > On Wed, Jun 24, 2015 at 09:29:56AM +0100, Marc Zyngier wrote:
> >> On 22/06/15 09:44, Peter Maydell wrote:
> >>> On 17 June 2015 at 10:00, Suzuki K. Poulose <suzuki.poulose@arm.com> wrote:
> >>>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> >>>>
> >>>> This patch adds a generic ARM v8 KVM target cpu type for use
> >>>> by the new CPUs which eventualy ends up using the common sys_reg
> >>>> table. For backward compatibility the existing targets have been
> >>>> preserved. Any new target CPU that can be covered by generic v8
> >>>> sys_reg tables should make use of the new generic target.
> >>>
> >>> How do you intend this to work for cross-host migration?
> >>
> >> It is not meant to work for cross migration at all.
> >>
> >>> Is the idea that the kernel guarantees that "generic" looks
> >>> 100% the same to the guest regardless of host hardware? I'm
> >>> not sure that can be made to work, given impdef differences
> >>> in ID register values, bp/wp registers, and so on.
> >>>
> >>> Given that, it seems to me that we still need to provide
> >>> KVM_ARM_TARGET_$THISCPU defines so userspace can request
> >>> a specific guest CPU flavour; so what does this patch
> >>> provide that isn't already provided by just having userspace
> >>> query for the "preferred" CPU type as it does already?
> >>
> >> The way I see this working is that a "generic" CPU cannot be migrated
> >> (because we don't know anything about it). If it can be identified as a
> >> known (non generic) implementation, then we can migrate it.
> >>
> > Concretely, how should this work?  Be enforced by userspace or should we
> > deny certain SET_ONE_REG operations from working on this target?
> 
> I can definitely see MIDR overriding failing on a generic CPU. Also, you
> shouldn't be able to select a generic CPU if we know about the physical CPU.
> 

If I am migrating from an A57 to an A53, but the VM was started as
"generic CPU" then we rely on the user discovering that this is not
supported when trying to set the MIDR from userspace in KVM?

> > 
> > Also, can we imagine any scenario where the generic CPU cannot me
> > modeled for a VM on a specific piece of hardware (current or future)?
> 
> What is the definition of a generic CPU here? In the above, generic
> really means "Unknown", so I can't immediately see what it would mean to
> model this.
> 
ok, good way to phrase it, I suppose that's a non-issue then.

Thanks,
-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier June 25, 2015, 12:40 p.m. UTC | #11
On 25/06/15 13:30, Christoffer Dall wrote:
> On Wed, Jun 24, 2015 at 10:32:38AM +0100, Marc Zyngier wrote:
>> Hi Christoffer,
>>
>> On 24/06/15 09:51, Christoffer Dall wrote:
>>> On Wed, Jun 24, 2015 at 09:29:56AM +0100, Marc Zyngier wrote:
>>>> On 22/06/15 09:44, Peter Maydell wrote:
>>>>> On 17 June 2015 at 10:00, Suzuki K. Poulose <suzuki.poulose@arm.com> wrote:
>>>>>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>>>>>
>>>>>> This patch adds a generic ARM v8 KVM target cpu type for use
>>>>>> by the new CPUs which eventualy ends up using the common sys_reg
>>>>>> table. For backward compatibility the existing targets have been
>>>>>> preserved. Any new target CPU that can be covered by generic v8
>>>>>> sys_reg tables should make use of the new generic target.
>>>>>
>>>>> How do you intend this to work for cross-host migration?
>>>>
>>>> It is not meant to work for cross migration at all.
>>>>
>>>>> Is the idea that the kernel guarantees that "generic" looks
>>>>> 100% the same to the guest regardless of host hardware? I'm
>>>>> not sure that can be made to work, given impdef differences
>>>>> in ID register values, bp/wp registers, and so on.
>>>>>
>>>>> Given that, it seems to me that we still need to provide
>>>>> KVM_ARM_TARGET_$THISCPU defines so userspace can request
>>>>> a specific guest CPU flavour; so what does this patch
>>>>> provide that isn't already provided by just having userspace
>>>>> query for the "preferred" CPU type as it does already?
>>>>
>>>> The way I see this working is that a "generic" CPU cannot be migrated
>>>> (because we don't know anything about it). If it can be identified as a
>>>> known (non generic) implementation, then we can migrate it.
>>>>
>>> Concretely, how should this work?  Be enforced by userspace or should we
>>> deny certain SET_ONE_REG operations from working on this target?
>>
>> I can definitely see MIDR overriding failing on a generic CPU. Also, you
>> shouldn't be able to select a generic CPU if we know about the physical CPU.
>>
> 
> If I am migrating from an A57 to an A53, but the VM was started as
> "generic CPU" then we rely on the user discovering that this is not
> supported when trying to set the MIDR from userspace in KVM?

A57 to A53 is probably a bad example because we actively support both.
So let's replace your A57 with an A72. With this patch, the A72 would be
reported as "generic CPU", and MIDR access would fail on the A53.

Admittedly, this is a bit late. We could also refuse to instantiate a
"generic CPU" on A53, but that's not much better timing wise.

Not sure we can do much better though.

	M.
Marc Zyngier June 25, 2015, 1:44 p.m. UTC | #12
On 25/06/15 13:40, Marc Zyngier wrote:
> On 25/06/15 13:30, Christoffer Dall wrote:
>> On Wed, Jun 24, 2015 at 10:32:38AM +0100, Marc Zyngier wrote:
>>> Hi Christoffer,
>>>
>>> On 24/06/15 09:51, Christoffer Dall wrote:
>>>> On Wed, Jun 24, 2015 at 09:29:56AM +0100, Marc Zyngier wrote:
>>>>> On 22/06/15 09:44, Peter Maydell wrote:
>>>>>> On 17 June 2015 at 10:00, Suzuki K. Poulose <suzuki.poulose@arm.com> wrote:
>>>>>>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>>>>>>
>>>>>>> This patch adds a generic ARM v8 KVM target cpu type for use
>>>>>>> by the new CPUs which eventualy ends up using the common sys_reg
>>>>>>> table. For backward compatibility the existing targets have been
>>>>>>> preserved. Any new target CPU that can be covered by generic v8
>>>>>>> sys_reg tables should make use of the new generic target.
>>>>>>
>>>>>> How do you intend this to work for cross-host migration?
>>>>>
>>>>> It is not meant to work for cross migration at all.
>>>>>
>>>>>> Is the idea that the kernel guarantees that "generic" looks
>>>>>> 100% the same to the guest regardless of host hardware? I'm
>>>>>> not sure that can be made to work, given impdef differences
>>>>>> in ID register values, bp/wp registers, and so on.
>>>>>>
>>>>>> Given that, it seems to me that we still need to provide
>>>>>> KVM_ARM_TARGET_$THISCPU defines so userspace can request
>>>>>> a specific guest CPU flavour; so what does this patch
>>>>>> provide that isn't already provided by just having userspace
>>>>>> query for the "preferred" CPU type as it does already?
>>>>>
>>>>> The way I see this working is that a "generic" CPU cannot be migrated
>>>>> (because we don't know anything about it). If it can be identified as a
>>>>> known (non generic) implementation, then we can migrate it.
>>>>>
>>>> Concretely, how should this work?  Be enforced by userspace or should we
>>>> deny certain SET_ONE_REG operations from working on this target?
>>>
>>> I can definitely see MIDR overriding failing on a generic CPU. Also, you
>>> shouldn't be able to select a generic CPU if we know about the physical CPU.
>>>
>>
>> If I am migrating from an A57 to an A53, but the VM was started as
>> "generic CPU" then we rely on the user discovering that this is not
>> supported when trying to set the MIDR from userspace in KVM?
> 
> A57 to A53 is probably a bad example because we actively support both.
> So let's replace your A57 with an A72. With this patch, the A72 would be
> reported as "generic CPU", and MIDR access would fail on the A53.

Having thought about this a bit more...

It should always be possible to emulate a "known" CPU on a generic host,
and it should be able to migrate. The case we can't migrate is when we
let the guest be generic (which I guess should really be unknown, and
not generic).

So if the user specify "-cpu cortex-a57" on the command line, the guest
should be able to migrate from an A72 to an A53. if the user specified
"-cpu host", the resulting guest won't be able to migrate.

Does it make sense?

Thanks,

	M.
Peter Maydell June 25, 2015, 1:49 p.m. UTC | #13
On 25 June 2015 at 14:44, Marc Zyngier <marc.zyngier@arm.com> wrote:
> It should always be possible to emulate a "known" CPU on a generic host,
> and it should be able to migrate. The case we can't migrate is when we
> let the guest be generic (which I guess should really be unknown, and
> not generic).
>
> So if the user specify "-cpu cortex-a57" on the command line, the guest
> should be able to migrate from an A72 to an A53. if the user specified
> "-cpu host", the resulting guest won't be able to migrate.
>
> Does it make sense?

Yes. We've always said "-cpu host" won't be cross-host migratable
from a QEMU point of view.

-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoffer Dall June 26, 2015, 9:53 a.m. UTC | #14
On Thu, Jun 25, 2015 at 02:49:08PM +0100, Peter Maydell wrote:
> On 25 June 2015 at 14:44, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > It should always be possible to emulate a "known" CPU on a generic host,
> > and it should be able to migrate. The case we can't migrate is when we
> > let the guest be generic (which I guess should really be unknown, and
> > not generic).
> >
> > So if the user specify "-cpu cortex-a57" on the command line, the guest
> > should be able to migrate from an A72 to an A53. if the user specified
> > "-cpu host", the resulting guest won't be able to migrate.
> >
> > Does it make sense?
> 
> Yes. We've always said "-cpu host" won't be cross-host migratable
> from a QEMU point of view.
> 
ok, this makes sense.  It's basically up to userspace to mandate that
trying to migrate something that used unknown cpu (via -cpu host or
whatever) cannot be migrated.

-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chalamarla, Tirumalesh June 29, 2015, 5:13 p.m. UTC | #15
> On Jun 26, 2015, at 2:53 AM, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> 
> On Thu, Jun 25, 2015 at 02:49:08PM +0100, Peter Maydell wrote:
>> On 25 June 2015 at 14:44, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> It should always be possible to emulate a "known" CPU on a generic host,
>>> and it should be able to migrate. The case we can't migrate is when we
>>> let the guest be generic (which I guess should really be unknown, and
>>> not generic).
>>> 
>>> So if the user specify "-cpu cortex-a57" on the command line, the guest
>>> should be able to migrate from an A72 to an A53. if the user specified
>>> "-cpu host", the resulting guest won't be able to migrate.
>>> 
>>> Does it make sense?
>> 
>> Yes. We've always said "-cpu host" won't be cross-host migratable
>> from a QEMU point of view.
>> 
> ok, this makes sense.  It's basically up to userspace to mandate that
> trying to migrate something that used unknown cpu (via -cpu host or
> whatever) cannot be migrated.
> 

Will this also prevents migrating between same implementations, if no how is this identified.
This seems to be making emulation a requirment for ARM64 KVM. 


Thanks,
Tirumalesh. 
 

> -Christoffer
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier June 29, 2015, 5:30 p.m. UTC | #16
On 29/06/15 18:13, Chalamarla, Tirumalesh wrote:
> 
>> On Jun 26, 2015, at 2:53 AM, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>>
>> On Thu, Jun 25, 2015 at 02:49:08PM +0100, Peter Maydell wrote:
>>> On 25 June 2015 at 14:44, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> It should always be possible to emulate a "known" CPU on a generic host,
>>>> and it should be able to migrate. The case we can't migrate is when we
>>>> let the guest be generic (which I guess should really be unknown, and
>>>> not generic).
>>>>
>>>> So if the user specify "-cpu cortex-a57" on the command line, the guest
>>>> should be able to migrate from an A72 to an A53. if the user specified
>>>> "-cpu host", the resulting guest won't be able to migrate.
>>>>
>>>> Does it make sense?
>>>
>>> Yes. We've always said "-cpu host" won't be cross-host migratable
>>> from a QEMU point of view.
>>>
>> ok, this makes sense.  It's basically up to userspace to mandate that
>> trying to migrate something that used unknown cpu (via -cpu host or
>> whatever) cannot be migrated.
>>
> 
> Will this also prevents migrating between same implementations, if no how is this identified.

This shouldn't. It is pretty easy to look at the incoming guest's MIDR,
and verify that it matches the default MIDR on the receiving system. Any
difference, and the migration should abort.

> This seems to be making emulation a requirment for ARM64 KVM. 

I don't believe so. Care to elaborate?

	M.
Peter Maydell June 29, 2015, 5:38 p.m. UTC | #17
On 29 June 2015 at 18:30, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 29/06/15 18:13, Chalamarla, Tirumalesh wrote:
>> Will this also prevents migrating between same implementations,
>> if no how is this identified.
>
> This shouldn't. It is pretty easy to look at the incoming guest's MIDR,
> and verify that it matches the default MIDR on the receiving system. Any
> difference, and the migration should abort.

Do you really want to forbid migration between (say)
Cortex-A57 r3p0 and Cortex-A57 r3p1 ?

That seems pretty harsh.

-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier June 29, 2015, 5:52 p.m. UTC | #18
On 29/06/15 18:38, Peter Maydell wrote:
> On 29 June 2015 at 18:30, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 29/06/15 18:13, Chalamarla, Tirumalesh wrote:
>>> Will this also prevents migrating between same implementations,
>>> if no how is this identified.
>>
>> This shouldn't. It is pretty easy to look at the incoming guest's MIDR,
>> and verify that it matches the default MIDR on the receiving system. Any
>> difference, and the migration should abort.
> 
> Do you really want to forbid migration between (say)
> Cortex-A57 r3p0 and Cortex-A57 r3p1 ?
> 
> That seems pretty harsh.

I think we may need to allow for some flexibility (same major version
only, or +/- 1 minor version...). The idea I'm trying to convey is that
with "generic CPI", migration is not guaranteed unless we're on
extremely similar hardware.

	M.
Chalamarla, Tirumalesh June 29, 2015, 6:39 p.m. UTC | #19
> On Jun 29, 2015, at 10:52 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
> On 29/06/15 18:38, Peter Maydell wrote:
>> On 29 June 2015 at 18:30, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> On 29/06/15 18:13, Chalamarla, Tirumalesh wrote:
>>>> Will this also prevents migrating between same implementations,
>>>> if no how is this identified.
>>> 
>>> This shouldn't. It is pretty easy to look at the incoming guest's MIDR,
>>> and verify that it matches the default MIDR on the receiving system. Any
>>> difference, and the migration should abort.
>> 
>> Do you really want to forbid migration between (say)
>> Cortex-A57 r3p0 and Cortex-A57 r3p1 ?
>> 
>> That seems pretty harsh.
> 
> I think we may need to allow for some flexibility (same major version
> only, or +/- 1 minor version...). The idea I'm trying to convey is that
> with "generic CPI", migration is not guaranteed unless we're on
> extremely similar hardware.
> 
yes, this is the point i am trying to make, we need some flexibility. so that it works with same chips with different passes may be. 
if we are allowing this, then we are not putting emulation as a requirement. 

Thanks,
Tirumalesh. 

> 	M.
> -- 
> Jazz is not dead. It just smells funny...

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier July 3, 2015, 8:08 a.m. UTC | #20
On 02/07/15 21:29, Chalamarla, Tirumalesh wrote:
> is there a chance that this get merged in to 4.2? if not is it possible
> to accept the other patches like adding implementations explicitly(like
> Thunder).

We first need to reach a conclusion on this. Until then, I don't plan to
get anything in. As for 4.2, it feels way too late (the merge window is
almost closed now).

Thanks,

	M.

> 
> Thanks,
> Tirumalesh. 
>> On Jun 29, 2015, at 11:39 AM, Chalamarla, Tirumalesh
>> <Tirumalesh.Chalamarla@caviumnetworks.com
>> <mailto:Tirumalesh.Chalamarla@caviumnetworks.com>> wrote:
>>
>>>
>>> On Jun 29, 2015, at 10:52 AM, Marc Zyngier <marc.zyngier@arm.com
>>> <mailto:marc.zyngier@arm.com>> wrote:
>>>
>>> On 29/06/15 18:38, Peter Maydell wrote:
>>>> On 29 June 2015 at 18:30, Marc Zyngier <marc.zyngier@arm.com
>>>> <mailto:marc.zyngier@arm.com>> wrote:
>>>>> On 29/06/15 18:13, Chalamarla, Tirumalesh wrote:
>>>>>> Will this also prevents migrating between same implementations,
>>>>>> if no how is this identified.
>>>>>
>>>>> This shouldn't. It is pretty easy to look at the incoming guest's MIDR,
>>>>> and verify that it matches the default MIDR on the receiving
>>>>> system. Any
>>>>> difference, and the migration should abort.
>>>>
>>>> Do you really want to forbid migration between (say)
>>>> Cortex-A57 r3p0 and Cortex-A57 r3p1 ?
>>>>
>>>> That seems pretty harsh.
>>>
>>> I think we may need to allow for some flexibility (same major version
>>> only, or +/- 1 minor version...). The idea I'm trying to convey is that
>>> with "generic CPI", migration is not guaranteed unless we're on
>>> extremely similar hardware.
>>>
>> yes, this is the point i am trying to make, we need some flexibility.
>> so that it works with same chips with different passes may be. 
>> if we are allowing this, then we are not putting emulation as a
>> requirement. 
>>
>> Thanks,
>> Tirumalesh. 
>>
>>> M.
>>> -- 
>>> Jazz is not dead. It just smells funny...
>
Peter Maydell July 3, 2015, 8:12 a.m. UTC | #21
On 3 July 2015 at 09:08, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 02/07/15 21:29, Chalamarla, Tirumalesh wrote:
>> is there a chance that this get merged in to 4.2? if not is it possible
>> to accept the other patches like adding implementations explicitly(like
>> Thunder).
>
> We first need to reach a conclusion on this. Until then, I don't plan to
> get anything in. As for 4.2, it feels way too late (the merge window is
> almost closed now).

I would still like to see the proponents of this patch say
what their model is for userspace support of cross-host migration,
if we're abandoning the model the current API envisages.

thanks
-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier July 3, 2015, 8:28 a.m. UTC | #22
On 03/07/15 09:12, Peter Maydell wrote:
> On 3 July 2015 at 09:08, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 02/07/15 21:29, Chalamarla, Tirumalesh wrote:
>>> is there a chance that this get merged in to 4.2? if not is it possible
>>> to accept the other patches like adding implementations explicitly(like
>>> Thunder).
>>
>> We first need to reach a conclusion on this. Until then, I don't plan to
>> get anything in. As for 4.2, it feels way too late (the merge window is
>> almost closed now).
> 
> I would still like to see the proponents of this patch say
> what their model is for userspace support of cross-host migration,
> if we're abandoning the model the current API envisages.

I thought we had discussed this above, and don't really see this as a
departure from the current model:

- "-cpu host" results in "GENERIC" being used: VM can only be migrated
to the exact same HW (no cross-host migration). MIDR should probably
become RO.
- "-cpu host" results in "A57" (for example): VM can be migrated to a
variety of A57 platforms, and allow for some fuzzing on the revision (or
accept any revision).
- "-cpu a57" forces an A57 model to be emulated, always. It is always
possible to migrate such a VM on any host.

I think only the first point is new, but the last two are what we have
(or what we should have).

Does it answer your concerns, or did you have something else in mind?

	M.
Peter Maydell July 3, 2015, 9:34 a.m. UTC | #23
On 3 July 2015 at 09:28, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 03/07/15 09:12, Peter Maydell wrote:
>> I would still like to see the proponents of this patch say
>> what their model is for userspace support of cross-host migration,
>> if we're abandoning the model the current API envisages.
>
> I thought we had discussed this above, and don't really see this as a
> departure from the current model:
>
> - "-cpu host" results in "GENERIC" being used: VM can only be migrated
> to the exact same HW (no cross-host migration). MIDR should probably
> become RO.
> - "-cpu host" results in "A57" (for example): VM can be migrated to a
> variety of A57 platforms, and allow for some fuzzing on the revision (or
> accept any revision).
> - "-cpu a57" forces an A57 model to be emulated, always. It is always
> possible to migrate such a VM on any host.
>
> I think only the first point is new, but the last two are what we have
> (or what we should have).

Right, but the implicit idea of this GENERIC patch seems to
be that new host CPU types don't get their own KVM_ARM_TARGET_*
constant, and are thus forever unable to do cross-host migration.
It's not clear to me why we'd want to have new CPUs be second
class citizens like that.

-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier July 3, 2015, 10:10 a.m. UTC | #24
On 03/07/15 10:34, Peter Maydell wrote:
> On 3 July 2015 at 09:28, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 03/07/15 09:12, Peter Maydell wrote:
>>> I would still like to see the proponents of this patch say
>>> what their model is for userspace support of cross-host migration,
>>> if we're abandoning the model the current API envisages.
>>
>> I thought we had discussed this above, and don't really see this as a
>> departure from the current model:
>>
>> - "-cpu host" results in "GENERIC" being used: VM can only be migrated
>> to the exact same HW (no cross-host migration). MIDR should probably
>> become RO.
>> - "-cpu host" results in "A57" (for example): VM can be migrated to a
>> variety of A57 platforms, and allow for some fuzzing on the revision (or
>> accept any revision).
>> - "-cpu a57" forces an A57 model to be emulated, always. It is always
>> possible to migrate such a VM on any host.
>>
>> I think only the first point is new, but the last two are what we have
>> (or what we should have).
> 
> Right, but the implicit idea of this GENERIC patch seems to
> be that new host CPU types don't get their own KVM_ARM_TARGET_*
> constant, and are thus forever unable to do cross-host migration.
> It's not clear to me why we'd want to have new CPUs be second
> class citizens like that.

I certainly don't want to see *any* CPU be a second class citizen. But
let's face it, we're adding more and more targets that don't implement
anything new, and just satisfy themselves with the generic implementation.

I see it as an incentive to provide something useful (tables of all the
registers with default values?) so that cross-host migration becomes a
reality instead of the figment of our imagination (as it is now). If it
wasn't already ABI, I'd have removed the existing targets until we have
something meaningful to put there.

Now, I also have my own doubts about cross-host migration (timers
anyone?). But I don't see the above as a change in policy. More as a way
to outline the fact that we currently don't have the right level of
information/infrastructure to support it at all.

Thanks,

	M.
Christoffer Dall July 17, 2015, 9:33 a.m. UTC | #25
On Fri, Jul 03, 2015 at 11:10:09AM +0100, Marc Zyngier wrote:
> On 03/07/15 10:34, Peter Maydell wrote:
> > On 3 July 2015 at 09:28, Marc Zyngier <marc.zyngier@arm.com> wrote:
> >> On 03/07/15 09:12, Peter Maydell wrote:
> >>> I would still like to see the proponents of this patch say
> >>> what their model is for userspace support of cross-host migration,
> >>> if we're abandoning the model the current API envisages.
> >>
> >> I thought we had discussed this above, and don't really see this as a
> >> departure from the current model:
> >>
> >> - "-cpu host" results in "GENERIC" being used: VM can only be migrated
> >> to the exact same HW (no cross-host migration). MIDR should probably
> >> become RO.
> >> - "-cpu host" results in "A57" (for example): VM can be migrated to a
> >> variety of A57 platforms, and allow for some fuzzing on the revision (or
> >> accept any revision).
> >> - "-cpu a57" forces an A57 model to be emulated, always. It is always
> >> possible to migrate such a VM on any host.
> >>
> >> I think only the first point is new, but the last two are what we have
> >> (or what we should have).
> > 
> > Right, but the implicit idea of this GENERIC patch seems to
> > be that new host CPU types don't get their own KVM_ARM_TARGET_*
> > constant, and are thus forever unable to do cross-host migration.
> > It's not clear to me why we'd want to have new CPUs be second
> > class citizens like that.
> 
> I certainly don't want to see *any* CPU be a second class citizen. But
> let's face it, we're adding more and more targets that don't implement
> anything new, and just satisfy themselves with the generic implementation.
> 
> I see it as an incentive to provide something useful (tables of all the
> registers with default values?) so that cross-host migration becomes a
> reality instead of the figment of our imagination (as it is now). If it
> wasn't already ABI, I'd have removed the existing targets until we have
> something meaningful to put there.

What we're doing now certainly seems silly, because we're adding kernel
patches without bringing anything to the table...

> 
> Now, I also have my own doubts about cross-host migration (timers
> anyone?). But I don't see the above as a change in policy. More as a way
> to outline the fact that we currently don't have the right level of
> information/infrastructure to support it at all.
> 
The one thing that I've lost track of here (sorry) is whether we're
enforcing the inability to do cross-host migration with the generic
target when this patch is merged or do we leave this up to the graces of
userspace?

Thanks,
-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier July 17, 2015, 9:56 a.m. UTC | #26
On 17/07/15 10:33, Christoffer Dall wrote:
> On Fri, Jul 03, 2015 at 11:10:09AM +0100, Marc Zyngier wrote:
>> On 03/07/15 10:34, Peter Maydell wrote:
>>> On 3 July 2015 at 09:28, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> On 03/07/15 09:12, Peter Maydell wrote:
>>>>> I would still like to see the proponents of this patch say
>>>>> what their model is for userspace support of cross-host migration,
>>>>> if we're abandoning the model the current API envisages.
>>>>
>>>> I thought we had discussed this above, and don't really see this as a
>>>> departure from the current model:
>>>>
>>>> - "-cpu host" results in "GENERIC" being used: VM can only be migrated
>>>> to the exact same HW (no cross-host migration). MIDR should probably
>>>> become RO.
>>>> - "-cpu host" results in "A57" (for example): VM can be migrated to a
>>>> variety of A57 platforms, and allow for some fuzzing on the revision (or
>>>> accept any revision).
>>>> - "-cpu a57" forces an A57 model to be emulated, always. It is always
>>>> possible to migrate such a VM on any host.
>>>>
>>>> I think only the first point is new, but the last two are what we have
>>>> (or what we should have).
>>>
>>> Right, but the implicit idea of this GENERIC patch seems to
>>> be that new host CPU types don't get their own KVM_ARM_TARGET_*
>>> constant, and are thus forever unable to do cross-host migration.
>>> It's not clear to me why we'd want to have new CPUs be second
>>> class citizens like that.
>>
>> I certainly don't want to see *any* CPU be a second class citizen. But
>> let's face it, we're adding more and more targets that don't implement
>> anything new, and just satisfy themselves with the generic implementation.
>>
>> I see it as an incentive to provide something useful (tables of all the
>> registers with default values?) so that cross-host migration becomes a
>> reality instead of the figment of our imagination (as it is now). If it
>> wasn't already ABI, I'd have removed the existing targets until we have
>> something meaningful to put there.
> 
> What we're doing now certainly seems silly, because we're adding kernel
> patches without bringing anything to the table...
> 
>>
>> Now, I also have my own doubts about cross-host migration (timers
>> anyone?). But I don't see the above as a change in policy. More as a way
>> to outline the fact that we currently don't have the right level of
>> information/infrastructure to support it at all.
>>
> The one thing that I've lost track of here (sorry) is whether we're
> enforcing the inability to do cross-host migration with the generic
> target when this patch is merged or do we leave this up to the graces of
> userspace?

The jury is still out on that one.

I was initially not going to enforce anything (after all, this isn't
that different from the whole CNTVOFF story where we allow userspace to
shoot itself in the foot), but I'm equally happy to make MIDR_EL1
read-only if we're creating a generic guest...

Thanks,

	M.
Christoffer Dall July 17, 2015, 10:15 a.m. UTC | #27
On Fri, Jul 17, 2015 at 10:56:39AM +0100, Marc Zyngier wrote:
> On 17/07/15 10:33, Christoffer Dall wrote:
> > On Fri, Jul 03, 2015 at 11:10:09AM +0100, Marc Zyngier wrote:
> >> On 03/07/15 10:34, Peter Maydell wrote:
> >>> On 3 July 2015 at 09:28, Marc Zyngier <marc.zyngier@arm.com> wrote:
> >>>> On 03/07/15 09:12, Peter Maydell wrote:
> >>>>> I would still like to see the proponents of this patch say
> >>>>> what their model is for userspace support of cross-host migration,
> >>>>> if we're abandoning the model the current API envisages.
> >>>>
> >>>> I thought we had discussed this above, and don't really see this as a
> >>>> departure from the current model:
> >>>>
> >>>> - "-cpu host" results in "GENERIC" being used: VM can only be migrated
> >>>> to the exact same HW (no cross-host migration). MIDR should probably
> >>>> become RO.
> >>>> - "-cpu host" results in "A57" (for example): VM can be migrated to a
> >>>> variety of A57 platforms, and allow for some fuzzing on the revision (or
> >>>> accept any revision).
> >>>> - "-cpu a57" forces an A57 model to be emulated, always. It is always
> >>>> possible to migrate such a VM on any host.
> >>>>
> >>>> I think only the first point is new, but the last two are what we have
> >>>> (or what we should have).
> >>>
> >>> Right, but the implicit idea of this GENERIC patch seems to
> >>> be that new host CPU types don't get their own KVM_ARM_TARGET_*
> >>> constant, and are thus forever unable to do cross-host migration.
> >>> It's not clear to me why we'd want to have new CPUs be second
> >>> class citizens like that.
> >>
> >> I certainly don't want to see *any* CPU be a second class citizen. But
> >> let's face it, we're adding more and more targets that don't implement
> >> anything new, and just satisfy themselves with the generic implementation.
> >>
> >> I see it as an incentive to provide something useful (tables of all the
> >> registers with default values?) so that cross-host migration becomes a
> >> reality instead of the figment of our imagination (as it is now). If it
> >> wasn't already ABI, I'd have removed the existing targets until we have
> >> something meaningful to put there.
> > 
> > What we're doing now certainly seems silly, because we're adding kernel
> > patches without bringing anything to the table...
> > 
> >>
> >> Now, I also have my own doubts about cross-host migration (timers
> >> anyone?). But I don't see the above as a change in policy. More as a way
> >> to outline the fact that we currently don't have the right level of
> >> information/infrastructure to support it at all.
> >>
> > The one thing that I've lost track of here (sorry) is whether we're
> > enforcing the inability to do cross-host migration with the generic
> > target when this patch is merged or do we leave this up to the graces of
> > userspace?
> 
> The jury is still out on that one.
> 
> I was initially not going to enforce anything (after all, this isn't
> that different from the whole CNTVOFF story where we allow userspace to
> shoot itself in the foot), but I'm equally happy to make MIDR_EL1
> read-only if we're creating a generic guest...
> 
Looking at the code, midr_el1 is already an invariant register, so isn't
this automagically enforced already?

In that case, I'm fine with merging this patch.

-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier July 17, 2015, 10:19 a.m. UTC | #28
On 17/07/15 11:15, Christoffer Dall wrote:
> On Fri, Jul 17, 2015 at 10:56:39AM +0100, Marc Zyngier wrote:
>> On 17/07/15 10:33, Christoffer Dall wrote:
>>> On Fri, Jul 03, 2015 at 11:10:09AM +0100, Marc Zyngier wrote:
>>>> On 03/07/15 10:34, Peter Maydell wrote:
>>>>> On 3 July 2015 at 09:28, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>>>> On 03/07/15 09:12, Peter Maydell wrote:
>>>>>>> I would still like to see the proponents of this patch say
>>>>>>> what their model is for userspace support of cross-host migration,
>>>>>>> if we're abandoning the model the current API envisages.
>>>>>>
>>>>>> I thought we had discussed this above, and don't really see this as a
>>>>>> departure from the current model:
>>>>>>
>>>>>> - "-cpu host" results in "GENERIC" being used: VM can only be migrated
>>>>>> to the exact same HW (no cross-host migration). MIDR should probably
>>>>>> become RO.
>>>>>> - "-cpu host" results in "A57" (for example): VM can be migrated to a
>>>>>> variety of A57 platforms, and allow for some fuzzing on the revision (or
>>>>>> accept any revision).
>>>>>> - "-cpu a57" forces an A57 model to be emulated, always. It is always
>>>>>> possible to migrate such a VM on any host.
>>>>>>
>>>>>> I think only the first point is new, but the last two are what we have
>>>>>> (or what we should have).
>>>>>
>>>>> Right, but the implicit idea of this GENERIC patch seems to
>>>>> be that new host CPU types don't get their own KVM_ARM_TARGET_*
>>>>> constant, and are thus forever unable to do cross-host migration.
>>>>> It's not clear to me why we'd want to have new CPUs be second
>>>>> class citizens like that.
>>>>
>>>> I certainly don't want to see *any* CPU be a second class citizen. But
>>>> let's face it, we're adding more and more targets that don't implement
>>>> anything new, and just satisfy themselves with the generic implementation.
>>>>
>>>> I see it as an incentive to provide something useful (tables of all the
>>>> registers with default values?) so that cross-host migration becomes a
>>>> reality instead of the figment of our imagination (as it is now). If it
>>>> wasn't already ABI, I'd have removed the existing targets until we have
>>>> something meaningful to put there.
>>>
>>> What we're doing now certainly seems silly, because we're adding kernel
>>> patches without bringing anything to the table...
>>>
>>>>
>>>> Now, I also have my own doubts about cross-host migration (timers
>>>> anyone?). But I don't see the above as a change in policy. More as a way
>>>> to outline the fact that we currently don't have the right level of
>>>> information/infrastructure to support it at all.
>>>>
>>> The one thing that I've lost track of here (sorry) is whether we're
>>> enforcing the inability to do cross-host migration with the generic
>>> target when this patch is merged or do we leave this up to the graces of
>>> userspace?
>>
>> The jury is still out on that one.
>>
>> I was initially not going to enforce anything (after all, this isn't
>> that different from the whole CNTVOFF story where we allow userspace to
>> shoot itself in the foot), but I'm equally happy to make MIDR_EL1
>> read-only if we're creating a generic guest...
>>
> Looking at the code, midr_el1 is already an invariant register, so isn't
> this automagically enforced already?

Ah, you're perfectly right, I has already in that fantasy world where we
can actually migrate VMs across implementations.

> In that case, I'm fine with merging this patch.

Cool. I'll queue that for 4.3.

Thanks,

	M.
Chalamarla, Tirumalesh July 17, 2015, 5:56 p.m. UTC | #29
> On Jul 17, 2015, at 3:19 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
> On 17/07/15 11:15, Christoffer Dall wrote:
>> On Fri, Jul 17, 2015 at 10:56:39AM +0100, Marc Zyngier wrote:
>>> On 17/07/15 10:33, Christoffer Dall wrote:
>>>> On Fri, Jul 03, 2015 at 11:10:09AM +0100, Marc Zyngier wrote:
>>>>> On 03/07/15 10:34, Peter Maydell wrote:
>>>>>> On 3 July 2015 at 09:28, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>>>>> On 03/07/15 09:12, Peter Maydell wrote:
>>>>>>>> I would still like to see the proponents of this patch say
>>>>>>>> what their model is for userspace support of cross-host migration,
>>>>>>>> if we're abandoning the model the current API envisages.
>>>>>>> 
>>>>>>> I thought we had discussed this above, and don't really see this as a
>>>>>>> departure from the current model:
>>>>>>> 
>>>>>>> - "-cpu host" results in "GENERIC" being used: VM can only be migrated
>>>>>>> to the exact same HW (no cross-host migration). MIDR should probably
>>>>>>> become RO.
>>>>>>> - "-cpu host" results in "A57" (for example): VM can be migrated to a
>>>>>>> variety of A57 platforms, and allow for some fuzzing on the revision (or
>>>>>>> accept any revision).
>>>>>>> - "-cpu a57" forces an A57 model to be emulated, always. It is always
>>>>>>> possible to migrate such a VM on any host.
>>>>>>> 
>>>>>>> I think only the first point is new, but the last two are what we have
>>>>>>> (or what we should have).
>>>>>> 
>>>>>> Right, but the implicit idea of this GENERIC patch seems to
>>>>>> be that new host CPU types don't get their own KVM_ARM_TARGET_*
>>>>>> constant, and are thus forever unable to do cross-host migration.
>>>>>> It's not clear to me why we'd want to have new CPUs be second
>>>>>> class citizens like that.
>>>>> 
>>>>> I certainly don't want to see *any* CPU be a second class citizen. But
>>>>> let's face it, we're adding more and more targets that don't implement
>>>>> anything new, and just satisfy themselves with the generic implementation.
>>>>> 
>>>>> I see it as an incentive to provide something useful (tables of all the
>>>>> registers with default values?) so that cross-host migration becomes a
>>>>> reality instead of the figment of our imagination (as it is now). If it
>>>>> wasn't already ABI, I'd have removed the existing targets until we have
>>>>> something meaningful to put there.
>>>> 
>>>> What we're doing now certainly seems silly, because we're adding kernel
>>>> patches without bringing anything to the table...
>>>> 
>>>>> 
>>>>> Now, I also have my own doubts about cross-host migration (timers
>>>>> anyone?). But I don't see the above as a change in policy. More as a way
>>>>> to outline the fact that we currently don't have the right level of
>>>>> information/infrastructure to support it at all.
>>>>> 
>>>> The one thing that I've lost track of here (sorry) is whether we're
>>>> enforcing the inability to do cross-host migration with the generic
>>>> target when this patch is merged or do we leave this up to the graces of
>>>> userspace?
>>> 
>>> The jury is still out on that one.
>>> 
>>> I was initially not going to enforce anything (after all, this isn't
>>> that different from the whole CNTVOFF story where we allow userspace to
>>> shoot itself in the foot), but I'm equally happy to make MIDR_EL1
>>> read-only if we're creating a generic guest...
>>> 
>> Looking at the code, midr_el1 is already an invariant register, so isn't
>> this automagically enforced already?
> 
> Ah, you're perfectly right, I has already in that fantasy world where we
> can actually migrate VMs across implementations.
> 
>> In that case, I'm fine with merging this patch.
> 
> Cool. I'll queue that for 4.3.
> 

this sounds nice. 
> Thanks,
> 
> 	M.
> -- 
> Jazz is not dead. It just smells funny...

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index d268320..f5de418 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -53,14 +53,20 @@  struct kvm_regs {
 	struct user_fpsimd_state fp_regs;
 };
 
-/* Supported Processor Types */
+/*
+ * Supported CPU Targets - Adding a new target type is not recommended,
+ * unless there are some special registers not supported by the
+ * genericv8 syreg table.
+ */
 #define KVM_ARM_TARGET_AEM_V8		0
 #define KVM_ARM_TARGET_FOUNDATION_V8	1
 #define KVM_ARM_TARGET_CORTEX_A57	2
 #define KVM_ARM_TARGET_XGENE_POTENZA	3
 #define KVM_ARM_TARGET_CORTEX_A53	4
+/* Generic ARM v8 target */
+#define KVM_ARM_TARGET_GENERIC_V8	5
 
-#define KVM_ARM_NUM_TARGETS		5
+#define KVM_ARM_NUM_TARGETS		6
 
 /* KVM_ARM_SET_DEVICE_ADDR ioctl id encoding */
 #define KVM_ARM_DEVICE_TYPE_SHIFT	0
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 9535bd5..124aa57 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -293,7 +293,8 @@  int __attribute_const__ kvm_target_cpu(void)
 		break;
 	};
 
-	return -EINVAL;
+	/* Return a default generic target */
+	return KVM_ARM_TARGET_GENERIC_V8;
 }
 
 int kvm_vcpu_preferred_target(struct kvm_vcpu_init *init)
diff --git a/arch/arm64/kvm/sys_regs_generic_v8.c b/arch/arm64/kvm/sys_regs_generic_v8.c
index 475fd29..1e45768 100644
--- a/arch/arm64/kvm/sys_regs_generic_v8.c
+++ b/arch/arm64/kvm/sys_regs_generic_v8.c
@@ -94,6 +94,8 @@  static int __init sys_reg_genericv8_init(void)
 					  &genericv8_target_table);
 	kvm_register_target_sys_reg_table(KVM_ARM_TARGET_XGENE_POTENZA,
 					  &genericv8_target_table);
+	kvm_register_target_sys_reg_table(KVM_ARM_TARGET_GENERIC_V8,
+					  &genericv8_target_table);
 
 	return 0;
 }