| Message ID | 1372840923-12808-1-git-send-email-anup.patel@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 3 July 2013 14:12, Anup Patel <anup.patel@linaro.org> wrote: > Update kvm_target_cpu() to allow Cortex-A57 guest CPU on APM X-Gene. > > Signed-off-by: Anup Patel <anup.patel@linaro.org> > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > --- > arch/arm64/kvm/guest.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c > index 2c3ff67..765f56f 100644 > --- a/arch/arm64/kvm/guest.c > +++ b/arch/arm64/kvm/guest.c > @@ -207,8 +207,13 @@ int __attribute_const__ kvm_target_cpu(void) > unsigned long implementor = read_cpuid_implementor(); > unsigned long part_number = read_cpuid_part_number(); > > - if (implementor != ARM_CPU_IMP_ARM) > - return -EINVAL; > + switch (implementor) { > + case ARM_CPU_IMP_ARM: > + case ARM_CPU_IMP_APM: > + break; > + default: > + return -EINVAL; > + } > > switch (part_number) { > case ARM_CPU_PART_AEM_V8: > @@ -216,6 +221,7 @@ int __attribute_const__ kvm_target_cpu(void) > case ARM_CPU_PART_FOUNDATION: > return KVM_ARM_TARGET_FOUNDATION_V8; > case ARM_CPU_PART_CORTEX_A57: > + case APM_CPU_PART_POTENZA: > /* Currently handled by the generic backend */ > return KVM_ARM_TARGET_CORTEX_A57; > default: > -- > 1.7.9.5 > Hi Catalin/Marc, This patch depends on the patch titled "arm64: Add defines for APM ARMv8 implementation" from Vinyak Kale. Regards, Anup
On 3 July 2013 09:42, Anup Patel <anup.patel@linaro.org> wrote: > Update kvm_target_cpu() to allow Cortex-A57 guest CPU on APM X-Gene. > > Signed-off-by: Anup Patel <anup.patel@linaro.org> > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > --- > arch/arm64/kvm/guest.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c > index 2c3ff67..765f56f 100644 > --- a/arch/arm64/kvm/guest.c > +++ b/arch/arm64/kvm/guest.c > @@ -207,8 +207,13 @@ int __attribute_const__ kvm_target_cpu(void) > unsigned long implementor = read_cpuid_implementor(); > unsigned long part_number = read_cpuid_part_number(); > > - if (implementor != ARM_CPU_IMP_ARM) > - return -EINVAL; > + switch (implementor) { > + case ARM_CPU_IMP_ARM: > + case ARM_CPU_IMP_APM: > + break; > + default: > + return -EINVAL; > + } Doesn't this change mean we now accept the below part numbers for all implementors? That doesn't look right. > switch (part_number) { > case ARM_CPU_PART_AEM_V8: > @@ -216,6 +221,7 @@ int __attribute_const__ kvm_target_cpu(void) > case ARM_CPU_PART_FOUNDATION: > return KVM_ARM_TARGET_FOUNDATION_V8; > case ARM_CPU_PART_CORTEX_A57: > + case APM_CPU_PART_POTENZA: > /* Currently handled by the generic backend */ > return KVM_ARM_TARGET_CORTEX_A57; > default: Do we really model all the system registers and so on correctly sufficiently to be able to present the guest with an A57 vcpu on an APM X-Gene host? (ie without accidentally leaking the host ID registers/system registers/impdef registers to the guest). thanks -- PMM
On 03/07/13 11:54, Peter Maydell wrote: > On 3 July 2013 09:42, Anup Patel <anup.patel@linaro.org> wrote: >> Update kvm_target_cpu() to allow Cortex-A57 guest CPU on APM X-Gene. >> >> Signed-off-by: Anup Patel <anup.patel@linaro.org> >> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> >> --- >> arch/arm64/kvm/guest.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c >> index 2c3ff67..765f56f 100644 >> --- a/arch/arm64/kvm/guest.c >> +++ b/arch/arm64/kvm/guest.c >> @@ -207,8 +207,13 @@ int __attribute_const__ kvm_target_cpu(void) >> unsigned long implementor = read_cpuid_implementor(); >> unsigned long part_number = read_cpuid_part_number(); >> >> - if (implementor != ARM_CPU_IMP_ARM) >> - return -EINVAL; >> + switch (implementor) { >> + case ARM_CPU_IMP_ARM: >> + case ARM_CPU_IMP_APM: >> + break; >> + default: >> + return -EINVAL; >> + } > > Doesn't this change mean we now accept the below part > numbers for all implementors? That doesn't look right. > >> switch (part_number) { >> case ARM_CPU_PART_AEM_V8: >> @@ -216,6 +221,7 @@ int __attribute_const__ kvm_target_cpu(void) >> case ARM_CPU_PART_FOUNDATION: >> return KVM_ARM_TARGET_FOUNDATION_V8; >> case ARM_CPU_PART_CORTEX_A57: >> + case APM_CPU_PART_POTENZA: >> /* Currently handled by the generic backend */ >> return KVM_ARM_TARGET_CORTEX_A57; >> default: > > Do we really model all the system registers and so on correctly > sufficiently to be able to present the guest with an A57 vcpu > on an APM X-Gene host? (ie without accidentally leaking the > host ID registers/system registers/impdef registers to the > guest). There is no such thing in the KVM/arm64 code. The guest sees the real host, and I really don't lie the idea of lying to userspace by pretending that we're going to emulate an A57. Anup: I suggest you rework that patch to present a X-Gene vcpu. You can implement it by declaring a new target and implementing it in terms of the generic backend one if that's convenient. But just pretending this is an A57 is not going to fly, sorry. Cheers, M.
On Wed, Jul 3, 2013 at 4:24 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 3 July 2013 09:42, Anup Patel <anup.patel@linaro.org> wrote: >> Update kvm_target_cpu() to allow Cortex-A57 guest CPU on APM X-Gene. >> >> Signed-off-by: Anup Patel <anup.patel@linaro.org> >> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> >> --- >> arch/arm64/kvm/guest.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c >> index 2c3ff67..765f56f 100644 >> --- a/arch/arm64/kvm/guest.c >> +++ b/arch/arm64/kvm/guest.c >> @@ -207,8 +207,13 @@ int __attribute_const__ kvm_target_cpu(void) >> unsigned long implementor = read_cpuid_implementor(); >> unsigned long part_number = read_cpuid_part_number(); >> >> - if (implementor != ARM_CPU_IMP_ARM) >> - return -EINVAL; >> + switch (implementor) { >> + case ARM_CPU_IMP_ARM: >> + case ARM_CPU_IMP_APM: >> + break; >> + default: >> + return -EINVAL; >> + } > > Doesn't this change mean we now accept the below part > numbers for all implementors? That doesn't look right. Sure, we can use nested switch case instead of two separate switch cases. > >> switch (part_number) { >> case ARM_CPU_PART_AEM_V8: >> @@ -216,6 +221,7 @@ int __attribute_const__ kvm_target_cpu(void) >> case ARM_CPU_PART_FOUNDATION: >> return KVM_ARM_TARGET_FOUNDATION_V8; >> case ARM_CPU_PART_CORTEX_A57: >> + case APM_CPU_PART_POTENZA: >> /* Currently handled by the generic backend */ >> return KVM_ARM_TARGET_CORTEX_A57; >> default: > > Do we really model all the system registers and so on correctly > sufficiently to be able to present the guest with an A57 vcpu > on an APM X-Gene host? (ie without accidentally leaking the > host ID registers/system registers/impdef registers to the > guest). Yes, there are very few APM impdef registers. For now A57 vcpu is not allowed to access these impdef registers. For rest of the registers, A57 vcpu will see: 1. MIDR and MPIDR as expected on a A57 vcpu. 2. Cache configuration related registers will be same as that of underlying host so that cache operations work fine in guest > > thanks > -- PMM > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm --Anup
On Wed, Jul 3, 2013 at 7:29 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > On 03/07/13 11:54, Peter Maydell wrote: >> On 3 July 2013 09:42, Anup Patel <anup.patel@linaro.org> wrote: >>> Update kvm_target_cpu() to allow Cortex-A57 guest CPU on APM X-Gene. >>> >>> Signed-off-by: Anup Patel <anup.patel@linaro.org> >>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> >>> --- >>> arch/arm64/kvm/guest.c | 10 ++++++++-- >>> 1 file changed, 8 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c >>> index 2c3ff67..765f56f 100644 >>> --- a/arch/arm64/kvm/guest.c >>> +++ b/arch/arm64/kvm/guest.c >>> @@ -207,8 +207,13 @@ int __attribute_const__ kvm_target_cpu(void) >>> unsigned long implementor = read_cpuid_implementor(); >>> unsigned long part_number = read_cpuid_part_number(); >>> >>> - if (implementor != ARM_CPU_IMP_ARM) >>> - return -EINVAL; >>> + switch (implementor) { >>> + case ARM_CPU_IMP_ARM: >>> + case ARM_CPU_IMP_APM: >>> + break; >>> + default: >>> + return -EINVAL; >>> + } >> >> Doesn't this change mean we now accept the below part >> numbers for all implementors? That doesn't look right. >> >>> switch (part_number) { >>> case ARM_CPU_PART_AEM_V8: >>> @@ -216,6 +221,7 @@ int __attribute_const__ kvm_target_cpu(void) >>> case ARM_CPU_PART_FOUNDATION: >>> return KVM_ARM_TARGET_FOUNDATION_V8; >>> case ARM_CPU_PART_CORTEX_A57: >>> + case APM_CPU_PART_POTENZA: >>> /* Currently handled by the generic backend */ >>> return KVM_ARM_TARGET_CORTEX_A57; >>> default: >> >> Do we really model all the system registers and so on correctly >> sufficiently to be able to present the guest with an A57 vcpu >> on an APM X-Gene host? (ie without accidentally leaking the >> host ID registers/system registers/impdef registers to the >> guest). > > There is no such thing in the KVM/arm64 code. The guest sees the real > host, and I really don't lie the idea of lying to userspace by > pretending that we're going to emulate an A57. > > Anup: I suggest you rework that patch to present a X-Gene vcpu. You can > implement it by declaring a new target and implementing it in terms of > the generic backend one if that's convenient. But just pretending this > is an A57 is not going to fly, sorry. Sure, I will add separate target vcpu for X-Gene and try to re-use your generic backend. > > Cheers, > > M. > -- > Jazz is not dead. It just smells funny... > > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm --Anup
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 2c3ff67..765f56f 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -207,8 +207,13 @@ int __attribute_const__ kvm_target_cpu(void) unsigned long implementor = read_cpuid_implementor(); unsigned long part_number = read_cpuid_part_number(); - if (implementor != ARM_CPU_IMP_ARM) - return -EINVAL; + switch (implementor) { + case ARM_CPU_IMP_ARM: + case ARM_CPU_IMP_APM: + break; + default: + return -EINVAL; + } switch (part_number) { case ARM_CPU_PART_AEM_V8: @@ -216,6 +221,7 @@ int __attribute_const__ kvm_target_cpu(void) case ARM_CPU_PART_FOUNDATION: return KVM_ARM_TARGET_FOUNDATION_V8; case ARM_CPU_PART_CORTEX_A57: + case APM_CPU_PART_POTENZA: /* Currently handled by the generic backend */ return KVM_ARM_TARGET_CORTEX_A57; default: