| Message ID | 1447753261-7552-21-git-send-email-shannon.zhao@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 17 Nov 2015, shannon.zhao@linaro.org wrote: > From: Shannon Zhao <shannon.zhao@linaro.org> > > ACPI 5.1 only has two explicit methods to boot up SMP, PSCI and Parking > protocol, but the Parking protocol is only specified for ARMv7 now, so > make PSCI as the only way for the SMP boot protocol before some updates > for the ACPI spec or the Parking protocol spec. > > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > Signed-off-by: Parth Dixit <parth.dixit@linaro.org> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> > --- > xen/arch/arm/arm64/smpboot.c | 7 ++++++- > xen/arch/arm/psci.c | 30 +++++++++++++++++++++++------- > xen/arch/arm/smpboot.c | 7 ++++++- > 3 files changed, 35 insertions(+), 9 deletions(-) > > diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c > index 7928f69..93cb6b3 100644 > --- a/xen/arch/arm/arm64/smpboot.c > +++ b/xen/arch/arm/arm64/smpboot.c > @@ -7,6 +7,7 @@ > #include <xen/vmap.h> > #include <asm/io.h> > #include <asm/psci.h> > +#include <asm/acpi.h> > > struct smp_enable_ops { > int (*prepare_cpu)(int); > @@ -96,7 +97,11 @@ static int __init dt_arch_cpu_init(int cpu, struct dt_device_node *dn) > > int __init arch_cpu_init(int cpu, struct dt_device_node *dn) > { > - return dt_arch_cpu_init(cpu, dn); > + if( acpi_disabled ) > + return dt_arch_cpu_init(cpu, dn); > + else > + /* acpi only supports psci at present */ > + return smp_psci_init(cpu); > } > > int __init arch_cpu_up(int cpu) > diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c > index d800cb6..dede0e1 100644 > --- a/xen/arch/arm/psci.c > +++ b/xen/arch/arm/psci.c > @@ -22,6 +22,7 @@ > #include <xen/mm.h> > #include <xen/smp.h> > #include <asm/psci.h> > +#include <asm/acpi.h> > > /* > * While a 64-bit OS can make calls with SMC32 calling conventions, for > @@ -86,6 +87,9 @@ int __init psci_init_0_1(void) > int ret; > const struct dt_device_node *psci; > > + if ( !acpi_disabled ) > + return -EINVAL; It might be worth printing an error message here > psci = dt_find_compatible_node(NULL, NULL, "arm,psci"); > if ( !psci ) > return -EOPNOTSUPP; > @@ -116,15 +120,24 @@ int __init psci_init_0_2(void) > { /* sentinel */ }, > }; > int ret; > - const struct dt_device_node *psci; > > - psci = dt_find_matching_node(NULL, psci_ids); > - if ( !psci ) > - return -EOPNOTSUPP; > + if( acpi_disabled ) > + { > + const struct dt_device_node *psci; > > - ret = psci_is_smc_method(psci); > - if ( ret ) > - return -EINVAL; > + psci = dt_find_matching_node(NULL, psci_ids); > + if ( !psci ) > + return -EOPNOTSUPP; > + > + ret = psci_is_smc_method(psci); > + if ( ret ) > + return -EINVAL; > + } > + else > + { > + if ( acpi_psci_hvc_present() ) > + return -EINVAL; and also here > + } > > psci_ver = call_smc(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0); > > @@ -148,6 +161,9 @@ int __init psci_init(void) > { > int ret; > > + if( !acpi_disabled && !acpi_psci_present() ) > + return -EOPNOTSUPP; > + > ret = psci_init_0_2(); > if ( ret ) > ret = psci_init_0_1(); > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > index d115228..513f1f6 100644 > --- a/xen/arch/arm/smpboot.c > +++ b/xen/arch/arm/smpboot.c > @@ -31,6 +31,7 @@ > #include <xen/console.h> > #include <asm/gic.h> > #include <asm/psci.h> > +#include <asm/acpi.h> > > cpumask_t cpu_online_map; > cpumask_t cpu_present_map; > @@ -247,7 +248,11 @@ void __init smp_init_cpus(void) > return; > } > > - dt_smp_init_cpus(); > + if ( acpi_disabled ) > + dt_smp_init_cpus(); > + else > + acpi_smp_init_cpus(); > + > } > > int __init > -- > 2.1.0 >
Hi Shannon, On 17/11/15 09:40, shannon.zhao@linaro.org wrote: > diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c > index d800cb6..dede0e1 100644 > --- a/xen/arch/arm/psci.c > +++ b/xen/arch/arm/psci.c > @@ -22,6 +22,7 @@ > #include <xen/mm.h> > #include <xen/smp.h> > #include <asm/psci.h> > +#include <asm/acpi.h> > > /* > * While a 64-bit OS can make calls with SMC32 calling conventions, for > @@ -86,6 +87,9 @@ int __init psci_init_0_1(void) > int ret; > const struct dt_device_node *psci; > > + if ( !acpi_disabled ) > + return -EINVAL; Please explain in the commit message why PSCI 0.1 is not supported on ACPI. Regards,
On 2015/11/30 22:57, Julien Grall wrote: > Hi Shannon, > > On 17/11/15 09:40, shannon.zhao@linaro.org wrote: >> > diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c >> > index d800cb6..dede0e1 100644 >> > --- a/xen/arch/arm/psci.c >> > +++ b/xen/arch/arm/psci.c >> > @@ -22,6 +22,7 @@ >> > #include <xen/mm.h> >> > #include <xen/smp.h> >> > #include <asm/psci.h> >> > +#include <asm/acpi.h> >> > >> > /* >> > * While a 64-bit OS can make calls with SMC32 calling conventions, for >> > @@ -86,6 +87,9 @@ int __init psci_init_0_1(void) >> > int ret; >> > const struct dt_device_node *psci; >> > >> > + if ( !acpi_disabled ) >> > + return -EINVAL; > Please explain in the commit message why PSCI 0.1 is not supported on ACPI. Hi, I check this again. There are not limitations of supporting PSCI version in ACPI SPEC. It should support PSCI 0.1 as well. But look at the code of linux kernel, it says it only supports PSCI 0.2+. #define ACPI_FADT_PSCI_COMPLIANT (1) /* 00: [V5+] PSCI 0.2+ is implemented */ So does it need to be consistent with Linux or support PSCI 0.1 in Xen as well? Thanks,
On Wed, 30 Dec 2015, Shannon Zhao wrote: > On 2015/11/30 22:57, Julien Grall wrote: > > Hi Shannon, > > > > On 17/11/15 09:40, shannon.zhao@linaro.org wrote: > >> > diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c > >> > index d800cb6..dede0e1 100644 > >> > --- a/xen/arch/arm/psci.c > >> > +++ b/xen/arch/arm/psci.c > >> > @@ -22,6 +22,7 @@ > >> > #include <xen/mm.h> > >> > #include <xen/smp.h> > >> > #include <asm/psci.h> > >> > +#include <asm/acpi.h> > >> > > >> > /* > >> > * While a 64-bit OS can make calls with SMC32 calling conventions, for > >> > @@ -86,6 +87,9 @@ int __init psci_init_0_1(void) > >> > int ret; > >> > const struct dt_device_node *psci; > >> > > >> > + if ( !acpi_disabled ) > >> > + return -EINVAL; > > Please explain in the commit message why PSCI 0.1 is not supported on ACPI. > > Hi, > > I check this again. There are not limitations of supporting PSCI version > in ACPI SPEC. It should support PSCI 0.1 as well. But look at the code > of linux kernel, it says it only supports PSCI 0.2+. > > #define ACPI_FADT_PSCI_COMPLIANT (1) /* 00: [V5+] PSCI 0.2+ is > implemented */ > > So does it need to be consistent with Linux or support PSCI 0.1 in Xen > as well? I don't think it needs to be consistent with Linux. I would support PSCI 0.1 too.
On Mon, Jan 04, 2016 at 02:51:51PM +0000, Stefano Stabellini wrote: > On Wed, 30 Dec 2015, Shannon Zhao wrote: > > On 2015/11/30 22:57, Julien Grall wrote: > > > Hi Shannon, > > > > > > On 17/11/15 09:40, shannon.zhao@linaro.org wrote: > > >> > diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c > > >> > index d800cb6..dede0e1 100644 > > >> > --- a/xen/arch/arm/psci.c > > >> > +++ b/xen/arch/arm/psci.c > > >> > @@ -22,6 +22,7 @@ > > >> > #include <xen/mm.h> > > >> > #include <xen/smp.h> > > >> > #include <asm/psci.h> > > >> > +#include <asm/acpi.h> > > >> > > > >> > /* > > >> > * While a 64-bit OS can make calls with SMC32 calling conventions, for > > >> > @@ -86,6 +87,9 @@ int __init psci_init_0_1(void) > > >> > int ret; > > >> > const struct dt_device_node *psci; > > >> > > > >> > + if ( !acpi_disabled ) > > >> > + return -EINVAL; > > > Please explain in the commit message why PSCI 0.1 is not supported on ACPI. > > > > Hi, > > > > I check this again. There are not limitations of supporting PSCI version > > in ACPI SPEC. It should support PSCI 0.1 as well. But look at the code > > of linux kernel, it says it only supports PSCI 0.2+. > > > > #define ACPI_FADT_PSCI_COMPLIANT (1) /* 00: [V5+] PSCI 0.2+ is > > implemented */ > > > > So does it need to be consistent with Linux or support PSCI 0.1 in Xen > > as well? > > I don't think it needs to be consistent with Linux. I would support PSCI > 0.1 too. That's not possible, so I don't follow. Prior to 0.2 the function IDs are not defined. The FADT has a single bit which describes PSCI 0.2+ being implemented, and does not describe function IDs. You must assume PSCI 0.2+ in order to have a set of function IDs to use. You should also assume the presence of the rest of the mandatory portions of PSCI 0.2, as these are also required per the combination of the PSCI spec and the ACPI spec. Mark.
On Wed, Dec 30, 2015 at 11:11:08AM +0800, Shannon Zhao wrote: > > > On 2015/11/30 22:57, Julien Grall wrote: > > Hi Shannon, > > > > On 17/11/15 09:40, shannon.zhao@linaro.org wrote: > >> > diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c > >> > index d800cb6..dede0e1 100644 > >> > --- a/xen/arch/arm/psci.c > >> > +++ b/xen/arch/arm/psci.c > >> > @@ -22,6 +22,7 @@ > >> > #include <xen/mm.h> > >> > #include <xen/smp.h> > >> > #include <asm/psci.h> > >> > +#include <asm/acpi.h> > >> > > >> > /* > >> > * While a 64-bit OS can make calls with SMC32 calling conventions, for > >> > @@ -86,6 +87,9 @@ int __init psci_init_0_1(void) > >> > int ret; > >> > const struct dt_device_node *psci; > >> > > >> > + if ( !acpi_disabled ) > >> > + return -EINVAL; > > Please explain in the commit message why PSCI 0.1 is not supported on ACPI. > > Hi, > > I check this again. There are not limitations of supporting PSCI version > in ACPI SPEC. It should support PSCI 0.1 as well. I believe that's an oversight in the ACPI documentation, which should state 0.2+. There was a deliberate decision that the FADT PSCI flag implied PSCI 0.2+, since prior to PSCI 0.2 function IDs were not well-defined, and functions crticial for some uses cases did not exist (e.g. AFFINITY_INFO for kexec-type things). I don't know why that did not make it into the documentation. Charles, thoughts? Mark.
On Mon, Jan 04, 2016 at 03:00:45PM +0000, Mark Rutland wrote: > On Mon, Jan 04, 2016 at 02:51:51PM +0000, Stefano Stabellini wrote: > > On Wed, 30 Dec 2015, Shannon Zhao wrote: > > > I check this again. There are not limitations of supporting PSCI version > > > in ACPI SPEC. It should support PSCI 0.1 as well. But look at the code > > > of linux kernel, it says it only supports PSCI 0.2+. > > > > > > #define ACPI_FADT_PSCI_COMPLIANT (1) /* 00: [V5+] PSCI 0.2+ is > > > implemented */ > > > > > > So does it need to be consistent with Linux or support PSCI 0.1 in Xen > > > as well? > > > > I don't think it needs to be consistent with Linux. I would support PSCI > > 0.1 too. > > That's not possible, so I don't follow. Prior to 0.2 the function IDs > are not defined. > > The FADT has a single bit which describes PSCI 0.2+ being implemented, > and does not describe function IDs. I've now spotted that this wording is indeed missing from the ACPI documentation. I believe this is a documentation bug, as the intent was always for the bit to imply PSCI 0.2+. Mark.
diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c index 7928f69..93cb6b3 100644 --- a/xen/arch/arm/arm64/smpboot.c +++ b/xen/arch/arm/arm64/smpboot.c @@ -7,6 +7,7 @@ #include <xen/vmap.h> #include <asm/io.h> #include <asm/psci.h> +#include <asm/acpi.h> struct smp_enable_ops { int (*prepare_cpu)(int); @@ -96,7 +97,11 @@ static int __init dt_arch_cpu_init(int cpu, struct dt_device_node *dn) int __init arch_cpu_init(int cpu, struct dt_device_node *dn) { - return dt_arch_cpu_init(cpu, dn); + if( acpi_disabled ) + return dt_arch_cpu_init(cpu, dn); + else + /* acpi only supports psci at present */ + return smp_psci_init(cpu); } int __init arch_cpu_up(int cpu) diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c index d800cb6..dede0e1 100644 --- a/xen/arch/arm/psci.c +++ b/xen/arch/arm/psci.c @@ -22,6 +22,7 @@ #include <xen/mm.h> #include <xen/smp.h> #include <asm/psci.h> +#include <asm/acpi.h> /* * While a 64-bit OS can make calls with SMC32 calling conventions, for @@ -86,6 +87,9 @@ int __init psci_init_0_1(void) int ret; const struct dt_device_node *psci; + if ( !acpi_disabled ) + return -EINVAL; + psci = dt_find_compatible_node(NULL, NULL, "arm,psci"); if ( !psci ) return -EOPNOTSUPP; @@ -116,15 +120,24 @@ int __init psci_init_0_2(void) { /* sentinel */ }, }; int ret; - const struct dt_device_node *psci; - psci = dt_find_matching_node(NULL, psci_ids); - if ( !psci ) - return -EOPNOTSUPP; + if( acpi_disabled ) + { + const struct dt_device_node *psci; - ret = psci_is_smc_method(psci); - if ( ret ) - return -EINVAL; + psci = dt_find_matching_node(NULL, psci_ids); + if ( !psci ) + return -EOPNOTSUPP; + + ret = psci_is_smc_method(psci); + if ( ret ) + return -EINVAL; + } + else + { + if ( acpi_psci_hvc_present() ) + return -EINVAL; + } psci_ver = call_smc(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0); @@ -148,6 +161,9 @@ int __init psci_init(void) { int ret; + if( !acpi_disabled && !acpi_psci_present() ) + return -EOPNOTSUPP; + ret = psci_init_0_2(); if ( ret ) ret = psci_init_0_1(); diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c index d115228..513f1f6 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -31,6 +31,7 @@ #include <xen/console.h> #include <asm/gic.h> #include <asm/psci.h> +#include <asm/acpi.h> cpumask_t cpu_online_map; cpumask_t cpu_present_map; @@ -247,7 +248,11 @@ void __init smp_init_cpus(void) return; } - dt_smp_init_cpus(); + if ( acpi_disabled ) + dt_smp_init_cpus(); + else + acpi_smp_init_cpus(); + } int __init