Re: [Xen-devel] [PATCH 6/9] x86/intel_pstate: the main boby of the intel_pstate driver
Hi Julien, On 24/04/2015 20:57, Julien Grall wrote > On 23/04/2016 18:58, Wei Wang wrote: > > diff --git a/xen/include/acpi/cpufreq/processor_perf.h > > b/xen/include/acpi/cpufreq/processor_perf.h > > index d8a1ba6..ebff11d 100644 > > --- a/xen/include/acpi/cpufreq/processor_perf.h > > +++ b/xen/include/acpi/cpufreq/processor_perf.h > > @@ -7,6 +7,7 @@ > > > > #define XEN_PX_INIT 0x8000 > > > > +int intel_pstate_init(void); > > The intel pstate driver is x86 specific. Although xen/include/acpi contains > common headers for common code. Thanks for your comments. But I saw "int powernow_cpufreq_init(void);" is put there. Best, Wei > Can you move the declaration in an x86 specific header (i.e in > xen/include/asm-x86)? > > If I'm not mistaken, you have other patch with similar things in this series. > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/2] Introducing GICv2m Supports
Hi Suravee, Thanks for adding support of GICv2m. I left Linaro at the beginning of the month. Can you use my citrix email (julien.gr...@citrix.com)? On 23/04/2015 09:51, Suravee Suthikulpanit wrote: This patch series introduce GICv2m supports in Xen Dom0. It looks like that the approach taken by this series won't fit for guest (all the "MSI SPIs" are routed to DOM0). Do you have any plan for guest support? This would be necessary if you want to passthrough PCI device on your device. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V2] xen: arm: X-Gene Storm check GIC DIST address for EOI quirk
Hi Pranav,
On 24/04/2015 15:46, Pranavkumar Sawargaonkar wrote:
In old X-Gene Storm firmware and DT, secure mode addresses have been
mentioned in GICv2 node. In this case maintenance interrupt is used
instead of EOI HW method.
This patch checks the GIC Distributor Base Address to enable EOI quirk
for old firmware.
Ref:
http://lists.xen.org/archives/html/xen-devel/2014-07/msg01263.html
ChangeLog:
V2:
- Fine tune interrupt controller node search as per comments on V1 patch
- Incorporating other misc comments on V1.
V1:
- Initial patch.
The changelog should be separated from the commit message by a "---"
follow by a new line. So git automatically will stripped the changelog
when Ian will apply the patch to Xen git.
Tested-by: Christoffer Dall
Signed-off-by: Pranavkumar Sawargaonkar
---
xen/arch/arm/platforms/xgene-storm.c | 42 +-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/platforms/xgene-storm.c
b/xen/arch/arm/platforms/xgene-storm.c
index 1812e5b..c9a6dfc 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
@@ -35,9 +36,46 @@ static u64 reset_addr, reset_size;
static u32 reset_mask;
static bool reset_vals_valid = false;
+#define XGENE_SEC_GICV2_DIST_ADDR0x7801
+static u32 __read_mostly xgene_quirks = PLATFORM_QUIRK_GIC_64K_STRIDE;
+
+static void __init xgene_check_pirq_eoi(void)
+{
+struct dt_device_node *node;
I think this can be const.
+int res;
+paddr_t dbase;
+static const struct dt_device_match xgene_dt_int_ctrl_match[] =
The static is not necessary here.
+{
+DT_MATCH_COMPATIBLE("arm,cortex-a15-gic"),
+{ /*sentinel*/ },
+};
+
+node = dt_find_interrupt_controller(xgene_dt_int_ctrl_match);
+if ( !node )
+panic("%s: Can not find interrupt controller node\n", __func__);
s/Can not/Cannot/ ?
Panic will add a newline. So it's not necessary here.
Although I'm not sure if we should use panic here. All the platform code
is returning an error code which will be handled to an upper function in
the stack (currently it's platform_init).
I don't have a strong opinion on it. I will let the ARM maintainers decide.
+
+res = dt_device_get_address(node, 0, &dbase, NULL);
+if ( !dbase )
+panic("%s: Cannot find a valid address for the "
+"distributor", __func__);
The indentation looks wrong here. Also, I'm not sure why you split the
message. The line won't be longer than 80 columns.
+
+/*
+ * In old X-Gene Storm firmware and DT, secure mode addresses have
+ * been mentioned in GICv2 node. We have to use maintenance interrupt
+ * instead of EOI HW in this case. We check the GIC Distributor Base
+ * Address to maintain compatibility with older firmware.
+ */
+if ( dbase == XGENE_SEC_GICV2_DIST_ADDR )
+{
+xgene_quirks |= PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI;
+printk("Xen: warning: Using OLD X-Gene Firmware,"
+"disabling PIRQ EOI mode ...\n");
Indentation
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 10/19] xen: arm: implement handling of registers trapped by CPTR_EL2.TTA
Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: Add explicit handler for 64-bit CP14 accesses, with more relevant debug message (as per other handlers) and to provide a place for a comment. Signed-off-by: Ian Campbell Reviewed-by: Julien Grall Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 12/19] xen: arm: Annotate the handlers for HSTR_EL2.T15
Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: Signed-off-by: Ian Campbell --- v2: s/Tx/T15/ --- xen/arch/arm/traps.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index a2bae51..86b5655 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1720,6 +1720,11 @@ static void do_cp15_32(struct cpu_user_regs *regs, * ARMv7 (DDI 0406C.b): B1.14.12 * ARMv8 (DDI 0487A.d): N/A * + * HSTR_EL2.T15 + * + * ARMv7 (DDI 0406C.b): B1.14.14 + * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55 + * * And all other unknown registers. */ default: @@ -1758,6 +1763,11 @@ static void do_cp15_64(struct cpu_user_regs *regs, * ARMv7 (DDI 0406C.b): B1.14.12 * ARMv8 (DDI 0487A.d): N/A * + * HSTR_EL2.Tx Should not this Tx switch to T15 too? Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 06/19] xen: arm: add minimum exception level argument to trap handler helpers
Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: Removes a load of boiler plate. Signed-off-by: Ian Campbell Reviewed-by: Julien Grall Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 13/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDRA
Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: DBGDRAR and DBGDSAR are actually two cp or sys registers each, one 32-bit and one 64-bit. The cpregs #define is suffixed "64" and annotations are added to both handlers. MDRAR_EL1 (arm64 version of DBGDRAR) wasn't handled, so add that here. Signed-off-by: Ian Campbell Reviewed-by: Julien Grall Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 04/19] xen: arm: provide and use a handle_raz_wi helper
Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: Reduces the use of goto in the trap handlers to none. Some explicitly 32-bit types become register_t here, but that's OK, on 32-bit they are 32-bit already and on 64-bit it is fine/harmless to set the larger register, a 32-bit guest won't see the top half in any case. Per section B1.2.1 (ARMv8 DDI0487 A.d) writes to wN registers are zero extended, so there is no risk of leaking the top half here. Unlike the previous code the advancing of PC is handled within the helper, rather than after the end of the switch as before. So return as the handler is called. Signed-off-by: Ian Campbell Reviewed-by: Julien Grall Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 14/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDOSA
Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: Gather the affected handlers in a single place per trap type. Add some HSR_SYSREG and AArch32 defines for those registers (because I'd already typed them in when I realised I didn't need them). Signed-off-by: Ian Campbell Reviewed-by: Julien Grall Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 11/19] xen: arm: Annotate handlers for CPTR_EL2.Tx
Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: Also expand on the comment when writing CPTR_EL2 to mention that most of the bits we are setting are RES1 on arm64 anyway. Signed-off-by: Ian Campbell Reviewed-by: Julien Grall Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 08/19] xen: arm: implement handling of ACTLR_EL1 trap
Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: While annotating ACTLR I noticed that we don't appear to handle the 64-bit version of this trap. Do so and annotate everything. Signed-off-by: Ian Campbell Reviewed-by: Julien Grall Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 07/19] xen: arm: Annotate trap handler for HSR_EL2.{TWI, TWE, TSC}
Hi Ian, Subject: s/HSR_EL2/HCR_EL2/ On 17/04/2015 19:01, Ian Campbell wrote: Reference the bit which enables the trap and the section/page which describes what that bit enables. These ones are pretty trivial, included for completeness. Signed-off-by: Ian Campbell Other than the typo in the subject: Reviewed-by: Julien Grall Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 6/9] x86/intel_pstate: the main boby of the intel_pstate driver
Hi Wei, On 23/04/2016 18:58, Wei Wang wrote: diff --git a/xen/include/acpi/cpufreq/processor_perf.h b/xen/include/acpi/cpufreq/processor_perf.h index d8a1ba6..ebff11d 100644 --- a/xen/include/acpi/cpufreq/processor_perf.h +++ b/xen/include/acpi/cpufreq/processor_perf.h @@ -7,6 +7,7 @@ #define XEN_PX_INIT 0x8000 +int intel_pstate_init(void); The intel pstate driver is x86 specific. Although xen/include/acpi contains common headers for common code. Can you move the declaration in an x86 specific header (i.e in xen/include/asm-x86)? If I'm not mistaken, you have other patch with similar things in this series. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 15/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDA
Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: Gather the affected handlers in a single place per trap type. Signed-off-by: Ian Campbell Reviewed-by: Julien Grall Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers
Hi Ian, On 17/04/2015 19:01, Ian Campbell wrote: Signed-off-by: Ian Campbell --- v2: Move last paramter of a handle_ro_raz call to next patch where it belongs. --- xen/arch/arm/traps.c | 52 -- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 8b1846a..b54aef6 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs, advance_pc(regs, hsr); } +/* Write only + write ignore */ [..] +/* Read only + read as zero */ I'm not sure if we finished the discussion on those comment on v1 before you sent the v2. The "+" is very confusing for me because it indicates two parts: write only and write ignore (same for the read). Both part doesn't really fit together. Although this helper clearly choose to implement WO as WI (resp. RO as RAZ). I think this should be clearer in order to avoid people think this can be used for RO but with a different value than 0. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V15 5/9] xen: Make gpfn related memops compatible with wider return values
Hi Jan, On 21/04/2015 20:13, Jan Beulich wrote: For this specific one - is there a reasonable use case? Other than for host PFN, we have control over guest ones, and I'm not sure managing a guest with GPFNs extending past 4 billion can be expected to work if only this one hypercall got fixed. IOW I'm expecting to NAK any such addition without proper rationale. There is hardware coming out with 48 bits address support (i.e 36 bit pfn). Even though the current layout of 64bit address space is using 40 bits IPA, I wouldn't be surprise if we decide to extend it soon (I have in mind PCI passthrough). Without this new hypercall, you rule out the possibility to run the toolstack (included memaccess or any software requiring the maximum PFN used by a domain) in a 32bit domain or 32bit userspace on 64bit domain. I don't have a good use case, but I don't see why we should omit a such possibility. It would be better if we can fix now rather than waiting until someone need it. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [v3 01/10] vmx: add new boot parameter to control PML enabling
On Fri, Apr 24, 2015 at 10:33 PM, Jan Beulich wrote:
On 24.04.15 at 10:19, wrote:
>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>> @@ -64,6 +64,36 @@ integer_param("ple_gap", ple_gap);
>> static unsigned int __read_mostly ple_window = 4096;
>> integer_param("ple_window", ple_window);
>>
>> +static bool_t __read_mostly opt_pml_enabled = 0;
>> +
>> +/*
>> + * The 'ept' parameter controls functionalities that depend on, or impact
>> the
>> + * EPT mechanism. Optional comma separated value may contain:
>> + *
>> + * pml Enable PML
>> + */
>> +static void __init parse_ept_param(char *s)
>> +{
>> +char *ss;
>> +
>> +do {
>> +bool_t val = !!strncmp(s, "no-", 3);
>> +if ( !val )
>
> In case another round is needed, a blank line is missing above.
>
>> +s += 3;
>> +
>> +ss = strchr(s, ',');
>> +if ( ss )
>> +*ss = '\0';
>> +
>> +if ( !strcmp(s, "pml") )
>> +opt_pml_enabled = val;
>> +
>> +s = ss + 1;
>> +} while ( ss );
>> +}
>> +
>> +custom_param("ept", parse_ept_param);
>
> And a superfluous blank line would want to be dropped here.
Sure. Will do both of your above comments if a further v4 is needed. Thanks.
And I suppose you are talking about the blank line before
custom_param("ept", parse_ept_param) ?
Thanks,
-Kai
>
> Jan
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
--
Thanks,
-Kai
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
