Re: [Xen-devel] Linux as 32-bit Dom0?
On 22/11/17 15:48, Jan Beulich wrote: On 22.11.17 at 15:40, wrote: >> On 22/11/17 15:05, Jan Beulich wrote: >>> Jürgen, Boris, >>> >>> am I trying something that's not allowed, but selectable via Kconfig? >>> On system with multiple IO-APICs (I assume that's what triggers the >>> problem) I get >>> >>> Kernel panic - not syncing: Max apic_id exceeded! >> >> Generally I don't think 32 bit dom0 is forbidden, but rarely used. I >> wouldn't be too sad in case we'd decide to drop that support. ;-) >> >> Can you please be a little bit more specific? >> >> How many IOAPICs? From the code I guess this is an INTEL system with not >> too recent IOAPIC versions (<0x14)? >> >> Having a little bit more of the boot log might help, too. > > Full log attached, which should answer all questions. This is > a Haswell system, so not too old an IO-APIC flavor I would say. From this data I can't explain why the system is crashing. Right now I have 3 possible explanations, all could be proofed by adding some printk statements in io_apic_get_unique_id(). Could you please print the value returned by get_physical_broadcast() and the complete apic_id_map right before the panic() call? The possibilities IMHO are: - the LAPIC version is limiting the number of available apicids - apic_id_map is somehow filled up completely with all bits set - a compiler bug leading to a false positive Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/xen: support early interrupts in xen pv guests
Add early interrupt handlers activated by idt_setup_early_handler() to
the handlers supported by Xen pv guests. This will allow for early
WARN() calls not crashing the guest.
Suggested-by: Andy Lutomirski
Signed-off-by: Juergen Gross
---
arch/x86/include/asm/segment.h | 12
arch/x86/mm/extable.c | 4 +++-
arch/x86/xen/enlighten_pv.c| 37 -
arch/x86/xen/xen-asm_64.S | 14 ++
4 files changed, 53 insertions(+), 14 deletions(-)
diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index b20f9d623f9c..8f09012b92e7 100644
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -236,11 +236,23 @@
*/
#define EARLY_IDT_HANDLER_SIZE 9
+/*
+ * xen_early_idt_handler_array is for Xen pv guests: for each entry in
+ * early_idt_handler_array it contains a prequel in the form of
+ * pop %rcx; pop %r11; jmp early_idt_handler_array[i]; summing up to
+ * max 8 bytes.
+ */
+#define XEN_EARLY_IDT_HANDLER_SIZE 8
+
#ifndef __ASSEMBLY__
extern const char
early_idt_handler_array[NUM_EXCEPTION_VECTORS][EARLY_IDT_HANDLER_SIZE];
extern void early_ignore_irq(void);
+#if defined(CONFIG_X86_64) && defined(CONFIG_XEN_PV)
+extern const char
xen_early_idt_handler_array[NUM_EXCEPTION_VECTORS][XEN_EARLY_IDT_HANDLER_SIZE];
+#endif
+
/*
* Load a segment. Fall back on loading the zero segment if something goes
* wrong. This variant assumes that loading zero fully clears the segment.
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 3321b446b66c..88754bfd425f 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -1,6 +1,7 @@
#include
#include
#include
+#include
#include
#include
@@ -212,8 +213,9 @@ void __init early_fixup_exception(struct pt_regs *regs, int
trapnr)
* Old CPUs leave the high bits of CS on the stack
* undefined. I'm not sure which CPUs do this, but at least
* the 486 DX works this way.
+* Xen pv domains are not using the default __KERNEL_CS.
*/
- if (regs->cs != __KERNEL_CS)
+ if (!xen_pv_domain() && regs->cs != __KERNEL_CS)
goto fail;
/*
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 5b2b3f3f6531..f2414c6c5e7c 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -622,7 +622,7 @@ static struct trap_array_entry trap_array[] = {
{ simd_coprocessor_error, xen_simd_coprocessor_error, false },
};
-static bool get_trap_addr(void **addr, unsigned int ist)
+static bool __ref get_trap_addr(void **addr, unsigned int ist)
{
unsigned int nr;
bool ist_okay = false;
@@ -644,6 +644,14 @@ static bool get_trap_addr(void **addr, unsigned int ist)
}
}
+ if (nr == ARRAY_SIZE(trap_array) &&
+ *addr >= (void *)early_idt_handler_array[0] &&
+ *addr < (void *)early_idt_handler_array[NUM_EXCEPTION_VECTORS]) {
+ nr = (*addr - (void *)early_idt_handler_array[0]) /
+EARLY_IDT_HANDLER_SIZE;
+ *addr = (void *)xen_early_idt_handler_array[nr];
+ }
+
if (WARN_ON(ist != 0 && !ist_okay))
return false;
@@ -1262,6 +1270,21 @@ asmlinkage __visible void __init xen_start_kernel(void)
xen_setup_gdt(0);
xen_init_irq_ops();
+
+ /* Let's presume PV guests always boot on vCPU with id 0. */
+ per_cpu(xen_vcpu_id, 0) = 0;
+
+ /*
+* Setup xen_vcpu early because idt_setup_early_handler needs it for
+* local_irq_disable(), irqs_disabled().
+*
+* Don't do the full vcpu_info placement stuff until we have
+* the cpu_possible_mask and a non-dummy shared_info.
+*/
+ xen_vcpu_info_reset(0);
+
+ idt_setup_early_handler();
+
xen_init_capabilities();
#ifdef CONFIG_X86_LOCAL_APIC
@@ -1295,18 +1318,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
*/
acpi_numa = -1;
#endif
- /* Let's presume PV guests always boot on vCPU with id 0. */
- per_cpu(xen_vcpu_id, 0) = 0;
-
- /*
-* Setup xen_vcpu early because start_kernel needs it for
-* local_irq_disable(), irqs_disabled().
-*
-* Don't do the full vcpu_info placement stuff until we have
-* the cpu_possible_mask and a non-dummy shared_info.
-*/
- xen_vcpu_info_reset(0);
-
WARN_ON(xen_cpuhp_setup(xen_cpu_up_prepare_pv, xen_cpu_dead_pv));
local_irq_disable();
diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
index 8a10c9a9e2b5..417b339e5c8e 100644
--- a/arch/x86/xen/xen-asm_64.S
+++ b/arch/x86/xen/xen-asm_64.S
@@ -15,6 +15,7 @@
#include
+#include
#include
.macro xen_pv_trap name
@@ -54,6 +55,19 @@ xen_pv_trap entry_
Re: [Xen-devel] [PATCH] x86/xen: support early interrupts in xen pv guests
Sorry, Andy, forgot to Cc: you...
On 24/11/17 09:42, Juergen Gross wrote:
> Add early interrupt handlers activated by idt_setup_early_handler() to
> the handlers supported by Xen pv guests. This will allow for early
> WARN() calls not crashing the guest.
>
> Suggested-by: Andy Lutomirski
> Signed-off-by: Juergen Gross
> ---
> arch/x86/include/asm/segment.h | 12
> arch/x86/mm/extable.c | 4 +++-
> arch/x86/xen/enlighten_pv.c| 37 -
> arch/x86/xen/xen-asm_64.S | 14 ++
> 4 files changed, 53 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
> index b20f9d623f9c..8f09012b92e7 100644
> --- a/arch/x86/include/asm/segment.h
> +++ b/arch/x86/include/asm/segment.h
> @@ -236,11 +236,23 @@
> */
> #define EARLY_IDT_HANDLER_SIZE 9
>
> +/*
> + * xen_early_idt_handler_array is for Xen pv guests: for each entry in
> + * early_idt_handler_array it contains a prequel in the form of
> + * pop %rcx; pop %r11; jmp early_idt_handler_array[i]; summing up to
> + * max 8 bytes.
> + */
> +#define XEN_EARLY_IDT_HANDLER_SIZE 8
> +
> #ifndef __ASSEMBLY__
>
> extern const char
> early_idt_handler_array[NUM_EXCEPTION_VECTORS][EARLY_IDT_HANDLER_SIZE];
> extern void early_ignore_irq(void);
>
> +#if defined(CONFIG_X86_64) && defined(CONFIG_XEN_PV)
> +extern const char
> xen_early_idt_handler_array[NUM_EXCEPTION_VECTORS][XEN_EARLY_IDT_HANDLER_SIZE];
> +#endif
> +
> /*
> * Load a segment. Fall back on loading the zero segment if something goes
> * wrong. This variant assumes that loading zero fully clears the segment.
> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
> index 3321b446b66c..88754bfd425f 100644
> --- a/arch/x86/mm/extable.c
> +++ b/arch/x86/mm/extable.c
> @@ -1,6 +1,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -212,8 +213,9 @@ void __init early_fixup_exception(struct pt_regs *regs,
> int trapnr)
>* Old CPUs leave the high bits of CS on the stack
>* undefined. I'm not sure which CPUs do this, but at least
>* the 486 DX works this way.
> + * Xen pv domains are not using the default __KERNEL_CS.
>*/
> - if (regs->cs != __KERNEL_CS)
> + if (!xen_pv_domain() && regs->cs != __KERNEL_CS)
> goto fail;
>
> /*
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index 5b2b3f3f6531..f2414c6c5e7c 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -622,7 +622,7 @@ static struct trap_array_entry trap_array[] = {
> { simd_coprocessor_error, xen_simd_coprocessor_error, false },
> };
>
> -static bool get_trap_addr(void **addr, unsigned int ist)
> +static bool __ref get_trap_addr(void **addr, unsigned int ist)
> {
> unsigned int nr;
> bool ist_okay = false;
> @@ -644,6 +644,14 @@ static bool get_trap_addr(void **addr, unsigned int ist)
> }
> }
>
> + if (nr == ARRAY_SIZE(trap_array) &&
> + *addr >= (void *)early_idt_handler_array[0] &&
> + *addr < (void *)early_idt_handler_array[NUM_EXCEPTION_VECTORS]) {
> + nr = (*addr - (void *)early_idt_handler_array[0]) /
> + EARLY_IDT_HANDLER_SIZE;
> + *addr = (void *)xen_early_idt_handler_array[nr];
> + }
> +
> if (WARN_ON(ist != 0 && !ist_okay))
> return false;
>
> @@ -1262,6 +1270,21 @@ asmlinkage __visible void __init xen_start_kernel(void)
> xen_setup_gdt(0);
>
> xen_init_irq_ops();
> +
> + /* Let's presume PV guests always boot on vCPU with id 0. */
> + per_cpu(xen_vcpu_id, 0) = 0;
> +
> + /*
> + * Setup xen_vcpu early because idt_setup_early_handler needs it for
> + * local_irq_disable(), irqs_disabled().
> + *
> + * Don't do the full vcpu_info placement stuff until we have
> + * the cpu_possible_mask and a non-dummy shared_info.
> + */
> + xen_vcpu_info_reset(0);
> +
> + idt_setup_early_handler();
> +
> xen_init_capabilities();
>
> #ifdef CONFIG_X86_LOCAL_APIC
> @@ -1295,18 +1318,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
>*/
> acpi_numa = -1;
> #endif
> - /* Let's presume PV guests always boot on vCPU with id 0. */
> - per_cpu(xen_vcpu_id, 0) = 0;
> -
> - /*
> - * Setup xen_vcpu early because start_kernel needs it for
> - * local_irq_disable
Re: [Xen-devel] Xen PV breakage after IRQ stack code refactoring
On 27/11/17 05:03, Andy Lutomirski wrote:
> On Sun, Nov 26, 2017 at 9:10 AM, Boris Ostrovsky
> wrote:
>> Andy,
>>
>> (Can't find the original patch in my mailbox)
>>
>> This hunk from 1d3e53e8624a ("x86/entry/64: Refactor IRQ stacks and make
>> them NMI-safe")
>>
>>
>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
>> index a9a8027..0d4483a 100644
>> --- a/arch/x86/entry/entry_64.S
>> +++ b/arch/x86/entry/entry_64.S
>> @@ -447,6 +447,59 @@ ENTRY(irq_entries_start)
>> .endr
>> END(irq_entries_start)
>>
>> +.macro DEBUG_ENTRY_ASSERT_IRQS_OFF
>> +#ifdef CONFIG_DEBUG_ENTRY
>> + pushfq
>> + testl $X86_EFLAGS_IF, (%rsp)
>> + jz .Lokay_\@
>> + ud2
>> +.Lokay_\@:
>> + addq $8, %rsp
>> +#endif
>> +.endm
>> +
>>
>> makes Xen PV guests somewhat unhappy because IF flag will be set.
>>
>> I was hoping to use ALTERNATIVE instruction but when we hit this for the
>> first time we haven't rewritten instructions yet. Moving check_bugs() a bit
>> higher helps but because this is common code I don't know how well it will
>> work on other architectures (and, in fact, whether it is even safe on x86 in
>> general, although that can be verified).
>>
>> Another option is to also add a parameter to DEBUG_ENTRY_ASSERT_IRQS_OFF (or
>> to ENTER_IRQ_STACK) from xen_do_hypervisor_callback (which is where the
>> failure happens) but this looks pretty fragile in that it assumes that
>> xen_do_hypervisor_callback is the only place where we use this codepath
>> before alt instructions are set.
>>
>> Any other suggestions?
>
> Do we have a convenient asm way to access the save_fl pvop?
No, but adding it would be pretty straight forward. Its something like:
#define SAVE_FLAGS(clobbers)\
PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_save_fl), clobbers, \
PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE);\
call PARA_INDIRECT(pv_irq_ops+PV_IRQ_save_fl);\
PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
similar to DISABLE_INTERRUPTS(), requiring just the definition of
PV_IRQ_save_fl in asm-offsets.c and the non-pvops definition of
SAVE_FLAGS() in irqflags.h.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCHv2] xen-netfront: remove warning when unloading module
On 23/11/17 15:18, Eduardo Otubo wrote: > v2: > * Replace busy wait with wait_event()/wake_up_all() > * Cannot garantee that at the time xennet_remove is called, the >xen_netback state will not be XenbusStateClosed, so added a >condition for that > * There's a small chance for the xen_netback state is >XenbusStateUnknown by the time the xen_netfront switches to Closed, >so added a condition for that. > > When unloading module xen_netfront from guest, dmesg would output > warning messages like below: > > [ 105.236836] xen:grant_table: WARNING: g.e. 0x903 still in use! > [ 105.236839] deferring g.e. 0x903 (pfn 0x35805) > > This problem relies on netfront and netback being out of sync. By the time > netfront revokes the g.e.'s netback didn't have enough time to free all of > them, hence displaying the warnings on dmesg. > > The trick here is to make netfront to wait until netback frees all the g.e.'s > and only then continue to cleanup for the module removal, and this is done by > manipulating both device states. > > Signed-off-by: Eduardo Otubo Acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags
On 27/11/17 19:05, Boris Ostrovsky wrote:
> Commit 1d3e53e8624a ("x86/entry/64: Refactor IRQ stacks and make
> them NMI-safe") added DEBUG_ENTRY_ASSERT_IRQS_OFF macro that acceses
> eflags using 'pushfq' instruction when testing for IF bit. On PV Xen
> guests looking at IF flag directly will always see it set, resulting
> in 'ud2'.
>
> Introduce SAVE_FLAGS() macro that will use appropriate save_fl pv op
> when running paravirt.
>
> Signed-off-by: Boris Ostrovsky
> ---
> arch/x86/entry/entry_64.S|5 ++---
> arch/x86/include/asm/irqflags.h |3 +++
> arch/x86/include/asm/paravirt.h |9 +
> arch/x86/kernel/asm-offsets_64.c |3 +++
> 4 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index f81d50d..4bb7719 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -466,12 +466,11 @@ END(irq_entries_start)
>
> .macro DEBUG_ENTRY_ASSERT_IRQS_OFF
> #ifdef CONFIG_DEBUG_ENTRY
> - pushfq
> - testl $X86_EFLAGS_IF, (%rsp)
> + SAVE_FLAGS(CLBR_ANY)
> + testl $X86_EFLAGS_IF, %eax
Are you sure %eax is allowed to be modified?
> jz .Lokay_\@
> ud2
> .Lokay_\@:
> - addq $8, %rsp
> #endif
> .endm
>
> diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
> index c8ef23f..7f65f3f 100644
> --- a/arch/x86/include/asm/irqflags.h
> +++ b/arch/x86/include/asm/irqflags.h
> @@ -142,6 +142,9 @@ static inline notrace unsigned long
> arch_local_irq_save(void)
> swapgs; \
> sysretl
>
> +#ifdef CONFIG_DEBUG_ENTRY
> +#define SAVE_FLAGS(x)pushfq
Isn't there a "pop %rax" missing (assuming %rax is allowed to be
modified) ?
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/3] x86/boot: add acpi rsdp address to setup_header
Xen PVH guests receive the address of the RSDP table from Xen. In order
to support booting a Xen PVH guest via grub2 using the standard x86
boot entry we need a way fro grub2 to pass the RSDP address to the
kernel.
For this purpose expand the struct setup_header to hold the physical
address of the RSDP address. Being zero means it isn't specified and
has to be located the legacy way (searching through low memory or
EBDA).
Signed-off-by: Juergen Gross
---
Documentation/x86/boot.txt| 19 +++
arch/x86/boot/header.S| 6 +-
arch/x86/include/uapi/asm/bootparam.h | 1 +
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
index 5e9b826b5f62..a33c224797e4 100644
--- a/Documentation/x86/boot.txt
+++ b/Documentation/x86/boot.txt
@@ -61,6 +61,13 @@ Protocol 2.12: (Kernel 3.8) Added the xloadflags field
and extension fields
to struct boot_params for loading bzImage and ramdisk
above 4G in 64bit.
+Protocol 2.13: (Kernel 3.14) Support 32- and 64-bit flags being set in
+ xloadflags to support booting a 64 bit kernel from 32 bit
+ EFI
+
+Protocol 2.14 (Kernel 4.16) Added acpi_rsdp_addr holding the physical
+ address of the ACPI RSDP table.
+
MEMORY LAYOUT
The traditional memory map for the kernel loader, used for Image or
@@ -197,6 +204,7 @@ Offset Proto NameMeaning
0258/8 2.10+ pref_addressPreferred loading address
0260/4 2.10+ init_size Linear memory required during initialization
0264/4 2.11+ handover_offset Offset of handover entry point
+0268/8 2.14+ acpi_rsdp_addr Physical address of RSDP table
(1) For backwards compatibility, if the setup_sects field contains 0, the
real value is 4.
@@ -744,6 +752,17 @@ Offset/size: 0x264/4
See EFI HANDOVER PROTOCOL below for more details.
+Field name:acpi_rsdp_addr
+Type: write
+Offset/size: 0x268/8
+Protocol: 2.14+
+
+ This field can be set by the boot loader to tell the kernel the
+ physical address of the ACPI RSDP table.
+
+ A value of 0 indicates the kernel should fall back to the standard
+ methods to locate the RSDP (search in EBDA/low memory).
+
THE IMAGE CHECKSUM
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 850b8762e889..e7184127f309 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -300,7 +300,7 @@ _start:
# Part 2 of the header, from the old setup.S
.ascii "HdrS" # header signature
- .word 0x020d # header version number (>= 0x0105)
+ .word 0x020e # header version number (>= 0x0105)
# or else old loadlin-1.5 will fail)
.globl realmode_swtch
realmode_swtch:.word 0, 0# default_switch, SETUPSEG
@@ -558,6 +558,10 @@ pref_address: .quad LOAD_PHYSICAL_ADDR
# preferred load addr
init_size: .long INIT_SIZE # kernel initialization size
handover_offset: .long 0 # Filled in by build.c
+acpi_rsdp_addr:.quad 0 # 64-bit physical
pointer to
+ # ACPI RSDP table, added with
+ # version 2.14
+
# End of setup header #
.section ".entrytext", "ax"
diff --git a/arch/x86/include/uapi/asm/bootparam.h
b/arch/x86/include/uapi/asm/bootparam.h
index afdd5ae0fcc4..5742e433e93e 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -85,6 +85,7 @@ struct setup_header {
__u64 pref_address;
__u32 init_size;
__u32 handover_offset;
+ __u64 acpi_rsdp_addr;
} __attribute__((packed));
struct sys_desc_table {
--
2.12.3
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/3] x86/acpi: take rsdp address for boot params if available
In case the rsdp address in struct boot_params is specified don't try
to find the table by searching, but take the address directly as set
by the boot loader.
Signed-off-by: Juergen Gross
---
drivers/acpi/osl.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 3bb46cb24a99..3b25e2ad7d75 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -45,6 +45,10 @@
#include
#include
+#ifdef CONFIG_X86
+#include
+#endif
+
#include "internal.h"
#define _COMPONENT ACPI_OS_SERVICES
@@ -195,6 +199,10 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
if (acpi_rsdp)
return acpi_rsdp;
#endif
+#ifdef CONFIG_X86
+ if (boot_params.hdr.acpi_rsdp_addr)
+ return boot_params.hdr.acpi_rsdp_addr;
+#endif
if (efi_enabled(EFI_CONFIG_TABLES)) {
if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
--
2.12.3
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 0/3] x86: make rsdp address accessible via boot params
In the non-EFI boot path the ACPI RSDP table is currently found via either EBDA or by searching through low memory for the RSDP magic. This requires the RSDP to be located in the first 1MB of physical memory. Xen PVH guests, however, get the RSDP address via the start of day information block. In order to support an arbitrary RSDP address this patch series adds the physical address of the RSDP to the boot params structure filled by the boot loader. A kernel booted directly in PVH mode can save the RSDP address in the boot params, while a kernel booted in PVH mode via grub can rely on the RSDP address being specified by grub2 (which in turn got the address via the start of day information block from Xen). Juergen Gross (3): x86/boot: add acpi rsdp address to setup_header x86/acpi: take rsdp address for boot params if available x86/xen: supply rsdp address in boot params for pvh guests Documentation/x86/boot.txt| 19 +++ arch/x86/boot/header.S| 6 +- arch/x86/include/uapi/asm/bootparam.h | 1 + arch/x86/xen/enlighten_pvh.c | 2 ++ drivers/acpi/osl.c| 8 5 files changed, 35 insertions(+), 1 deletion(-) -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 3/3] x86/xen: supply rsdp address in boot params for pvh guests
When booted via the special PVH entry save the RSDP address set in the boot information block in struct boot_params. This will enable Xen to locate the RSDP at an arbitrary address. Signed-off-by: Juergen Gross --- arch/x86/xen/enlighten_pvh.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c index 436c4f003e17..0175194f4236 100644 --- a/arch/x86/xen/enlighten_pvh.c +++ b/arch/x86/xen/enlighten_pvh.c @@ -71,6 +71,8 @@ static void __init init_pvh_bootparams(void) */ pvh_bootparams.hdr.version = 0x212; pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */ + + pvh_bootparams.hdr.acpi_rsdp_addr = pvh_start_info.rsdp_paddr; } /* -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/3] x86/acpi: take rsdp address for boot params if available
On 28/11/17 11:18, Roger Pau Monné wrote: > On Tue, Nov 28, 2017 at 10:43:59AM +0100, Juergen Gross wrote: >> In case the rsdp address in struct boot_params is specified don't try >> to find the table by searching, but take the address directly as set >> by the boot loader. >> >> Signed-off-by: Juergen Gross >> --- >> drivers/acpi/osl.c | 8 >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c >> index 3bb46cb24a99..3b25e2ad7d75 100644 >> --- a/drivers/acpi/osl.c >> +++ b/drivers/acpi/osl.c >> @@ -45,6 +45,10 @@ >> #include >> #include >> >> +#ifdef CONFIG_X86 >> +#include >> +#endif >> + >> #include "internal.h" >> >> #define _COMPONENT ACPI_OS_SERVICES >> @@ -195,6 +199,10 @@ acpi_physical_address __init >> acpi_os_get_root_pointer(void) >> if (acpi_rsdp) >> return acpi_rsdp; >> #endif >> +#ifdef CONFIG_X86 >> +if (boot_params.hdr.acpi_rsdp_addr) >> +return boot_params.hdr.acpi_rsdp_addr; >> +#endif > > I'm struggling to figure out how was PVH getting the RSDP previously, > because that should be removed now that it's in the zero-page. I don't think it should be removed, because this was the legacy case (searching through memory). It was pure luck that Xen put it at the right location. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/3] x86/xen: supply rsdp address in boot params for pvh guests
On 28/11/17 11:17, Roger Pau Monné wrote: > On Tue, Nov 28, 2017 at 10:44:00AM +0100, Juergen Gross wrote: >> When booted via the special PVH entry save the RSDP address set in the >> boot information block in struct boot_params. This will enable Xen to >> locate the RSDP at an arbitrary address. >> >> Signed-off-by: Juergen Gross >> --- >> arch/x86/xen/enlighten_pvh.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c >> index 436c4f003e17..0175194f4236 100644 >> --- a/arch/x86/xen/enlighten_pvh.c >> +++ b/arch/x86/xen/enlighten_pvh.c >> @@ -71,6 +71,8 @@ static void __init init_pvh_bootparams(void) >> */ >> pvh_bootparams.hdr.version = 0x212; > > Shouldn't this be 0x20e, like it's set in patch 1? I think it was meant to be 0x20c. But setting it to 0x20e in this patch seems to be a good idea. In the end it doesn't really matter, as hdr.version is meant to be read by the boot loader which is already history when this code is being executed. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags
On 28/11/17 16:28, Boris Ostrovsky wrote:
> Commit 1d3e53e8624a ("x86/entry/64: Refactor IRQ stacks and make
> them NMI-safe") added DEBUG_ENTRY_ASSERT_IRQS_OFF macro that acceses
> eflags using 'pushfq' instruction when testing for IF bit. On PV Xen
> guests looking at IF flag directly will always see it set, resulting
> in 'ud2'.
>
> Introduce SAVE_FLAGS() macro that will use appropriate save_fl pv op
> when running paravirt.
>
> Signed-off-by: Boris Ostrovsky
Reviewed-by: Juergen Gross
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/3] x86: make rsdp address accessible via boot params
On 28/11/17 22:03, Rafael J. Wysocki wrote: > On Tue, Nov 28, 2017 at 10:43 AM, Juergen Gross wrote: >> In the non-EFI boot path the ACPI RSDP table is currently found via >> either EBDA or by searching through low memory for the RSDP magic. >> This requires the RSDP to be located in the first 1MB of physical >> memory. Xen PVH guests, however, get the RSDP address via the start of >> day information block. >> >> In order to support an arbitrary RSDP address this patch series adds >> the physical address of the RSDP to the boot params structure filled >> by the boot loader. A kernel booted directly in PVH mode can save the >> RSDP address in the boot params, while a kernel booted in PVH mode via >> grub can rely on the RSDP address being specified by grub2 (which in >> turn got the address via the start of day information block from Xen). >> >> Juergen Gross (3): >> x86/boot: add acpi rsdp address to setup_header >> x86/acpi: take rsdp address for boot params if available >> x86/xen: supply rsdp address in boot params for pvh guests >> >> Documentation/x86/boot.txt| 19 +++ >> arch/x86/boot/header.S| 6 +- >> arch/x86/include/uapi/asm/bootparam.h | 1 + >> arch/x86/xen/enlighten_pvh.c | 2 ++ >> drivers/acpi/osl.c| 8 >> 5 files changed, 35 insertions(+), 1 deletion(-) >> >> -- > > Is this going to work with all existing setups? I think so, yes. In EFI environment this doesn't matter, direct PVH boot is working, grub2 support is optional (without grub2 support things are working as today). I'm already writing grub2 patches to support booting in PVH environment. Those were the reason I need this patch set, as otherwise Xen would have to put the RSDP into low memory which is limiting the guest's ability to use large page mappings for all its memory. Additionally something like this is needed for PVH dom0 support anyway. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH] KVM: x86: Allow Qemu/KVM to use PVH entry point
On 28/11/17 20:34, Maran Wilson wrote:
> For certain applications it is desirable to rapidly boot a KVM virtual
> machine. In cases where legacy hardware and software support within the
> guest is not needed, Qemu should be able to boot directly into the
> uncompressed Linux kernel binary without the need to run firmware.
>
> There already exists an ABI to allow this for Xen PVH guests and the ABI is
> supported by Linux and FreeBSD:
>
>https://xenbits.xen.org/docs/unstable/misc/hvmlite.html
>
> This PoC patch enables Qemu to use that same entry point for booting KVM
> guests.
>
> Even though the code is still PoC quality, I'm sending this as an RFC now
> since there are a number of different ways the specific implementation
> details can be handled. I chose a shared code path for Xen and KVM guests
> but could just as easily create a separate code path that is advertised by
> a different ELF note for KVM. There also seems to be some flexibility in
> how the e820 table data is passed and how (or if) it should be identified
> as e820 data. As a starting point, I've chosen the options that seem to
> result in the smallest patch with minimal to no changes required of the
> x86/HVM direct boot ABI.
I like the idea.
I'd rather split up the different hypervisor types early and use a
common set of service functions instead of special casing xen_guest
everywhere. This would make it much easier to support the KVM PVH
boot without the need to configure the kernel with CONFIG_XEN.
Another option would be to use the same boot path as with grub: set
the boot params in zeropage and start at startup_32.
Juergen
> ---
> arch/x86/xen/enlighten_pvh.c | 74
>
> 1 file changed, 55 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
> index 98ab176..d93f711 100644
> --- a/arch/x86/xen/enlighten_pvh.c
> +++ b/arch/x86/xen/enlighten_pvh.c
> @@ -31,21 +31,46 @@ static void xen_pvh_arch_setup(void)
> acpi_irq_model = ACPI_IRQ_MODEL_PLATFORM;
> }
>
> -static void __init init_pvh_bootparams(void)
> +static void __init init_pvh_bootparams(bool xen_guest)
> {
> struct xen_memory_map memmap;
> int rc;
>
> memset(&pvh_bootparams, 0, sizeof(pvh_bootparams));
>
> - memmap.nr_entries = ARRAY_SIZE(pvh_bootparams.e820_table);
> - set_xen_guest_handle(memmap.buffer, pvh_bootparams.e820_table);
> - rc = HYPERVISOR_memory_op(XENMEM_memory_map, &memmap);
> - if (rc) {
> - xen_raw_printk("XENMEM_memory_map failed (%d)\n", rc);
> - BUG();
> + if (xen_guest) {
> + memmap.nr_entries = ARRAY_SIZE(pvh_bootparams.e820_table);
> + set_xen_guest_handle(memmap.buffer, pvh_bootparams.e820_table);
> + rc = HYPERVISOR_memory_op(XENMEM_memory_map, &memmap);
> + if (rc) {
> + xen_raw_printk("XENMEM_memory_map failed (%d)\n", rc);
> + BUG();
> + }
> + pvh_bootparams.e820_entries = memmap.nr_entries;
> + } else if (pvh_start_info.nr_modules > 1) {
> + /* The second module should be the e820 data for KVM guests */
> + struct hvm_modlist_entry *modaddr;
> + char e820_sig[] = "e820 data";
> + struct boot_e820_entry *ep;
> + struct e820_table *tp;
> + char *cmdline_str;
> + int idx;
> +
> + modaddr = __va(pvh_start_info.modlist_paddr +
> +sizeof(struct hvm_modlist_entry));
> + cmdline_str = __va(modaddr->cmdline_paddr);
> +
> + if ((modaddr->cmdline_paddr) &&
> + (!strncmp(e820_sig, cmdline_str, sizeof(e820_sig {
> + tp = __va(modaddr->paddr);
> + ep = (struct boot_e820_entry *)tp->entries;
> +
> + pvh_bootparams.e820_entries = tp->nr_entries;
> +
> + for (idx = 0; idx < tp->nr_entries ; idx++, ep++)
> + pvh_bootparams.e820_table[idx] = *ep;
> + }
> }
> - pvh_bootparams.e820_entries = memmap.nr_entries;
>
> if (pvh_bootparams.e820_entries < E820_MAX_ENTRIES_ZEROPAGE - 1) {
> pvh_bootparams.e820_table[pvh_bootparams.e820_entries].addr =
> @@ -55,8 +80,9 @@ static void __init init_pvh_bootparams(void)
> pvh_bootparams.e820_table[pvh_bootparams.e820_entries].type =
> E820_TYPE_RESERVED;
> pvh_bootparams.e820_entries++;
> - } else
> + } else if (xen_guest) {
> xen_raw_printk("Warning: Can fit ISA range into e820\n");
> + }
>
> pvh_bootparams.hdr.cmd_line_ptr =
> pvh_start_info.cmdline_paddr;
> @@ -76,7 +102,7 @@ static void __init init_pvh_bootparams(void)
>* environment (i.e. hardware_subarch 0).
>*/
> pvh_bootparams.hdr.vers
[Xen-devel] [PATCH 2/8] loader/linux: support passing rsdp address via boot params
Xen PVH guests will have the RSDP at an arbitrary address. Support that
by passing the RSDP address via the boot parameters to Linux.
Signed-off-by: Juergen Gross
---
grub-core/loader/i386/linux.c | 6 ++
include/grub/i386/linux.h | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
index 083f9417c..14722d059 100644
--- a/grub-core/loader/i386/linux.c
+++ b/grub-core/loader/i386/linux.c
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
GRUB_MOD_LICENSE ("GPLv3+");
@@ -793,6 +794,11 @@ grub_cmd_linux (grub_command_t cmd __attribute__
((unused)),
prot_init_space = page_align (prot_size) * 3;
}
+#ifdef GRUB_KERNEL_USE_RSDP_ADDR
+ if (grub_le_to_cpu16 (lh.version) >= 0x020e)
+lh.acpi_rsdp_addr = grub_le_to_cpu64 (grub_rsdp_addr);
+#endif
+
if (allocate_pages (prot_size, &align,
min_align, relocatable,
preferred_address))
diff --git a/include/grub/i386/linux.h b/include/grub/i386/linux.h
index da0ca3b83..7194e8297 100644
--- a/include/grub/i386/linux.h
+++ b/include/grub/i386/linux.h
@@ -84,7 +84,7 @@ enum
GRUB_VIDEO_LINUX_TYPE_SIMPLE = 0x70/* Linear framebuffer without any
additional functions. */
};
-/* For the Linux/i386 boot protocol version 2.10. */
+/* For the Linux/i386 boot protocol version 2.14. */
struct linux_kernel_header
{
grub_uint8_t code1[0x0020];
@@ -139,6 +139,8 @@ struct linux_kernel_header
grub_uint64_t setup_data;
grub_uint64_t pref_address;
grub_uint32_t init_size;
+ grub_uint32_t handover_offset;
+ grub_uint64_t acpi_rsdp_addr;
} GRUB_PACKED;
/* Boot parameters for Linux based on 2.6.12. This is used by the setup
--
2.12.3
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 0/8] xen: add pvh guest support
This patch series adds support for booting Linux as PVH guest. Similar to i386/xen and x86_64/xen platforms the new i386/xenpvh platform grub is booted as a standalone image directly by Xen. For booting Linux kernel it is using the standard linux kernel loader. The only modification of the linux loader is to pass the ACPI RSDP address via boot parameters to the kernel, as that table might not be located at the usual physical address just below 1MB. As the related Linux kernel patches are not yet accepted please wait for this to happen before applying the series. Juergen Gross (8): xen: add some xen headers loader/linux: support passing rsdp address via boot params xen: carve out grant tab initialization into dedicated function xen: add xen pvh guest support to grub-core xenpvh: add build runes for grub-core xenpvh: support building a standalone image xenpvh: support grub-install for xenpvh xenpvh: add support to configure configure.ac | 3 + gentpl.py | 4 +- grub-core/Makefile.am | 12 + grub-core/Makefile.core.def | 35 ++ grub-core/kern/i386/tsc.c | 2 +- grub-core/kern/i386/xen/pvh.c | 344 ++ grub-core/kern/i386/xen/startup_pvh.S | 80 grub-core/kern/xen/init.c | 101 -- grub-core/loader/i386/linux.c | 6 + include/grub/i386/linux.h | 4 +- include/grub/i386/pc/int.h| 3 + include/grub/i386/tsc.h | 2 +- include/grub/i386/xen/hypercall.h | 5 +- include/grub/i386/xenpvh/boot.h | 1 + include/grub/i386/xenpvh/console.h| 1 + include/grub/i386/xenpvh/int.h| 1 + include/grub/i386/xenpvh/kernel.h | 30 ++ include/grub/i386/xenpvh/memory.h | 54 +++ include/grub/i386/xenpvh/time.h | 1 + include/grub/kernel.h | 4 +- include/grub/offsets.h| 3 + include/grub/util/install.h | 1 + include/grub/util/mkimage.h | 3 +- include/grub/xen.h| 6 + include/xen/hvm/hvm_op.h | 296 +++ include/xen/hvm/params.h | 284 +++ include/xen/hvm/start_info.h | 98 + include/xen/memory.h | 665 ++ include/xen/physdev.h | 387 include/xen/trace.h | 339 + include/xen/xen.h | 104 -- util/grub-install-common.c| 1 + util/grub-install.c | 7 + util/grub-mkimage32.c | 1 + util/grub-mkimage64.c | 1 + util/grub-mkimagexx.c | 44 ++- util/mkimage.c| 23 +- 37 files changed, 2872 insertions(+), 84 deletions(-) create mode 100644 grub-core/kern/i386/xen/pvh.c create mode 100644 grub-core/kern/i386/xen/startup_pvh.S create mode 100644 include/grub/i386/xenpvh/boot.h create mode 100644 include/grub/i386/xenpvh/console.h create mode 100644 include/grub/i386/xenpvh/int.h create mode 100644 include/grub/i386/xenpvh/kernel.h create mode 100644 include/grub/i386/xenpvh/memory.h create mode 100644 include/grub/i386/xenpvh/time.h create mode 100644 include/xen/hvm/hvm_op.h create mode 100644 include/xen/hvm/params.h create mode 100644 include/xen/hvm/start_info.h create mode 100644 include/xen/memory.h create mode 100644 include/xen/physdev.h create mode 100644 include/xen/trace.h -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 8/8] xenpvh: add support to configure
Support platform i386/xenpvh in configure.
Signed-off-by: Juergen Gross
---
configure.ac | 3 +++
1 file changed, 3 insertions(+)
diff --git a/configure.ac b/configure.ac
index c7888e40f..d987d3379 100644
--- a/configure.ac
+++ b/configure.ac
@@ -147,6 +147,7 @@ case "$target_cpu"-"$platform" in
i386-efi) ;;
x86_64-efi) ;;
i386-xen) ;;
+ i386-xenpvh) ;;
x86_64-xen) ;;
i386-pc) ;;
i386-multiboot) ;;
@@ -215,6 +216,7 @@ case "$platform" in
multiboot) machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_MULTIBOOT=1"
;;
efi) machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_EFI=1" ;;
xen) machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_XEN=1" ;;
+ xenpvh) machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_XENPVH=1" ;;
ieee1275)machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_IEEE1275=1"
;;
uboot) machine_CPPFLAGS="$machine_CPPFLAGS -DGRUB_MACHINE_UBOOT=1" ;;
qemu)machine_CPPFLAGS="$machine_CPPFLAGS
-DGRUB_MACHINE_QEMU=1" ;;
@@ -1908,6 +1910,7 @@ AM_CONDITIONAL([COND_i386_coreboot], [test x$target_cpu =
xi386 -a x$platform =
AM_CONDITIONAL([COND_i386_multiboot], [test x$target_cpu = xi386 -a x$platform
= xmultiboot])
AM_CONDITIONAL([COND_x86_64_efi], [test x$target_cpu = xx86_64 -a x$platform =
xefi])
AM_CONDITIONAL([COND_i386_xen], [test x$target_cpu = xi386 -a x$platform =
xxen])
+AM_CONDITIONAL([COND_i386_xenpvh], [test x$target_cpu = xi386 -a x$platform =
xxenpvh])
AM_CONDITIONAL([COND_x86_64_xen], [test x$target_cpu = xx86_64 -a x$platform =
xxen])
AM_CONDITIONAL([COND_mips_loongson], [test x$target_cpu = xmipsel -a
x$platform = xloongson])
AM_CONDITIONAL([COND_mips_qemu_mips], [test "(" x$target_cpu = xmips -o
x$target_cpu = xmipsel ")" -a x$platform = xqemu_mips])
--
2.12.3
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 7/8] xenpvh: support grub-install for xenpvh
Add xenpvh support to grub-install.
Signed-off-by: Juergen Gross
---
include/grub/util/install.h | 1 +
util/grub-install-common.c | 1 +
util/grub-install.c | 7 +++
3 files changed, 9 insertions(+)
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 5910b0c09..faddbacc6 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -100,6 +100,7 @@ enum grub_install_plat
GRUB_INSTALL_PLATFORM_MIPS_QEMU_MIPS,
GRUB_INSTALL_PLATFORM_I386_XEN,
GRUB_INSTALL_PLATFORM_X86_64_XEN,
+GRUB_INSTALL_PLATFORM_I386_XENPVH,
GRUB_INSTALL_PLATFORM_ARM64_EFI,
GRUB_INSTALL_PLATFORM_ARM_COREBOOT,
GRUB_INSTALL_PLATFORM_MAX
diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index 9e3e358c9..12b7aec6a 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -662,6 +662,7 @@ static struct
[GRUB_INSTALL_PLATFORM_X86_64_EFI] = { "x86_64", "efi" },
[GRUB_INSTALL_PLATFORM_I386_XEN] = { "i386","xen" },
[GRUB_INSTALL_PLATFORM_X86_64_XEN] = { "x86_64", "xen" },
+[GRUB_INSTALL_PLATFORM_I386_XENPVH] = { "i386","xenpvh"},
[GRUB_INSTALL_PLATFORM_MIPSEL_LOONGSON] = { "mipsel", "loongson" },
[GRUB_INSTALL_PLATFORM_MIPSEL_QEMU_MIPS] = { "mipsel", "qemu_mips" },
[GRUB_INSTALL_PLATFORM_MIPS_QEMU_MIPS] = { "mips","qemu_mips" },
diff --git a/util/grub-install.c b/util/grub-install.c
index ef912f5dd..212bc1f38 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -496,6 +496,7 @@ have_bootdev (enum grub_install_plat pl)
case GRUB_INSTALL_PLATFORM_I386_XEN:
case GRUB_INSTALL_PLATFORM_X86_64_XEN:
+case GRUB_INSTALL_PLATFORM_I386_XENPVH:
return 0;
/* pacify warning. */
@@ -913,6 +914,7 @@ main (int argc, char *argv[])
case GRUB_INSTALL_PLATFORM_ARM_UBOOT:
case GRUB_INSTALL_PLATFORM_I386_XEN:
case GRUB_INSTALL_PLATFORM_X86_64_XEN:
+case GRUB_INSTALL_PLATFORM_I386_XENPVH:
break;
case GRUB_INSTALL_PLATFORM_I386_QEMU:
@@ -960,6 +962,7 @@ main (int argc, char *argv[])
case GRUB_INSTALL_PLATFORM_MIPS_QEMU_MIPS:
case GRUB_INSTALL_PLATFORM_I386_XEN:
case GRUB_INSTALL_PLATFORM_X86_64_XEN:
+case GRUB_INSTALL_PLATFORM_I386_XENPVH:
free (install_device);
install_device = NULL;
break;
@@ -1477,6 +1480,7 @@ main (int argc, char *argv[])
case GRUB_INSTALL_PLATFORM_ARM_UBOOT:
case GRUB_INSTALL_PLATFORM_I386_XEN:
case GRUB_INSTALL_PLATFORM_X86_64_XEN:
+ case GRUB_INSTALL_PLATFORM_I386_XENPVH:
grub_util_warn ("%s", _("no hints available for your
platform. Expect reduced performance"));
break;
/* pacify warning. */
@@ -1568,6 +1572,7 @@ main (int argc, char *argv[])
case GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275:
case GRUB_INSTALL_PLATFORM_I386_XEN:
case GRUB_INSTALL_PLATFORM_X86_64_XEN:
+case GRUB_INSTALL_PLATFORM_I386_XENPVH:
core_name = "core.elf";
snprintf (mkimage_target, sizeof (mkimage_target),
"%s-%s",
@@ -1660,6 +1665,7 @@ main (int argc, char *argv[])
case GRUB_INSTALL_PLATFORM_SPARC64_IEEE1275:
case GRUB_INSTALL_PLATFORM_I386_XEN:
case GRUB_INSTALL_PLATFORM_X86_64_XEN:
+case GRUB_INSTALL_PLATFORM_I386_XENPVH:
break;
/* pacify warning. */
case GRUB_INSTALL_PLATFORM_MAX:
@@ -1918,6 +1924,7 @@ main (int argc, char *argv[])
case GRUB_INSTALL_PLATFORM_I386_QEMU:
case GRUB_INSTALL_PLATFORM_I386_XEN:
case GRUB_INSTALL_PLATFORM_X86_64_XEN:
+case GRUB_INSTALL_PLATFORM_I386_XENPVH:
grub_util_warn ("%s",
_("WARNING: no platform-specific install was performed"));
break;
--
2.12.3
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/8] xen: add some xen headers
In order to support grub2 in Xen PVH environment some additional Xen
headers are needed. Add them.
Signed-off-by: Juergen Gross
---
include/xen/hvm/hvm_op.h | 296 +++
include/xen/hvm/params.h | 284 ++
include/xen/hvm/start_info.h | 98 +++
include/xen/memory.h | 665 +++
include/xen/physdev.h| 387 +
include/xen/trace.h | 339 ++
include/xen/xen.h| 104 +--
7 files changed, 2142 insertions(+), 31 deletions(-)
create mode 100644 include/xen/hvm/hvm_op.h
create mode 100644 include/xen/hvm/params.h
create mode 100644 include/xen/hvm/start_info.h
create mode 100644 include/xen/memory.h
create mode 100644 include/xen/physdev.h
create mode 100644 include/xen/trace.h
diff --git a/include/xen/hvm/hvm_op.h b/include/xen/hvm/hvm_op.h
new file mode 100644
index 0..0bdafdf59
--- /dev/null
+++ b/include/xen/hvm/hvm_op.h
@@ -0,0 +1,296 @@
+/*
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (c) 2007, Keir Fraser
+ */
+
+#ifndef __XEN_PUBLIC_HVM_HVM_OP_H__
+#define __XEN_PUBLIC_HVM_HVM_OP_H__
+
+#include "../xen.h"
+#include "../trace.h"
+#include "../event_channel.h"
+
+/* Get/set subcommands: extra argument == pointer to xen_hvm_param struct. */
+#define HVMOP_set_param 0
+#define HVMOP_get_param 1
+struct xen_hvm_param {
+domid_t domid;/* IN */
+uint32_t index;/* IN */
+uint64_t value;/* IN/OUT */
+};
+typedef struct xen_hvm_param xen_hvm_param_t;
+DEFINE_XEN_GUEST_HANDLE(xen_hvm_param_t);
+
+#if __XEN_INTERFACE_VERSION__ < 0x00040900
+
+/* Set the logical level of one of a domain's PCI INTx wires. */
+#define HVMOP_set_pci_intx_level 2
+struct xen_hvm_set_pci_intx_level {
+/* Domain to be updated. */
+domid_t domid;
+/* PCI INTx identification in PCI topology (domain:bus:device:intx). */
+uint8_t domain, bus, device, intx;
+/* Assertion level (0 = unasserted, 1 = asserted). */
+uint8_t level;
+};
+typedef struct xen_hvm_set_pci_intx_level xen_hvm_set_pci_intx_level_t;
+DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_pci_intx_level_t);
+
+/* Set the logical level of one of a domain's ISA IRQ wires. */
+#define HVMOP_set_isa_irq_level 3
+struct xen_hvm_set_isa_irq_level {
+/* Domain to be updated. */
+domid_t domid;
+/* ISA device identification, by ISA IRQ (0-15). */
+uint8_t isa_irq;
+/* Assertion level (0 = unasserted, 1 = asserted). */
+uint8_t level;
+};
+typedef struct xen_hvm_set_isa_irq_level xen_hvm_set_isa_irq_level_t;
+DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_isa_irq_level_t);
+
+#define HVMOP_set_pci_link_route 4
+struct xen_hvm_set_pci_link_route {
+/* Domain to be updated. */
+domid_t domid;
+/* PCI link identifier (0-3). */
+uint8_t link;
+/* ISA IRQ (1-15), or 0 (disable link). */
+uint8_t isa_irq;
+};
+typedef struct xen_hvm_set_pci_link_route xen_hvm_set_pci_link_route_t;
+DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_pci_link_route_t);
+
+#endif /* __XEN_INTERFACE_VERSION__ < 0x00040900 */
+
+/* Flushes all VCPU TLBs: @arg must be NULL. */
+#define HVMOP_flush_tlbs 5
+
+typedef enum {
+HVMMEM_ram_rw, /* Normal read/write guest RAM */
+HVMMEM_ram_ro, /* Read-only; writes are discarded */
+HVMMEM_mmio_dm,/* Reads and write go to the device model */
+#if __XEN_INTERFACE_VERSION__ < 0x00040700
+HVMMEM_mmio_write_dm, /* Read-only; writes go to the device model */
+#else
+HVMMEM_unused, /* Placeholder; setting memory to this type
+ will fail for code after 4.7.0 */
+#endif
+HVMMEM_ioreq_server/* Memory type claimed by an ioreq server; type
+ changes to th
[Xen-devel] [PATCH 6/8] xenpvh: support building a standalone image
Suppor mkimage for xenpvh.
Signed-off-by: Juergen Gross
---
include/grub/util/mkimage.h | 3 ++-
util/grub-mkimage32.c | 1 +
util/grub-mkimage64.c | 1 +
util/grub-mkimagexx.c | 44
util/mkimage.c | 23 ++-
5 files changed, 66 insertions(+), 6 deletions(-)
diff --git a/include/grub/util/mkimage.h b/include/grub/util/mkimage.h
index b3a5ca132..3f5bc2e00 100644
--- a/include/grub/util/mkimage.h
+++ b/include/grub/util/mkimage.h
@@ -71,7 +71,8 @@ struct grub_install_image_target_desc
IMAGE_I386_IEEE1275,
IMAGE_LOONGSON_ELF, IMAGE_QEMU, IMAGE_PPC, IMAGE_YEELOONG_FLASH,
IMAGE_FULOONG2F_FLASH, IMAGE_I386_PC_PXE, IMAGE_MIPS_ARC,
-IMAGE_QEMU_MIPS_FLASH, IMAGE_UBOOT, IMAGE_XEN, IMAGE_I386_PC_ELTORITO
+IMAGE_QEMU_MIPS_FLASH, IMAGE_UBOOT, IMAGE_XEN, IMAGE_I386_PC_ELTORITO,
+IMAGE_XENPVH
} id;
enum
{
diff --git a/util/grub-mkimage32.c b/util/grub-mkimage32.c
index 9b31397bc..4253c4897 100644
--- a/util/grub-mkimage32.c
+++ b/util/grub-mkimage32.c
@@ -18,5 +18,6 @@
# define ELF_R_TYPE(val) ELF32_R_TYPE(val)
# define ELF_ST_TYPE(val) ELF32_ST_TYPE(val)
#define XEN_NOTE_SIZE 132
+#define XENPVH_NOTE_SIZE 20
#include "grub-mkimagexx.c"
diff --git a/util/grub-mkimage64.c b/util/grub-mkimage64.c
index d83345924..c862be8c0 100644
--- a/util/grub-mkimage64.c
+++ b/util/grub-mkimage64.c
@@ -18,5 +18,6 @@
# define ELF_R_TYPE(val) ELF64_R_TYPE(val)
# define ELF_ST_TYPE(val) ELF64_ST_TYPE(val)
#define XEN_NOTE_SIZE 120
+#define XENPVH_NOTE_SIZE 24
#include "grub-mkimagexx.c"
diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
index a2bb05439..a024e57b6 100644
--- a/util/grub-mkimagexx.c
+++ b/util/grub-mkimagexx.c
@@ -207,12 +207,12 @@ SUFFIX (grub_mkimage_generate_elf) (const struct
grub_install_image_target_desc
phnum++;
footer_size += sizeof (struct grub_ieee1275_note);
}
- if (image_target->id == IMAGE_XEN)
+ if (image_target->id == IMAGE_XEN || image_target->id == IMAGE_XENPVH)
{
phnum++;
shnum++;
string_size += sizeof (".xen");
- footer_size += XEN_NOTE_SIZE;
+ footer_size += (image_target->id == IMAGE_XEN) ? XEN_NOTE_SIZE :
XENPVH_NOTE_SIZE;
}
header_size = ALIGN_UP (sizeof (*ehdr) + phnum * sizeof (*phdr)
+ shnum * sizeof (*shdr) + string_size,
layout->align);
@@ -399,6 +399,39 @@ SUFFIX (grub_mkimage_generate_elf) (const struct
grub_install_image_target_desc
phdr->p_offset = grub_host_to_target32 (header_size + program_size);
}
+ if (image_target->id == IMAGE_XENPVH)
+{
+ char *note_start = (elf_img + program_size + header_size);
+ Elf_Nhdr *note_ptr;
+ char *ptr = (char *) note_start;
+
+ grub_util_info ("adding XEN NOTE segment");
+
+ /* Phys32 Entry. */
+ note_ptr = (Elf_Nhdr *) ptr;
+ note_ptr->n_namesz = grub_host_to_target32 (sizeof (GRUB_XEN_NOTE_NAME));
+ note_ptr->n_descsz = grub_host_to_target32 (image_target->voidp_sizeof);
+ note_ptr->n_type = grub_host_to_target32 (18);
+ ptr += sizeof (Elf_Nhdr);
+ memcpy (ptr, GRUB_XEN_NOTE_NAME, sizeof (GRUB_XEN_NOTE_NAME));
+ ptr += ALIGN_UP (sizeof (GRUB_XEN_NOTE_NAME), 4);
+ memset (ptr, 0, image_target->voidp_sizeof);
+ *(grub_uint32_t *) ptr = GRUB_KERNEL_I386_XENPVH_LINK_ADDR;
+ ptr += image_target->voidp_sizeof;
+
+ assert (XENPVH_NOTE_SIZE == (ptr - note_start));
+
+ phdr++;
+ phdr->p_type = grub_host_to_target32 (PT_NOTE);
+ phdr->p_flags = grub_host_to_target32 (PF_R);
+ phdr->p_align = grub_host_to_target32 (image_target->voidp_sizeof);
+ phdr->p_vaddr = 0;
+ phdr->p_paddr = 0;
+ phdr->p_filesz = grub_host_to_target32 (XENPVH_NOTE_SIZE);
+ phdr->p_memsz = 0;
+ phdr->p_offset = grub_host_to_target32 (header_size + program_size);
+}
+
if (note)
{
int note_size = sizeof (struct grub_ieee1275_note);
@@ -474,7 +507,7 @@ SUFFIX (grub_mkimage_generate_elf) (const struct
grub_install_image_target_desc
shdr->sh_entsize = grub_host_to_target32 (0);
shdr++;
-if (image_target->id == IMAGE_XEN)
+if (image_target->id == IMAGE_XEN || image_target->id == IMAGE_XENPVH)
{
memcpy (ptr, ".xen", sizeof (".xen"));
shdr->sh_name = grub_host_to_target32 (ptr - str_start);
@@ -482,7 +515,10 @@ SUFFIX (grub_mkimage_generate_elf) (const struct
grub_install_image_target_desc
shdr->sh_type = grub_host_to_target32 (SHT_PROGBITS);
shdr->sh_addr = grub_host_to_target_addr (target_addr +
layout->kernel_size);
shdr->sh_offset = grub_host_to_target_addr (program_size + header_size);
- shdr-&g
[Xen-devel] [PATCH 3/8] xen: carve out grant tab initialization into dedicated function
Initialize the grant tab in a dedicated function. This will enable
using it for PVH guests, too.
Signed-off-by: Juergen Gross
---
grub-core/kern/xen/init.c | 35 +--
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
index 0559c033c..29f5bc23d 100644
--- a/grub-core/kern/xen/init.c
+++ b/grub-core/kern/xen/init.c
@@ -318,6 +318,25 @@ grub_xenstore_dir (const char *dir,
unsigned long gntframe = 0;
+static void
+grub_xen_setup_gnttab (void)
+{
+ struct gnttab_set_version gnttab_setver;
+ struct gnttab_setup_table gnttab_setup;
+
+ grub_memset (&gnttab_setver, 0, sizeof (gnttab_setver));
+
+ gnttab_setver.version = 1;
+ grub_xen_grant_table_op (GNTTABOP_set_version, &gnttab_setver, 1);
+
+ grub_memset (&gnttab_setup, 0, sizeof (gnttab_setup));
+ gnttab_setup.dom = DOMID_SELF;
+ gnttab_setup.nr_frames = 1;
+ gnttab_setup.frame_list.p = &gntframe;
+
+ grub_xen_grant_table_op (GNTTABOP_setup_table, &gnttab_setup, 1);
+}
+
#define MAX_N_UNUSABLE_PAGES 4
static int
@@ -357,26 +376,12 @@ map_all_pages (void)
(grub_xen_mfn_t *) grub_xen_start_page_addr->mfn_list;
grub_uint64_t *pg = (grub_uint64_t *) window;
grub_uint64_t oldpgstart, oldpgend;
- struct gnttab_setup_table gnttab_setup;
- struct gnttab_set_version gnttab_setver;
grub_size_t n_unusable_pages = 0;
struct mmu_update m2p_updates[2 * MAX_N_UNUSABLE_PAGES];
if (total_pages > MAX_TOTAL_PAGES - 4)
total_pages = MAX_TOTAL_PAGES - 4;
- grub_memset (&gnttab_setver, 0, sizeof (gnttab_setver));
-
- gnttab_setver.version = 1;
- grub_xen_grant_table_op (GNTTABOP_set_version, &gnttab_setver, 1);
-
- grub_memset (&gnttab_setup, 0, sizeof (gnttab_setup));
- gnttab_setup.dom = DOMID_SELF;
- gnttab_setup.nr_frames = 1;
- gnttab_setup.frame_list.p = &gntframe;
-
- grub_xen_grant_table_op (GNTTABOP_setup_table, &gnttab_setup, 1);
-
for (j = 0; j < total_pages - n_unusable_pages; j++)
while (!grub_xen_is_page_usable (mfn_list[j]))
{
@@ -537,6 +542,8 @@ grub_machine_init (void)
+ GRUB_KERNEL_MACHINE_MOD_GAP,
GRUB_KERNEL_MACHINE_MOD_ALIGN);
+ grub_xen_setup_gnttab ();
+
map_all_pages ();
grub_console_init ();
--
2.12.3
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 5/8] xenpvh: add build runes for grub-core
Add the modifications to the build system needed to build a xenpvh
grub.
Signed-off-by: Juergen Gross
---
gentpl.py | 4 ++--
grub-core/Makefile.am | 12
grub-core/Makefile.core.def | 35 +++
3 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/gentpl.py b/gentpl.py
index da67965a4..9732b4aee 100644
--- a/gentpl.py
+++ b/gentpl.py
@@ -28,7 +28,7 @@ import re
GRUB_PLATFORMS = [ "emu", "i386_pc", "i386_efi", "i386_qemu", "i386_coreboot",
"i386_multiboot", "i386_ieee1275", "x86_64_efi",
- "i386_xen", "x86_64_xen",
+ "i386_xen", "x86_64_xen", "i386_xenpvh",
"mips_loongson", "sparc64_ieee1275",
"powerpc_ieee1275", "mips_arc", "ia64_efi",
"mips_qemu_mips", "arm_uboot", "arm_efi", "arm64_efi",
@@ -71,7 +71,7 @@ GROUPS["videomodules"] = GRUB_PLATFORMS[:];
for i in GROUPS["videoinkernel"]: GROUPS["videomodules"].remove(i)
# Similar for terminfo
-GROUPS["terminfoinkernel"] = [ "emu", "mips_loongson", "mips_arc",
"mips_qemu_mips" ] + GROUPS["xen"] + GROUPS["ieee1275"] + GROUPS["uboot"];
+GROUPS["terminfoinkernel"] = [ "emu", "mips_loongson", "mips_arc",
"mips_qemu_mips", "i386_xenpvh" ] + GROUPS["xen"] + GROUPS["ieee1275"] +
GROUPS["uboot"];
GROUPS["terminfomodule"] = GRUB_PLATFORMS[:];
for i in GROUPS["terminfoinkernel"]: GROUPS["terminfomodule"].remove(i)
diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
index 104513847..38c9a0485 100644
--- a/grub-core/Makefile.am
+++ b/grub-core/Makefile.am
@@ -101,6 +101,18 @@ KERNEL_HEADER_FILES +=
$(top_builddir)/include/grub/machine/int.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
endif
+if COND_i386_xenpvh
+KERNEL_HEADER_FILES += $(top_builddir)/include/grub/machine/kernel.h
+KERNEL_HEADER_FILES += $(top_builddir)/include/grub/machine/int.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/terminfo.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/extcmd.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/loader.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/lib/arg.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/xen.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/xen/hypercall.h
+endif
+
if COND_i386_efi
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/efi.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/disk.h
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 2c1d62cee..68bf59de4 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -79,6 +79,8 @@ kernel = {
i386_xen_ldflags = '$(TARGET_IMG_BASE_LDOPT),0';
x86_64_xen_ldflags = '$(TARGET_IMG_LDFLAGS)';
x86_64_xen_ldflags = '$(TARGET_IMG_BASE_LDOPT),0';
+ i386_xenpvh_ldflags = '$(TARGET_IMG_LDFLAGS)';
+ i386_xenpvh_ldflags = '$(TARGET_IMG_BASE_LDOPT),0x10';
mips_loongson_ldflags= '-Wl,-Ttext,0x8020';
powerpc_ieee1275_ldflags = '-Wl,-Ttext,0x20';
@@ -100,6 +102,7 @@ kernel = {
x86_64_efi_startup = kern/x86_64/efi/startup.S;
i386_xen_startup = kern/i386/xen/startup.S;
x86_64_xen_startup = kern/x86_64/xen/startup.S;
+ i386_xenpvh_startup = kern/i386/xen/startup_pvh.S;
i386_qemu_startup = kern/i386/qemu/startup.S;
i386_ieee1275_startup = kern/i386/ieee1275/startup.S;
i386_coreboot_startup = kern/i386/coreboot/startup.S;
@@ -177,6 +180,7 @@ kernel = {
i386 = kern/i386/dl.c;
i386_xen = kern/i386/dl.c;
+ i386_xenpvh = kern/i386/dl.c;
i386_coreboot = kern/i386/coreboot/init.c;
i386_multiboot = kern/i386/coreboot/init.c;
@@ -222,6 +226,14 @@ kernel = {
xen = disk/xen/xendisk.c;
xen = commands/boot.c;
+ i386_xenpvh = kern/i386/tsc.c;
+ i386_xenpvh = kern/i386/xen/tsc.c;
+ i386_xenpvh = commands/boot.c;
+ i386_xenpvh = kern/xen/init.c;
+ i386_xenpvh = kern/i386/xen/pvh.c;
+ i386_xenpvh = term/xen/console.c;
+ i386_xenpvh = disk/xen/xendisk.c;
+
ia64_efi = kern/ia64/efi/startup.S;
ia64_efi = kern/ia64/efi/init.c;
ia64_efi = kern/ia64/dl.c;
@@ -802,6 +814,7 @@ module = {
name = cpuid;
common = commands/i386/cpuid.c;
enable = x86;
+ enable = i386_xenpvh;
enable = i386_xen;
enable = x86_64_xen;
};
@@ -861,6 +874,7 @@ module = {
i386_coreboot = lib/i386/halt.c;
i386_qemu = lib/i386/halt.
[Xen-devel] [PATCH 4/8] xen: add xen pvh guest support to grub-core
Add all the grub-core code needed for Xen PVH guest support. This
includes:
- The new PVH entry point of grub
- PVH specific initialization code
- machine specific header files
- modifications in Xen specific code to work in PVH environment
- modifications in other core code to be reusable with PVH
Enabling all this code is done later.
Signed-off-by: Juergen Gross
---
grub-core/kern/i386/tsc.c | 2 +-
grub-core/kern/i386/xen/pvh.c | 344 ++
grub-core/kern/i386/xen/startup_pvh.S | 80
grub-core/kern/xen/init.c | 66 ---
include/grub/i386/pc/int.h| 3 +
include/grub/i386/tsc.h | 2 +-
include/grub/i386/xen/hypercall.h | 5 +-
include/grub/i386/xenpvh/boot.h | 1 +
include/grub/i386/xenpvh/console.h| 1 +
include/grub/i386/xenpvh/int.h| 1 +
include/grub/i386/xenpvh/kernel.h | 30 +++
include/grub/i386/xenpvh/memory.h | 54 ++
include/grub/i386/xenpvh/time.h | 1 +
include/grub/kernel.h | 4 +-
include/grub/offsets.h| 3 +
include/grub/xen.h| 6 +
16 files changed, 573 insertions(+), 30 deletions(-)
create mode 100644 grub-core/kern/i386/xen/pvh.c
create mode 100644 grub-core/kern/i386/xen/startup_pvh.S
create mode 100644 include/grub/i386/xenpvh/boot.h
create mode 100644 include/grub/i386/xenpvh/console.h
create mode 100644 include/grub/i386/xenpvh/int.h
create mode 100644 include/grub/i386/xenpvh/kernel.h
create mode 100644 include/grub/i386/xenpvh/memory.h
create mode 100644 include/grub/i386/xenpvh/time.h
diff --git a/grub-core/kern/i386/tsc.c b/grub-core/kern/i386/tsc.c
index f266eb131..43fee3a13 100644
--- a/grub-core/kern/i386/tsc.c
+++ b/grub-core/kern/i386/tsc.c
@@ -65,7 +65,7 @@ grub_tsc_init (void)
tsc_boot_time = grub_get_tsc ();
-#ifdef GRUB_MACHINE_XEN
+#if defined (GRUB_MACHINE_XEN) || defined (GRUB_MACHINE_XENPVH)
(void) (grub_tsc_calibrate_from_xen () || calibrate_tsc_hardcode());
#elif defined (GRUB_MACHINE_EFI)
(void) (grub_tsc_calibrate_from_pmtimer () || grub_tsc_calibrate_from_pit ()
|| grub_tsc_calibrate_from_efi() || calibrate_tsc_hardcode());
diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
new file mode 100644
index 0..6f154d176
--- /dev/null
+++ b/grub-core/kern/i386/xen/pvh.c
@@ -0,0 +1,344 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2011 Free Software Foundation, Inc.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct xen_machine_mmap_entry
+{
+ grub_uint64_t addr;
+ grub_uint64_t len;
+ grub_uint32_t type;
+} GRUB_PACKED;
+
+grub_uint64_t grub_rsdp_addr;
+
+static struct { char _entry[32]; } hypercall_page[128]
+ __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
+
+static grub_uint32_t xen_cpuid_base;
+static struct start_info grub_xen_start_page;
+static struct xen_machine_mmap_entry map[128];
+static unsigned int nr_map_entries;
+
+static void
+grub_xen_early_halt (void)
+{
+ asm volatile ("hlt");
+}
+
+static void
+grub_xen_cpuid_base (void)
+{
+ grub_uint32_t base, eax, signature[3];
+
+ for (base = 0x4000; base < 0x4001; base += 0x100)
+{
+ grub_cpuid (base, eax, signature[0], signature[1], signature[2]);
+ if (!grub_memcmp ("XenVMMXenVMM", signature, 12) && (eax - base) >= 2)
+{
+ xen_cpuid_base = base;
+ return;
+}
+}
+
+ grub_xen_early_halt ();
+}
+
+static void
+grub_xen_setup_hypercall_page (void)
+{
+ grub_uint32_t msr, pfn, eax, ebx, ecx, edx;
+
+ grub_cpuid (xen_cpuid_base + 2, eax, ebx, ecx, edx);
+ msr = ebx;
+ pfn = (grub_uint32_t) (&hypercall_page[0]);
+
+ asm volatile ("wrmsr" : : "c" (msr), "a" (pfn), "d" (0) : "memory");
+}
+
+int
+grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t a0,
+ grub_uint32_t a1, grub_uint32_t a2,
+ grub_uint32_t a3, grub_uint32_t a4,
+ grub_uint32_t a5 __attribute__ ((unused)))
+{
+ register unsigned long __res asm("eax");
+ register unsigned long __arg0 asm("ebx") =
Re: [Xen-devel] [RFC PATCH] KVM: x86: Allow Qemu/KVM to use PVH entry point
On 29/11/17 15:03, Boris Ostrovsky wrote: > On 11/29/2017 03:50 AM, Roger Pau Monné wrote: >> On Wed, Nov 29, 2017 at 09:21:59AM +0100, Juergen Gross wrote: >>> On 28/11/17 20:34, Maran Wilson wrote: >>>> For certain applications it is desirable to rapidly boot a KVM virtual >>>> machine. In cases where legacy hardware and software support within the >>>> guest is not needed, Qemu should be able to boot directly into the >>>> uncompressed Linux kernel binary without the need to run firmware. >>>> >>>> There already exists an ABI to allow this for Xen PVH guests and the ABI is >>>> supported by Linux and FreeBSD: >>>> >>>>https://xenbits.xen.org/docs/unstable/misc/hvmlite.html >> I would also add a link to: >> >> http://xenbits.xen.org/docs/unstable/hypercall/x86_64/include,public,arch-x86,hvm,start_info.h.html#Struct_hvm_start_info >> >>>> This PoC patch enables Qemu to use that same entry point for booting KVM >>>> guests. >>>> >>>> Even though the code is still PoC quality, I'm sending this as an RFC now >>>> since there are a number of different ways the specific implementation >>>> details can be handled. I chose a shared code path for Xen and KVM guests >>>> but could just as easily create a separate code path that is advertised by >>>> a different ELF note for KVM. There also seems to be some flexibility in >>>> how the e820 table data is passed and how (or if) it should be identified >>>> as e820 data. As a starting point, I've chosen the options that seem to >>>> result in the smallest patch with minimal to no changes required of the >>>> x86/HVM direct boot ABI. >>> I like the idea. >>> >>> I'd rather split up the different hypervisor types early and use a >>> common set of service functions instead of special casing xen_guest >>> everywhere. This would make it much easier to support the KVM PVH >>> boot without the need to configure the kernel with CONFIG_XEN. >>> >>> Another option would be to use the same boot path as with grub: set >>> the boot params in zeropage and start at startup_32. >> I think I prefer this approach since AFAICT it should allow for >> greater code share with the common boot path. > > zeropage is x86/Linux-specific so we'd need some sort of firmware (like > grub) between a hypervisor and Linux to convert hvm_start_info to > bootparams. qemu? Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] libxl: put RSDP for PVH guest near 4GB
Instead of locating the RSDP table below 1MB put it just below 4GB
like the rest of the ACPI tables in case of PVH guests. This will
avoid punching more holes than necessary into the memory map.
Signed-off-by: Juergen Gross
---
tools/libxc/xc_dom_hvmloader.c | 2 +-
tools/libxl/libxl_x86_acpi.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/libxc/xc_dom_hvmloader.c b/tools/libxc/xc_dom_hvmloader.c
index 59f94e51e5..3f0bd65547 100644
--- a/tools/libxc/xc_dom_hvmloader.c
+++ b/tools/libxc/xc_dom_hvmloader.c
@@ -136,7 +136,7 @@ static int module_init_one(struct xc_dom_image *dom,
struct xc_dom_seg seg;
void *dest;
-if ( module->length )
+if ( module->length && !module->guest_addr_out )
{
if ( xc_dom_alloc_segment(dom, &seg, name, 0, module->length) )
goto err;
diff --git a/tools/libxl/libxl_x86_acpi.c b/tools/libxl/libxl_x86_acpi.c
index 9a7c90467d..b2a861d845 100644
--- a/tools/libxl/libxl_x86_acpi.c
+++ b/tools/libxl/libxl_x86_acpi.c
@@ -23,7 +23,6 @@
/* Number of pages holding ACPI tables */
#define NUM_ACPI_PAGES 16
/* Store RSDP in the last 64 bytes of BIOS RO memory */
-#define RSDP_ADDRESS (0x10 - 64)
#define ACPI_INFO_PHYSICAL_ADDRESS 0xfc00
struct libxl_acpi_ctxt {
@@ -220,7 +219,8 @@ int libxl__dom_load_acpi(libxl__gc *gc,
dom->acpi_modules[0].data = (void *)config.rsdp;
dom->acpi_modules[0].length = 64;
-dom->acpi_modules[0].guest_addr_out = RSDP_ADDRESS;
+dom->acpi_modules[0].guest_addr_out = ACPI_INFO_PHYSICAL_ADDRESS +
+(1 + acpi_pages_num) * libxl_ctxt.page_size;
dom->acpi_modules[1].data = (void *)config.infop;
dom->acpi_modules[1].length = 4096;
--
2.12.3
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] libxl: put RSDP for PVH guest near 4GB
On 29/11/17 15:25, Jan Beulich wrote: On 29.11.17 at 15:13, wrote: >> --- a/tools/libxl/libxl_x86_acpi.c >> +++ b/tools/libxl/libxl_x86_acpi.c >> @@ -23,7 +23,6 @@ >> /* Number of pages holding ACPI tables */ >> #define NUM_ACPI_PAGES 16 >> /* Store RSDP in the last 64 bytes of BIOS RO memory */ >> -#define RSDP_ADDRESS (0x10 - 64) > > Leaving a then stale comment around? Indeed. Thanks for noticing. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH] KVM: x86: Allow Qemu/KVM to use PVH entry point
On 29/11/17 15:44, Paolo Bonzini wrote:
> On 29/11/2017 15:25, Boris Ostrovsky wrote:
> zeropage is x86/Linux-specific so we'd need some sort of firmware (like
> grub) between a hypervisor and Linux to convert hvm_start_info to
> bootparams.
qemu?
>>
>> I think KVM folks didn't want to do this. I can't find the thread but I
>> believe it was somewhere during Clear Containers discussion. Paolo?
>
> QEMU is the right place to parse the ELF file and save it in memory.
> You would have to teach QEMU to find the Xen note in ELF-format kernels
> (just like it looks for the multiboot header), and use a different
> option ROM ("pvhboot.c" for example).
>
> However I don't like to bypass the BIOS; for -kernel, KVM starts the
> guest with an option ROM (linuxboot-dma.c or multiboot.S in QEMU
> sources) that takes care of boot.
>
> In either case, you would have a new option ROM. It could either be
> very simple and similar to multiboot.S, or it could be larger and do the
> same task as xen-pvh.S and enlighten_pvh.c (then get the address of
> startup_32 or startup_64 from FW_CFG_KERNEL_ENTRY and jump there). The
> ugly part is that the option ROM would have to know more details about
> what it is going to boot, including for example whether it's 32-bit or
> 64-bit, so I don't really think it is a good idea.
As grub2 doesn't have to know, qemu shouldn't have to know either.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/8] xen: add pvh guest support
On 30/11/17 22:03, Daniel Kiper wrote: > On Wed, Nov 29, 2017 at 02:46:42PM +0100, Juergen Gross wrote: >> This patch series adds support for booting Linux as PVH guest. >> >> Similar to i386/xen and x86_64/xen platforms the new i386/xenpvh >> platform grub is booted as a standalone image directly by Xen. >> >> For booting Linux kernel it is using the standard linux kernel >> loader. The only modification of the linux loader is to pass the >> ACPI RSDP address via boot parameters to the kernel, as that table >> might not be located at the usual physical address just below 1MB. >> >> As the related Linux kernel patches are not yet accepted please >> wait for this to happen before applying the series. > > So, may I review the patches or should I hold on? And could you > provide a link to the "Linux kernel patches" mentioned above? Please review! The Linux patches are available at: https://lists.xen.org/archives/html/xen-devel/2017-11/msg01681.html With the additional Xen patch https://lists.xen.org/archives/html/xen-devel/2017-11/msg01807.html the complete setup can be tested. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] libxl: put RSDP for PVH guest near 4GB
Instead of locating the RSDP table below 1MB put it just below 4GB
like the rest of the ACPI tables in case of PVH guests. This will
avoid punching more holes than necessary into the memory map.
Signed-off-by: Juergen Gross
Acked-by: Wei Liu
---
tools/libxc/xc_dom_hvmloader.c | 2 +-
tools/libxl/libxl_x86_acpi.c | 5 ++---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/tools/libxc/xc_dom_hvmloader.c b/tools/libxc/xc_dom_hvmloader.c
index 59f94e51e5..3f0bd65547 100644
--- a/tools/libxc/xc_dom_hvmloader.c
+++ b/tools/libxc/xc_dom_hvmloader.c
@@ -136,7 +136,7 @@ static int module_init_one(struct xc_dom_image *dom,
struct xc_dom_seg seg;
void *dest;
-if ( module->length )
+if ( module->length && !module->guest_addr_out )
{
if ( xc_dom_alloc_segment(dom, &seg, name, 0, module->length) )
goto err;
diff --git a/tools/libxl/libxl_x86_acpi.c b/tools/libxl/libxl_x86_acpi.c
index 9a7c90467d..fe87418bc1 100644
--- a/tools/libxl/libxl_x86_acpi.c
+++ b/tools/libxl/libxl_x86_acpi.c
@@ -22,8 +22,6 @@
/* Number of pages holding ACPI tables */
#define NUM_ACPI_PAGES 16
-/* Store RSDP in the last 64 bytes of BIOS RO memory */
-#define RSDP_ADDRESS (0x10 - 64)
#define ACPI_INFO_PHYSICAL_ADDRESS 0xfc00
struct libxl_acpi_ctxt {
@@ -220,7 +218,8 @@ int libxl__dom_load_acpi(libxl__gc *gc,
dom->acpi_modules[0].data = (void *)config.rsdp;
dom->acpi_modules[0].length = 64;
-dom->acpi_modules[0].guest_addr_out = RSDP_ADDRESS;
+dom->acpi_modules[0].guest_addr_out = ACPI_INFO_PHYSICAL_ADDRESS +
+(1 + acpi_pages_num) * libxl_ctxt.page_size;
dom->acpi_modules[1].data = (void *)config.infop;
dom->acpi_modules[1].length = 4096;
--
2.12.3
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH
On 21/11/17 12:06, Juergen Gross wrote:
> The "special pages" for PVH guests include the frames for console and
> Xenstore ring buffers. Those have to be marked as "Reserved" in the
> guest's E820 map, as otherwise conflicts might arise later e.g. when
> hotplugging memory into the guest.
>
> Signed-off-by: Juergen Gross
> ---
> This is a bugfix for PVH guests. Please consider for 4.10.
Ping?
> ---
> tools/libxl/libxl_x86.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> index 5f91fe4f92..d82013f6ed 100644
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -530,6 +530,9 @@ int libxl__arch_domain_construct_memmap(libxl__gc *gc,
> if (d_config->rdms[i].policy != LIBXL_RDM_RESERVE_POLICY_INVALID)
> e820_entries++;
>
> +/* Add mmio entry for PVH. */
> +if (dom->mmio_size && d_config->b_info.type == LIBXL_DOMAIN_TYPE_PVH)
> +e820_entries++;
>
> /* If we should have a highmem range. */
> if (highmem_size)
> @@ -564,6 +567,14 @@ int libxl__arch_domain_construct_memmap(libxl__gc *gc,
> nr++;
> }
>
> +/* mmio area */
> +if (dom->mmio_size && d_config->b_info.type == LIBXL_DOMAIN_TYPE_PVH) {
> +e820[nr].addr = dom->mmio_start;
> +e820[nr].size = dom->mmio_size;
> +e820[nr].type = E820_RESERVED;
> +nr++;
> +}
> +
> for (i = 0; i < MAX_ACPI_MODULES; i++) {
> if (dom->acpi_modules[i].length) {
> e820[nr].addr = dom->acpi_modules[i].guest_addr_out &
> ~(page_size - 1);
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [patch 01/60] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags
On 04/12/17 15:07, Thomas Gleixner wrote:
> From: Boris Ostrovsky
>
> Commit 1d3e53e8624a ("x86/entry/64: Refactor IRQ stacks and make them
> NMI-safe") added DEBUG_ENTRY_ASSERT_IRQS_OFF macro that acceses eflags
> using 'pushfq' instruction when testing for IF bit. On PV Xen guests
> looking at IF flag directly will always see it set, resulting in 'ud2'.
>
> Introduce SAVE_FLAGS() macro that will use appropriate save_fl pv op when
> running paravirt.
>
> Signed-off-by: Boris Ostrovsky
> Signed-off-by: Thomas Gleixner
> Cc: jgr...@suse.com
> Cc: xen-devel@lists.xenproject.org
> Cc: l...@kernel.org
> Link:
> https://lkml.kernel.org/r/1512159805-6314-1-git-send-email-boris.ostrov...@oracle.com
Reviewed-by: Juergen Gross
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [for-4.10] Re: [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH
On 05/12/17 16:23, Julien Grall wrote: > Hi Juergen, > > On 04/12/17 15:49, Juergen Gross wrote: >> On 21/11/17 12:06, Juergen Gross wrote: >>> The "special pages" for PVH guests include the frames for console and >>> Xenstore ring buffers. Those have to be marked as "Reserved" in the >>> guest's E820 map, as otherwise conflicts might arise later e.g. when >>> hotplugging memory into the guest. >>> >>> Signed-off-by: Juergen Gross >>> --- >>> This is a bugfix for PVH guests. Please consider for 4.10. >> >> Ping? > > I was waiting an ack from tools maintainers before looking for a release > perspective. > > I would recommend to tag your patch is 4.10 to help reviewers prioritize > review on your patch. I have done it now. > > I am looking at releasing Xen 4.10 in the next few days. Can you explain > the pros/cons of this patch? Pros: PVH guests with 4GB of memory or more will work. :-) Cons: There is a more general solution (as Roger pointed out), but this would require much more work. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] xen/pvcalls: Fix a check in pvcalls_front_remove()
On 05/12/17 15:38, Dan Carpenter wrote:
> bedata->ref can't be less than zero because it's unsigned. This affects
> certain error paths in probe. We first set ->ref = -1 and then we set
> it to a valid value later.
>
> Fixes: 219681909913 ("xen/pvcalls: connect to the backend")
> Signed-off-by: Dan Carpenter
Reviewed-by: Juergen Gross
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/pvcalls: check for xenbus_read() errors
On 05/12/17 15:38, Dan Carpenter wrote: > Smatch complains that "len" is uninitialized if xenbus_read() fails so > let's add some error handling. > > Signed-off-by: Dan Carpenter Reviewed-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [for-4.10] Re: [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH
On 06/12/17 10:53, Julien Grall wrote: > Hi Juergen, > > On 12/05/2017 04:19 PM, Juergen Gross wrote: >> On 05/12/17 16:23, Julien Grall wrote: >>> Hi Juergen, >>> >>> On 04/12/17 15:49, Juergen Gross wrote: >>>> On 21/11/17 12:06, Juergen Gross wrote: >>>>> The "special pages" for PVH guests include the frames for console and >>>>> Xenstore ring buffers. Those have to be marked as "Reserved" in the >>>>> guest's E820 map, as otherwise conflicts might arise later e.g. when >>>>> hotplugging memory into the guest. >>>>> >>>>> Signed-off-by: Juergen Gross >>>>> --- >>>>> This is a bugfix for PVH guests. Please consider for 4.10. >>>> >>>> Ping? >>> >>> I was waiting an ack from tools maintainers before looking for a release >>> perspective. >>> >>> I would recommend to tag your patch is 4.10 to help reviewers prioritize >>> review on your patch. I have done it now. >>> >>> I am looking at releasing Xen 4.10 in the next few days. Can you explain >>> the pros/cons of this patch? >> >> Pros: PVH guests with 4GB of memory or more will work. :-) > > They never worked before? Or is it a regression? If it is a regression > when did it appear? Hmm, seems we are lucky: Linux kernel will not try to map any memory there (I just tested it). So we don't need that patch in 4.10 for Linux running as PVH guest. Not sure about BSD, though. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [for-4.10] Re: [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH
On 06/12/17 12:50, Julien Grall wrote: > Hi, > > On 12/06/2017 11:47 AM, Roger Pau Monné wrote: >> On Wed, Dec 06, 2017 at 12:22:00PM +0100, Juergen Gross wrote: >>> On 06/12/17 10:53, Julien Grall wrote: >>>> Hi Juergen, >>>> >>>> On 12/05/2017 04:19 PM, Juergen Gross wrote: >>>>> On 05/12/17 16:23, Julien Grall wrote: >>>>>> Hi Juergen, >>>>>> >>>>>> On 04/12/17 15:49, Juergen Gross wrote: >>>>>>> On 21/11/17 12:06, Juergen Gross wrote: >>>>>>>> The "special pages" for PVH guests include the frames for >>>>>>>> console and >>>>>>>> Xenstore ring buffers. Those have to be marked as "Reserved" in the >>>>>>>> guest's E820 map, as otherwise conflicts might arise later e.g. >>>>>>>> when >>>>>>>> hotplugging memory into the guest. >>>>>>>> >>>>>>>> Signed-off-by: Juergen Gross >>>>>>>> --- >>>>>>>> This is a bugfix for PVH guests. Please consider for 4.10. >>>>>>> >>>>>>> Ping? >>>>>> >>>>>> I was waiting an ack from tools maintainers before looking for a >>>>>> release >>>>>> perspective. >>>>>> >>>>>> I would recommend to tag your patch is 4.10 to help reviewers >>>>>> prioritize >>>>>> review on your patch. I have done it now. >>>>>> >>>>>> I am looking at releasing Xen 4.10 in the next few days. Can you >>>>>> explain >>>>>> the pros/cons of this patch? >>>>> >>>>> Pros: PVH guests with 4GB of memory or more will work. :-) >>>> >>>> They never worked before? Or is it a regression? If it is a regression >>>> when did it appear? >>> >>> Hmm, seems we are lucky: Linux kernel will not try to map any memory >>> there (I just tested it). So we don't need that patch in 4.10 for Linux >>> running as PVH guest. Not sure about BSD, though. >> >> I haven't yet committed any PVHv2 support to FreeBSD, but in any case >> I always avoid using memory below 4GB just in case... > > I will defer this patch post 4.10. We can add a release note if you > think it is worth it. I don't think that is needed. And in case someone hits this problem we can easily apply the patch to a 4.10 update release later. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available
In case the rsdp address in struct boot_params is specified don't try
to find the table by searching, but take the address directly as set
by the boot loader.
Signed-off-by: Juergen Gross
---
drivers/acpi/osl.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 3bb46cb24a99..3b25e2ad7d75 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -45,6 +45,10 @@
#include
#include
+#ifdef CONFIG_X86
+#include
+#endif
+
#include "internal.h"
#define _COMPONENT ACPI_OS_SERVICES
@@ -195,6 +199,10 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
if (acpi_rsdp)
return acpi_rsdp;
#endif
+#ifdef CONFIG_X86
+ if (boot_params.hdr.acpi_rsdp_addr)
+ return boot_params.hdr.acpi_rsdp_addr;
+#endif
if (efi_enabled(EFI_CONFIG_TABLES)) {
if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
--
2.12.3
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 0/3] x86: make rsdp address accessible via boot params
In the non-EFI boot path the ACPI RSDP table is currently found via either EBDA or by searching through low memory for the RSDP magic. This requires the RSDP to be located in the first 1MB of physical memory. Xen PVH guests, however, get the RSDP address via the start of day information block. In order to support an arbitrary RSDP address this patch series adds the physical address of the RSDP to the boot params structure filled by the boot loader. A kernel booted directly in PVH mode can save the RSDP address in the boot params, while a kernel booted in PVH mode via grub can rely on the RSDP address being specified by grub2 (which in turn got the address via the start of day information block from Xen). Juergen Gross (3): x86/boot: add acpi rsdp address to setup_header x86/acpi: take rsdp address for boot params if available x86/xen: supply rsdp address in boot params for pvh guests Documentation/x86/boot.txt| 19 +++ arch/x86/boot/header.S| 6 +- arch/x86/include/uapi/asm/bootparam.h | 1 + arch/x86/xen/enlighten_pvh.c | 5 - drivers/acpi/osl.c| 8 5 files changed, 37 insertions(+), 2 deletions(-) -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header
Xen PVH guests receive the address of the RSDP table from Xen. In order
to support booting a Xen PVH guest via grub2 using the standard x86
boot entry we need a way fro grub2 to pass the RSDP address to the
kernel.
For this purpose expand the struct setup_header to hold the physical
address of the RSDP address. Being zero means it isn't specified and
has to be located the legacy way (searching through low memory or
EBDA).
Signed-off-by: Juergen Gross
Reviewed-by: Roger Pau Monné
---
Documentation/x86/boot.txt| 19 +++
arch/x86/boot/header.S| 6 +-
arch/x86/include/uapi/asm/bootparam.h | 1 +
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
index 5e9b826b5f62..a33c224797e4 100644
--- a/Documentation/x86/boot.txt
+++ b/Documentation/x86/boot.txt
@@ -61,6 +61,13 @@ Protocol 2.12: (Kernel 3.8) Added the xloadflags field
and extension fields
to struct boot_params for loading bzImage and ramdisk
above 4G in 64bit.
+Protocol 2.13: (Kernel 3.14) Support 32- and 64-bit flags being set in
+ xloadflags to support booting a 64 bit kernel from 32 bit
+ EFI
+
+Protocol 2.14 (Kernel 4.16) Added acpi_rsdp_addr holding the physical
+ address of the ACPI RSDP table.
+
MEMORY LAYOUT
The traditional memory map for the kernel loader, used for Image or
@@ -197,6 +204,7 @@ Offset Proto NameMeaning
0258/8 2.10+ pref_addressPreferred loading address
0260/4 2.10+ init_size Linear memory required during initialization
0264/4 2.11+ handover_offset Offset of handover entry point
+0268/8 2.14+ acpi_rsdp_addr Physical address of RSDP table
(1) For backwards compatibility, if the setup_sects field contains 0, the
real value is 4.
@@ -744,6 +752,17 @@ Offset/size: 0x264/4
See EFI HANDOVER PROTOCOL below for more details.
+Field name:acpi_rsdp_addr
+Type: write
+Offset/size: 0x268/8
+Protocol: 2.14+
+
+ This field can be set by the boot loader to tell the kernel the
+ physical address of the ACPI RSDP table.
+
+ A value of 0 indicates the kernel should fall back to the standard
+ methods to locate the RSDP (search in EBDA/low memory).
+
THE IMAGE CHECKSUM
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 850b8762e889..e7184127f309 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -300,7 +300,7 @@ _start:
# Part 2 of the header, from the old setup.S
.ascii "HdrS" # header signature
- .word 0x020d # header version number (>= 0x0105)
+ .word 0x020e # header version number (>= 0x0105)
# or else old loadlin-1.5 will fail)
.globl realmode_swtch
realmode_swtch:.word 0, 0# default_switch, SETUPSEG
@@ -558,6 +558,10 @@ pref_address: .quad LOAD_PHYSICAL_ADDR
# preferred load addr
init_size: .long INIT_SIZE # kernel initialization size
handover_offset: .long 0 # Filled in by build.c
+acpi_rsdp_addr:.quad 0 # 64-bit physical
pointer to
+ # ACPI RSDP table, added with
+ # version 2.14
+
# End of setup header #
.section ".entrytext", "ax"
diff --git a/arch/x86/include/uapi/asm/bootparam.h
b/arch/x86/include/uapi/asm/bootparam.h
index afdd5ae0fcc4..5742e433e93e 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -85,6 +85,7 @@ struct setup_header {
__u64 pref_address;
__u32 init_size;
__u32 handover_offset;
+ __u64 acpi_rsdp_addr;
} __attribute__((packed));
struct sys_desc_table {
--
2.12.3
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 3/3] x86/xen: supply rsdp address in boot params for pvh guests
When booted via the special PVH entry save the RSDP address set in the boot information block in struct boot_params. This will enable Xen to locate the RSDP at an arbitrary address. Set the boot loader version to 2.14 (0x020e) replacing the wrong 0x0212 which should have been 0x020c. Signed-off-by: Juergen Gross --- V2: set bootloader version to 2.14 (Roger Pau Monné) --- arch/x86/xen/enlighten_pvh.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c index 436c4f003e17..036e3a5f284a 100644 --- a/arch/x86/xen/enlighten_pvh.c +++ b/arch/x86/xen/enlighten_pvh.c @@ -68,9 +68,12 @@ static void __init init_pvh_bootparams(void) * * Version 2.12 supports Xen entry point but we will use default x86/PC * environment (i.e. hardware_subarch 0). +* The RSDP address is available from version 2.14 on. */ - pvh_bootparams.hdr.version = 0x212; + pvh_bootparams.hdr.version = 0x20e; pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */ + + pvh_bootparams.hdr.acpi_rsdp_addr = pvh_start_info.rsdp_paddr; } /* -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available
On 08/12/17 08:05, Ingo Molnar wrote:
>
> * Juergen Gross wrote:
>
>> In case the rsdp address in struct boot_params is specified don't try
>> to find the table by searching, but take the address directly as set
>> by the boot loader.
>>
>> Signed-off-by: Juergen Gross
>> ---
>> drivers/acpi/osl.c | 8
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
>> index 3bb46cb24a99..3b25e2ad7d75 100644
>> --- a/drivers/acpi/osl.c
>> +++ b/drivers/acpi/osl.c
>> @@ -45,6 +45,10 @@
>> #include
>> #include
>>
>> +#ifdef CONFIG_X86
>> +#include
>> +#endif
>> +
>> #include "internal.h"
>>
>> #define _COMPONENT ACPI_OS_SERVICES
>> @@ -195,6 +199,10 @@ acpi_physical_address __init
>> acpi_os_get_root_pointer(void)
>> if (acpi_rsdp)
>> return acpi_rsdp;
>> #endif
>> +#ifdef CONFIG_X86
>> +if (boot_params.hdr.acpi_rsdp_addr)
>> +return boot_params.hdr.acpi_rsdp_addr;
>> +#endif
>
> Argh, that's typical short sighted hackery, layering violations and general
> eyesore combined into a single patch ...
>
> Those #ifdefs are a disgrace, plus why should generic ACPI code include
> platform
> details like boot_params.hdr/acpi_rsdp_addr? It's also not very extensible to
> non-x86 - so someone will have to redo this work for ARM64 as well in the
> future
> ...
>
> So how about doing it right:
>
> 1)
>
> Add a __weak acpi_arch_get_root_pointer() __weak function to
> drivers/acpi/osl.c:
>
>
> __weak acpi_physical_address acpi_arch_get_root_pointer(void)
> {
> return 0;
> }
>
> 2)
>
> use it in acpi_os_get_root_pointer():
>
> ...
> pa = acpi_arch_get_root_pointer();
> if (pa)
> return pa;
> ...
>
> 3)
>
> Override the default variant in x86's acpi.c via something like:
>
> acpi_physical_address acpi_arch_get_root_pointer(void)
> {
> return boot_params.hdr.acpi_rsdp_addr;
> }
>
> 4)
>
> Add this to arch/x86/include/asm/acpi.h:
>
> extern acpi_physical_address acpi_arch_get_root_pointer(void);
>
> 5)
>
> Add #include to drivers/acpi/osl.c.
>
>
> That looks much cleaner, has no layering violations and is infinitely more
> extensible, right?
Right.
Thanks for the very constructive comment.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header
On 08/12/17 08:16, Ingo Molnar wrote: > > * Juergen Gross wrote: > >> Xen PVH guests receive the address of the RSDP table from Xen. In order >> to support booting a Xen PVH guest via grub2 using the standard x86 >> boot entry we need a way fro grub2 to pass the RSDP address to the >> kernel. >> >> For this purpose expand the struct setup_header to hold the physical >> address of the RSDP address. Being zero means it isn't specified and >> has to be located the legacy way (searching through low memory or >> EBDA). > > s/fro > /for > > pedantry: > > s/grub2 > /Grub2 Okay. > >> Signed-off-by: Juergen Gross >> Reviewed-by: Roger Pau Monné >> --- >> Documentation/x86/boot.txt| 19 +++ >> arch/x86/boot/header.S| 6 +- >> arch/x86/include/uapi/asm/bootparam.h | 1 + >> 3 files changed, 25 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt >> index 5e9b826b5f62..a33c224797e4 100644 >> --- a/Documentation/x86/boot.txt >> +++ b/Documentation/x86/boot.txt >> @@ -61,6 +61,13 @@ Protocol 2.12:(Kernel 3.8) Added the xloadflags field >> and extension fields >> to struct boot_params for loading bzImage and ramdisk >> above 4G in 64bit. >> >> +Protocol 2.13: (Kernel 3.14) Support 32- and 64-bit flags being set in >> +xloadflags to support booting a 64 bit kernel from 32 bit >> +EFI > > The changelog should I think declare that we add documentation for the 2.13 > protocol iteration as well. > > Also, please use a consistent spelling of '32-bit' and '64-bit' in the same > sentence! Okay. > >> +Field name: acpi_rsdp_addr >> +Type: write >> +Offset/size:0x268/8 >> +Protocol: 2.14+ >> + >> + This field can be set by the boot loader to tell the kernel the >> + physical address of the ACPI RSDP table. >> + >> + A value of 0 indicates the kernel should fall back to the standard >> + methods to locate the RSDP (search in EBDA/low memory). > > That's not the only method used: the ACPI RSDP address can also be discovered > via > efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values. Sure, but this is valid for booting via EFI only. > >> THE IMAGE CHECKSUM >> >> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S >> index 850b8762e889..e7184127f309 100644 >> --- a/arch/x86/boot/header.S >> +++ b/arch/x86/boot/header.S >> @@ -300,7 +300,7 @@ _start: >> # Part 2 of the header, from the old setup.S >> >> .ascii "HdrS" # header signature >> -.word 0x020d # header version number (>= 0x0105) >> +.word 0x020e # header version number (>= 0x0105) >> # or else old loadlin-1.5 will fail) >> .globl realmode_swtch >> realmode_swtch: .word 0, 0# default_switch, SETUPSEG >> @@ -558,6 +558,10 @@ pref_address: .quad LOAD_PHYSICAL_ADDR >> # preferred load addr >> init_size: .long INIT_SIZE # kernel initialization size >> handover_offset:.long 0 # Filled in by build.c >> >> +acpi_rsdp_addr: .quad 0 # 64-bit physical >> pointer to >> +# ACPI RSDP table, added with >> +# version 2.14 > > s/pointer to ACPI RSDP table > /pointer to the ACPI RSDP table Okay. > > Also, a more fundamental question: why doesn't Xen use EFI to hand over > hardware > configuration details? I think Jan has answered this question quite well. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 3/3] x86/xen: supply rsdp address in boot params for pvh guests
On 08/12/17 08:22, Ingo Molnar wrote: > > * Juergen Gross wrote: > >> When booted via the special PVH entry save the RSDP address set in the >> boot information block in struct boot_params. This will enable Xen to >> locate the RSDP at an arbitrary address. >> >> Set the boot loader version to 2.14 (0x020e) replacing the wrong 0x0212 >> which should have been 0x020c. >> >> Signed-off-by: Juergen Gross >> --- >> V2: set bootloader version to 2.14 (Roger Pau Monné) >> --- >> arch/x86/xen/enlighten_pvh.c | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c >> index 436c4f003e17..036e3a5f284a 100644 >> --- a/arch/x86/xen/enlighten_pvh.c >> +++ b/arch/x86/xen/enlighten_pvh.c >> @@ -68,9 +68,12 @@ static void __init init_pvh_bootparams(void) >> * >> * Version 2.12 supports Xen entry point but we will use default x86/PC >> * environment (i.e. hardware_subarch 0). >> + * The RSDP address is available from version 2.14 on. >> */ >> -pvh_bootparams.hdr.version = 0x212; >> +pvh_bootparams.hdr.version = 0x20e; > > While 0x212 was "obvious" to read but totally wrong, it would be less fragile > and > more readable if the version was generated as something like: > > pvh_bootparams.hdr.version = (2 << 8) | 14; Sure, I can make that change. > > similar to how it's written in other cases: > >> pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */ > > Also, shouldn't the 0x212 fix be a separate patch, Cc: stable? The bug > appears to > have been introduced at around v4.12. While not really being very important, this seems to be cleaner, yes. After all this value is visible in sysfs, so it should be correct. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/3] x86/boot: add acpi rsdp address to setup_header
On 08/12/17 09:48, Ingo Molnar wrote: > > * Juergen Gross wrote: > >>>> +Offset/size: 0x268/8 >>>> +Protocol: 2.14+ >>>> + >>>> + This field can be set by the boot loader to tell the kernel the >>>> + physical address of the ACPI RSDP table. >>>> + >>>> + A value of 0 indicates the kernel should fall back to the standard >>>> + methods to locate the RSDP (search in EBDA/low memory). >>> >>> That's not the only method used: the ACPI RSDP address can also be >>> discovered via >>> efi.rsdp20 and efi.rsdp, both of which appear to be 32-bit values. >> >> Sure, but this is valid for booting via EFI only. > > Yeah, so what I tried to say is that the description as written is not fully > correct and triggered my pedantry: > > + A value of 0 indicates the kernel should fall back to the standard > + methods to locate the RSDP (search in EBDA/low memory). > > To make it correct we need to either write less: > > + A value of 0 indicates the kernel should fall back to the standard > + methods to locate the RSDP. > > or write more and make it open ended so it doesn't have to be extended with > every > method of getting the RSDP that might be added in the future: > > + A value of 0 indicates the kernel should fall back to the standard > + methods to locate the RSDP (search in EBDA/low memory, get it from > + EFI if present, etc.). > > ... or so? Aah, okay. I got your remark wrong then. I think I'll go with the shorter variant. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [GIT PULL] xen: fixes for 4.15-rc3
Linus, Please git pull the following tag: git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git for-linus-4.15-rc3-tag xen: fixes for 4.15-rc3 Those are just two small fixes for the nev pvcalls frontend driver. Thanks. Juergen drivers/xen/pvcalls-front.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Dan Carpenter (2): xen/pvcalls: check for xenbus_read() errors xen/pvcalls: Fix a check in pvcalls_front_remove() ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available
On 08/12/17 08:05, Ingo Molnar wrote:
>
> * Juergen Gross wrote:
...
> acpi_physical_address acpi_arch_get_root_pointer(void)
> {
> return boot_params.hdr.acpi_rsdp_addr;
> }
>
> 4)
>
> Add this to arch/x86/include/asm/acpi.h:
>
> extern acpi_physical_address acpi_arch_get_root_pointer(void);
Uuh, this leads to problems for files including directly:
acpi_physical_address won't be defined, and including
from arch/x86/include/asm/acpi.h will lead to:
#error unknown ACPI_MACHINE_WIDTH
This can only be avoided by including from
which seems to be the wrong layering.
So I could:
a) modify the sources including to use
instead
b) don't use acpi_physical_address but either u64 or unsigned long.
c) ?
What would be your preference?
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/3] x86/acpi: take rsdp address for boot params if available
On 08/12/17 12:26, Ingo Molnar wrote:
>
> * Juergen Gross wrote:
>
>> On 08/12/17 08:05, Ingo Molnar wrote:
>>>
>>> * Juergen Gross wrote:
>>
>> ...
>>
>>> acpi_physical_address acpi_arch_get_root_pointer(void)
>>> {
>>> return boot_params.hdr.acpi_rsdp_addr;
>>> }
>>>
>>> 4)
>>>
>>> Add this to arch/x86/include/asm/acpi.h:
>>>
>>> extern acpi_physical_address acpi_arch_get_root_pointer(void);
>>
>> Uuh, this leads to problems for files including directly:
>> acpi_physical_address won't be defined, and including
>> from arch/x86/include/asm/acpi.h will lead to:
>>
>> #error unknown ACPI_MACHINE_WIDTH
>>
>> This can only be avoided by including from
>> which seems to be the wrong layering.
>>
>> So I could:
>>
>> a) modify the sources including to use
>>instead
>> b) don't use acpi_physical_address but either u64 or unsigned long.
>> c) ?
>>
>> What would be your preference?
>
> Would it help if you put the prototype into linux/acpi.h perhaps? It's a
> generic
> facility in principle, even if only used by x86 at the moment.
Yes, that seems to work.
Thanks,
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/Xen: don't report ancient LAPIC version
On 08/12/17 12:17, Jan Beulich wrote: > Unconditionally reporting a value seen on the P4 or older invokes > functionality like io_apic_get_unique_id() on 32-bit builds, resulting > in a panic() with sufficiently many CPUs and/or IO-APICs. Doing what > that function does would be the hypervisor's responsibility anyway, so > makes no sense to be used when running on Xen. Uniformly report a more > modern version; this shouldn't matter much as both LAPIC and IO-APIC are > being managed entirely / mostly by the hypervisor. > > Signed-off-by: Jan Beulich Reviewed-by: Juergen Gross BTW: Cc: stable? Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 3/4] x86/xen: fix boot loader version reported for pvh guests
The boot loader version reported via sysfs is wrong in case of the kernel being booted via the Xen PVH boot entry. it should be 2.12 (0x020c), but it is reported to be 2.18 (0x0212). As the current way to set the version is error prone use the more readable variant (2 << 8) | 12. Cc: # 4.12 Signed-off-by: Juergen Gross --- arch/x86/xen/enlighten_pvh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c index 436c4f003e17..6e6430cb5e3f 100644 --- a/arch/x86/xen/enlighten_pvh.c +++ b/arch/x86/xen/enlighten_pvh.c @@ -69,7 +69,7 @@ static void __init init_pvh_bootparams(void) * Version 2.12 supports Xen entry point but we will use default x86/PC * environment (i.e. hardware_subarch 0). */ - pvh_bootparams.hdr.version = 0x212; + pvh_bootparams.hdr.version = (2 << 8) | 12; pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */ } -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 1/4] x86/boot: add acpi rsdp address to setup_header
Xen PVH guests receive the address of the RSDP table from Xen. In order
to support booting a Xen PVH guest via Grub2 using the standard x86
boot entry we need a way for Grub2 to pass the RSDP address to the
kernel.
For this purpose expand the struct setup_header to hold the physical
address of the RSDP address. Being zero means it isn't specified and
has to be located the legacy way (searching through low memory or
EBDA).
While documenting the new setup_header layout and protocol version
2.14 add the missing documentation of protocol version 2.13.
Signed-off-by: Juergen Gross
Reviewed-by: Roger Pau Monné
---
V3: fix commit message and some documentation bits (Ingo Molnar)
---
Documentation/x86/boot.txt| 19 +++
arch/x86/boot/header.S| 6 +-
arch/x86/include/uapi/asm/bootparam.h | 1 +
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
index 5e9b826b5f62..cec112909c35 100644
--- a/Documentation/x86/boot.txt
+++ b/Documentation/x86/boot.txt
@@ -61,6 +61,13 @@ Protocol 2.12: (Kernel 3.8) Added the xloadflags field
and extension fields
to struct boot_params for loading bzImage and ramdisk
above 4G in 64bit.
+Protocol 2.13: (Kernel 3.14) Support 32- and 64-bit flags being set in
+ xloadflags to support booting a 64-bit kernel from 32-bit
+ EFI
+
+Protocol 2.14 (Kernel 4.16) Added acpi_rsdp_addr holding the physical
+ address of the ACPI RSDP table.
+
MEMORY LAYOUT
The traditional memory map for the kernel loader, used for Image or
@@ -197,6 +204,7 @@ Offset Proto NameMeaning
0258/8 2.10+ pref_addressPreferred loading address
0260/4 2.10+ init_size Linear memory required during initialization
0264/4 2.11+ handover_offset Offset of handover entry point
+0268/8 2.14+ acpi_rsdp_addr Physical address of RSDP table
(1) For backwards compatibility, if the setup_sects field contains 0, the
real value is 4.
@@ -744,6 +752,17 @@ Offset/size: 0x264/4
See EFI HANDOVER PROTOCOL below for more details.
+Field name:acpi_rsdp_addr
+Type: write
+Offset/size: 0x268/8
+Protocol: 2.14+
+
+ This field can be set by the boot loader to tell the kernel the
+ physical address of the ACPI RSDP table.
+
+ A value of 0 indicates the kernel should fall back to the standard
+ methods to locate the RSDP.
+
THE IMAGE CHECKSUM
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 850b8762e889..4c881c850125 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -300,7 +300,7 @@ _start:
# Part 2 of the header, from the old setup.S
.ascii "HdrS" # header signature
- .word 0x020d # header version number (>= 0x0105)
+ .word 0x020e # header version number (>= 0x0105)
# or else old loadlin-1.5 will fail)
.globl realmode_swtch
realmode_swtch:.word 0, 0# default_switch, SETUPSEG
@@ -558,6 +558,10 @@ pref_address: .quad LOAD_PHYSICAL_ADDR
# preferred load addr
init_size: .long INIT_SIZE # kernel initialization size
handover_offset: .long 0 # Filled in by build.c
+acpi_rsdp_addr:.quad 0 # 64-bit physical
pointer to the
+ # ACPI RSDP table, added with
+ # version 2.14
+
# End of setup header #
.section ".entrytext", "ax"
diff --git a/arch/x86/include/uapi/asm/bootparam.h
b/arch/x86/include/uapi/asm/bootparam.h
index afdd5ae0fcc4..5742e433e93e 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -85,6 +85,7 @@ struct setup_header {
__u64 pref_address;
__u32 init_size;
__u32 handover_offset;
+ __u64 acpi_rsdp_addr;
} __attribute__((packed));
struct sys_desc_table {
--
2.12.3
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 4/4] x86/xen: supply rsdp address in boot params for pvh guests
When booted via the special PVH entry save the RSDP address set in the boot information block in struct boot_params. This will enable Xen to locate the RSDP at an arbitrary address. Set the boot loader version to 2.14 (0x020e) replacing the wrong 0x0212 which should have been 0x020c. Signed-off-by: Juergen Gross --- V2: set bootloader version to 2.14 (Roger Pau Monné) --- arch/x86/xen/enlighten_pvh.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c index 6e6430cb5e3f..e85e6dafe4bc 100644 --- a/arch/x86/xen/enlighten_pvh.c +++ b/arch/x86/xen/enlighten_pvh.c @@ -68,9 +68,12 @@ static void __init init_pvh_bootparams(void) * * Version 2.12 supports Xen entry point but we will use default x86/PC * environment (i.e. hardware_subarch 0). +* The RSDP address is available from version 2.14 on. */ - pvh_bootparams.hdr.version = (2 << 8) | 12; + pvh_bootparams.hdr.version = (2 << 8) | 14; pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */ + + pvh_bootparams.hdr.acpi_rsdp_addr = pvh_start_info.rsdp_paddr; } /* -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 0/4] x86: make rsdp address accessible via boot params
In the non-EFI boot path the ACPI RSDP table is currently found via either EBDA or by searching through low memory for the RSDP magic. This requires the RSDP to be located in the first 1MB of physical memory. Xen PVH guests, however, get the RSDP address via the start of day information block. In order to support an arbitrary RSDP address this patch series adds the physical address of the RSDP to the boot params structure filled by the boot loader. A kernel booted directly in PVH mode can save the RSDP address in the boot params, while a kernel booted in PVH mode via grub can rely on the RSDP address being specified by grub2 (which in turn got the address via the start of day information block from Xen). Juergen Gross (4): x86/boot: add acpi rsdp address to setup_header x86/acpi: take rsdp address for boot params if available x86/xen: fix boot loader version reported for pvh guests x86/xen: supply rsdp address in boot params for pvh guests Documentation/x86/boot.txt| 19 +++ arch/x86/boot/header.S| 6 +- arch/x86/include/uapi/asm/bootparam.h | 1 + arch/x86/kernel/acpi/boot.c | 7 +++ arch/x86/xen/enlighten_pvh.c | 5 - drivers/acpi/osl.c| 10 +- include/linux/acpi.h | 2 ++ 7 files changed, 47 insertions(+), 3 deletions(-) -- 2.12.3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 2/4] x86/acpi: take rsdp address for boot params if available
In case the rsdp address in struct boot_params is specified don't try
to find the table by searching, but take the address directly as set
by the boot loader.
Signed-off-by: Juergen Gross
---
V3: use a generic retrieval function with a __weak annotated default
function (Ingo Molnar)
---
arch/x86/kernel/acpi/boot.c | 7 +++
drivers/acpi/osl.c | 10 +-
include/linux/acpi.h| 2 ++
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index f4c463df8b08..26fc8972dc4b 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -47,6 +47,7 @@
#include
#include
#include
+#include
#include "sleep.h" /* To include x86_acpi_suspend_lowlevel */
static int __initdata acpi_force = 0;
@@ -1758,3 +1759,9 @@ void __init arch_reserve_mem_area(acpi_physical_address
addr, size_t size)
e820__range_add(addr, size, E820_TYPE_ACPI);
e820__update_table_print();
}
+
+acpi_physical_address acpi_arch_get_root_pointer(void)
+{
+ return boot_params.hdr.acpi_rsdp_addr;
+}
+EXPORT_SYMBOL_GPL(acpi_arch_get_root_pointer);
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 3bb46cb24a99..2b77db914752 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -178,6 +178,11 @@ void acpi_os_vprintf(const char *fmt, va_list args)
#endif
}
+__weak acpi_physical_address acpi_arch_get_root_pointer(void)
+{
+ return 0;
+}
+
#ifdef CONFIG_KEXEC
static unsigned long acpi_rsdp;
static int __init setup_acpi_rsdp(char *arg)
@@ -189,12 +194,15 @@ early_param("acpi_rsdp", setup_acpi_rsdp);
acpi_physical_address __init acpi_os_get_root_pointer(void)
{
- acpi_physical_address pa = 0;
+ acpi_physical_address pa;
#ifdef CONFIG_KEXEC
if (acpi_rsdp)
return acpi_rsdp;
#endif
+ pa = acpi_arch_get_root_pointer();
+ if (pa)
+ return pa;
if (efi_enabled(EFI_CONFIG_TABLES)) {
if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index dc1ebfeeb5ec..aa603cc5ad30 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1266,4 +1266,6 @@ static inline int lpit_read_residency_count_address(u64
*address)
}
#endif
+acpi_physical_address acpi_arch_get_root_pointer(void);
+
#endif /*_LINUX_ACPI_H*/
--
2.12.3
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 4/4] x86/xen: supply rsdp address in boot params for pvh guests
On 11/12/17 11:09, Jan Beulich wrote: On 08.12.17 at 16:11, wrote: >> Set the boot loader version to 2.14 (0x020e) replacing the wrong 0x0212 >> which should have been 0x020c. > > This part of the description has become partly stale now with the > new patch 3. Indeed. I'll wait for other comments before sending out a new version. Thanks, Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 41/45] arch/x86: remove duplicate includes
On 11/12/17 21:42, Pravin Shedge wrote: > These duplicate includes have been found with scripts/checkincludes.pl but > they have been removed manually to avoid removing false positives. > > Signed-off-by: Pravin Shedge Reviewed-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: XEN_ACPI_PROCESSOR is Dom0-only
On 12/12/17 11:18, Jan Beulich wrote: > Add a respective dependency. > > Signed-off-by: Jan Beulich Reviewed-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] x86-64/Xen: eliminate W+X mappings
On 12/12/17 11:48, Jan Beulich wrote:
On 12.12.17 at 11:38, wrote:
>> * Jan Beulich wrote:
>>> --- 4.15-rc3/arch/x86/xen/mmu_pv.c
>>> +++ 4.15-rc3-x86_64-Xen-avoid-W+X/arch/x86/xen/mmu_pv.c
>>> @@ -1902,6 +1902,16 @@ void __init xen_setup_kernel_pagetable(p
>>> /* Graft it onto L4[511][510] */
>>> copy_page(level2_kernel_pgt, l2);
>>>
>>> + /* Zap execute permission from the ident map. Due to the sharing of
>>> +* L1 entries we need to do this in the L2. */
>>
>> please use the customary (multi-line) comment style:
>>
>> /*
>>* Comment .
>>* .. goes here.
>>*/
>>
>> specified in Documentation/CodingStyle.
>
> I would have but didn't because all other comments in this function
> use this (wrong) style. I've concluded that consistency is better
> here than matching the style doc. If the Xen maintainers tell me
> otherwise, I'll happily adjust the patch.
Yes, please use the correct style with new comments.
>
>>> + if (__supported_pte_mask & _PAGE_NX)
>>> + for (i = 0; i < PTRS_PER_PMD; ++i) {
>>> + if (pmd_none(level2_ident_pgt[i]))
>>> + continue;
>>> + level2_ident_pgt[i] =
>>> + pmd_set_flags(level2_ident_pgt[i], _PAGE_NX);
>>
>> So the line break here is quite distracting, especially considering how
>> similar it
>> is to the alignment of the 'continue' statement. I.e. visually it looks like
>> control flow alignment.
>>
>> Would be much better to just leave it a single page and ignore checkpatch
>> here.
>
> Again I'll wait to see what the Xen maintainers think. I too dislike
> line splits like this one, but the line ended up quite a bit too long,
> not just a character or two. I also wasn't sure whether splitting
> between the function arguments would be okay, leaving the first
> line just slightly too long.
That would result in a 80 character line, which IMHO is the best choice
here.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] docs/process/xen-release-management: Lesson to learn
On 13/12/17 13:02, Ian Jackson wrote: > The 4.10 release preparation was significantly more hairy than ideal. > (We seem to have a good overall outcome despite, rather than because > of, our approach.) > > This is the second time (at least) that we have come close to failure > by committing to a release date before the exact code to be released > is known and has been made and tested. > > Evidently our docs makes it insufficiently clear not to do that. > > CC: Lars Kurth > CC: Julien Grall > CC: Juergen Gross > Signed-off-by: Ian Jackson Reviewed-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Xen release cycle revisited
Hi all, with 4.10 more or less finished it is time to plan for the next release 4.11. Since 4.7 we are using a 6 month release cycle [1] targeting to release in June and December. While this worked reasonably well for 4.7, 4.8 and 4.9 we had some difficulties with 4.10: bad luck with security patch timing shifted the 4.10 release more towards mid of December. Doing thorough testing of the latest security patches and trying to release at least 10 days before Christmas seemed to be almost mutually exclusive goals. So what do we learn from this experience? 1. Should we think about other planned release dates (e.g. May and November - would that collide with any holiday season)? 2. Shouldn't we have tried to include the latest security patches in 4.10, resulting in the need for 4.10.1 at once? 3. Should we let the release slip for almost a month in such a case? 4. Should we try harder to negotiate embargo dates of security issues to match the (targeted) release dates? 5. Should we modify the development/hardening periods? For 4.11 we shouldn't have this problem: while targeted for releasing in early June it wouldn't be a nightmare to let it slip into July. 4.12 however will probably face the same problem again and we should prepare for that possibility. Juergen [1]: https://lists.xen.org/archives/html/xen-devel/2015-10/msg00263.html ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/8] xen: add pvh guest support
On 14/12/17 12:19, Daniel Kiper wrote: > On Fri, Dec 01, 2017 at 12:12:50PM +0100, Daniel Kiper wrote: >> On Fri, Dec 01, 2017 at 06:37:37AM +0100, Juergen Gross wrote: >>> On 30/11/17 22:03, Daniel Kiper wrote: >>>> On Wed, Nov 29, 2017 at 02:46:42PM +0100, Juergen Gross wrote: >>>>> This patch series adds support for booting Linux as PVH guest. >>>>> >>>>> Similar to i386/xen and x86_64/xen platforms the new i386/xenpvh >>>>> platform grub is booted as a standalone image directly by Xen. >>>>> >>>>> For booting Linux kernel it is using the standard linux kernel >>>>> loader. The only modification of the linux loader is to pass the >>>>> ACPI RSDP address via boot parameters to the kernel, as that table >>>>> might not be located at the usual physical address just below 1MB. >>>>> >>>>> As the related Linux kernel patches are not yet accepted please >>>>> wait for this to happen before applying the series. >>>> >>>> So, may I review the patches or should I hold on? And could you >>>> provide a link to the "Linux kernel patches" mentioned above? >>> >>> Please review! >> >> Will do in a week or so. > > Swamped by some urgent internal work. I will try to review this before > Xmas but if it do not happen then I will do it in the first halt of > January. Sorry for delays. Thanks. BTW: the Linux kernel patches have been accepted by the x86 maintainers. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen release cycle revisited
On 14/12/17 12:28, Julien Grall wrote: > > > On 14/12/17 07:56, Juergen Gross wrote: >> Hi all, > > Hi Juergen, > > I would recommend to CC committers on that thread, so your thread don't > get lost in the xen-devel meanders :). > >> with 4.10 more or less finished it is time to plan for the next release >> 4.11. Since 4.7 we are using a 6 month release cycle [1] targeting to >> release in June and December. >> >> While this worked reasonably well for 4.7, 4.8 and 4.9 we had some >> difficulties with 4.10: bad luck with security patch timing shifted the >> 4.10 release more towards mid of December. Doing thorough testing of the >> latest security patches and trying to release at least 10 days before >> Christmas seemed to be almost mutually exclusive goals. >> >> So what do we learn from this experience? >> >> 1. Should we think about other planned release dates (e.g. May and >> November - would that collide with any holiday season)? >> >> 2. Shouldn't we have tried to include the latest security patches in >> 4.10, resulting in the need for 4.10.1 at once? > > I am not sure to understand this questions here. Hmm, yes, this is somehow garbled. Next try: 2. Should we have released 4.10 without those late security patches, resulting in the need for 4.10.1 at once? > >> >> 3. Should we let the release slip for almost a month in such a case? > > The problem is XSAs can happen at any time. Let's imagine we decided to > release in January, what if a new security was discovered during > christmas? Are we going to slip the release again? Go back to 2. :-) > >> >> 4. Should we try harder to negotiate embargo dates of security issues to >> match the (targeted) release dates? > > Those 4 XSAs was first released under embargoed a couple of days before > the targeted release dates. > > The usual embargo period is 2 weeks. I think it would be difficult to > request a shorter embargo period because downstream product need time to > apply/test the security fixes. Right. What about a longer embargo so that it ends well after the release date? Last minute XSAs just before a 2-3 week period where a release can't happen (like at Xmas) are the problem. Juergen > >> >> 5. Should we modify the development/hardening periods? >> >> For 4.11 we shouldn't have this problem: while targeted for releasing in >> early June it wouldn't be a nightmare to let it slip into July. 4.12 >> however will probably face the same problem again and we should prepare >> for that possibility. >> >> >> Juergen >> >> [1]: https://lists.xen.org/archives/html/xen-devel/2015-10/msg00263.html > > Cheers, > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/8] xen: add pvh guest support
On 14/12/17 12:32, Daniel Kiper wrote: > On Thu, Dec 14, 2017 at 12:26:02PM +0100, Juergen Gross wrote: >> On 14/12/17 12:19, Daniel Kiper wrote: >>> On Fri, Dec 01, 2017 at 12:12:50PM +0100, Daniel Kiper wrote: >>>> On Fri, Dec 01, 2017 at 06:37:37AM +0100, Juergen Gross wrote: >>>>> On 30/11/17 22:03, Daniel Kiper wrote: >>>>>> On Wed, Nov 29, 2017 at 02:46:42PM +0100, Juergen Gross wrote: >>>>>>> This patch series adds support for booting Linux as PVH guest. >>>>>>> >>>>>>> Similar to i386/xen and x86_64/xen platforms the new i386/xenpvh >>>>>>> platform grub is booted as a standalone image directly by Xen. >>>>>>> >>>>>>> For booting Linux kernel it is using the standard linux kernel >>>>>>> loader. The only modification of the linux loader is to pass the >>>>>>> ACPI RSDP address via boot parameters to the kernel, as that table >>>>>>> might not be located at the usual physical address just below 1MB. >>>>>>> >>>>>>> As the related Linux kernel patches are not yet accepted please >>>>>>> wait for this to happen before applying the series. >>>>>> >>>>>> So, may I review the patches or should I hold on? And could you >>>>>> provide a link to the "Linux kernel patches" mentioned above? >>>>> >>>>> Please review! >>>> >>>> Will do in a week or so. >>> >>> Swamped by some urgent internal work. I will try to review this before >>> Xmas but if it do not happen then I will do it in the first halt of >>> January. Sorry for delays. >> >> Thanks. >> >> BTW: the Linux kernel patches have been accepted by the x86 maintainers. > > Perfect! Could you send me their commit ids? Right now they are in the tip tree in the x86/boot branch: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/boot Commit-Ids are: 2f74cbf947f45fa082dda8eac1a1f1299a372f49 0c89cf36424f7c1177de8a5712514d7cc2eb369f 88750a6c33f813b815516990f01fb5ee488c477e 930ba49b2ce7b09a5eddc21385fd944ba6b4e829 Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen release cycle revisited
On 14/12/17 13:43, Julien Grall wrote: > > > On 14/12/17 11:38, Juergen Gross wrote: >> On 14/12/17 12:28, Julien Grall wrote: >>> >>> >>> On 14/12/17 07:56, Juergen Gross wrote: >>>> Hi all, >>> >>> Hi Juergen, >>> >>> I would recommend to CC committers on that thread, so your thread don't >>> get lost in the xen-devel meanders :). >>> >>>> with 4.10 more or less finished it is time to plan for the next release >>>> 4.11. Since 4.7 we are using a 6 month release cycle [1] targeting to >>>> release in June and December. >>>> >>>> While this worked reasonably well for 4.7, 4.8 and 4.9 we had some >>>> difficulties with 4.10: bad luck with security patch timing shifted the >>>> 4.10 release more towards mid of December. Doing thorough testing of >>>> the >>>> latest security patches and trying to release at least 10 days before >>>> Christmas seemed to be almost mutually exclusive goals. >>>> >>>> So what do we learn from this experience? >>>> >>>> 1. Should we think about other planned release dates (e.g. May and >>>> November - would that collide with any holiday season)? >>>> >>>> 2. Shouldn't we have tried to include the latest security patches in >>>> 4.10, resulting in the need for 4.10.1 at once? >>> >>> I am not sure to understand this questions here. >> >> Hmm, yes, this is somehow garbled. >> >> Next try: >> >> 2. Should we have released 4.10 without those late security patches, >> resulting in the need for 4.10.1 at once? > > We were not ready to release on the 2nd December. This would have put > the release date too close to XSAs published date. The risk was that the > security issues announcement would overshadow the release announcement. Okay. So for me it seems as if a planned release early December is the main problem: either the release slips no more than 2 weeks or it will slip for more than 5 weeks. Having only 2 weeks of spare time is a major risk. > >> >>> >>>> >>>> 3. Should we let the release slip for almost a month in such a case? >>> >>> The problem is XSAs can happen at any time. Let's imagine we decided to >>> release in January, what if a new security was discovered during >>> christmas? Are we going to slip the release again? >> >> Go back to 2. :-) >> >>> >>>> >>>> 4. Should we try harder to negotiate embargo dates of security >>>> issues to >>>> match the (targeted) release dates? >>> >>> Those 4 XSAs was first released under embargoed a couple of days before >>> the targeted release dates. >>> >>> The usual embargo period is 2 weeks. I think it would be difficult to >>> request a shorter embargo period because downstream product need time to >>> apply/test the security fixes. >> >> Right. What about a longer embargo so that it ends well after the >> release date? Last minute XSAs just before a 2-3 week period where >> a release can't happen (like at Xmas) are the problem. > > I guess that could work. The security team would have to convince the > discoverer if he/she is happy with it. Sure, like Ian pointed out in another thread. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86: consider effective protection attributes in W+X check
for (i = 0; i < PTRS_PER_PMD; i++) {
> st->current_address = normalize_addr(P + i * PMD_LEVEL_MULT);
> if (!pmd_none(*start)) {
> + prot = pmd_flags(*start);
> + eff = effective_prot(eff_in, prot);
> if (pmd_large(*start) || !pmd_present(*start)) {
> - prot = pmd_flags(*start);
> - note_page(m, st, __pgprot(prot), 4);
> + note_page(m, st, __pgprot(prot), eff, 4);
> } else if (!kasan_page_table(m, st, pmd_start)) {
> - walk_pte_level(m, st, *start,
> + walk_pte_level(m, st, *start, eff,
> P + i * PMD_LEVEL_MULT);
> }
You can drop the braces for both cases. Applies to similar
constructs below, too.
With that fixed you can add my:
Reviewed-by: Juergen Gross
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86: consider effective protection attributes in W+X check
On 14/12/17 15:15, Jan Beulich wrote:
>>>> On 14.12.17 at 15:04, wrote:
>> On 12/12/17 11:31, Jan Beulich wrote:
>>> @@ -335,42 +346,45 @@ static inline bool kasan_page_table(stru
>>>
>>> #if PTRS_PER_PMD > 1
>>>
>>> -static void walk_pmd_level(struct seq_file *m, struct pg_state *st, pud_t
>>> addr, unsigned long P)
>>> +static void walk_pmd_level(struct seq_file *m, struct pg_state *st, pud_t
>>> addr,
>>> + pgprotval_t eff_in, unsigned long P)
>>> {
>>> int i;
>>> pmd_t *start, *pmd_start;
>>> - pgprotval_t prot;
>>> + pgprotval_t prot, eff;
>>>
>>> pmd_start = start = (pmd_t *)pud_page_vaddr(addr);
>>> for (i = 0; i < PTRS_PER_PMD; i++) {
>>> st->current_address = normalize_addr(P + i * PMD_LEVEL_MULT);
>>> if (!pmd_none(*start)) {
>>> + prot = pmd_flags(*start);
>>> + eff = effective_prot(eff_in, prot);
>>> if (pmd_large(*start) || !pmd_present(*start)) {
>>> - prot = pmd_flags(*start);
>>> - note_page(m, st, __pgprot(prot), 4);
>>> + note_page(m, st, __pgprot(prot), eff, 4);
>>> } else if (!kasan_page_table(m, st, pmd_start)) {
>>> - walk_pte_level(m, st, *start,
>>> + walk_pte_level(m, st, *start, eff,
>>>P + i * PMD_LEVEL_MULT);
>>> }
>>
>> You can drop the braces for both cases. Applies to similar
>> constructs below, too.
>
> I did consider that, but decided against to allow the patch to show
> more clearly what it is that is actually being changed.
>
>> With that fixed you can add my:
>>
>> Reviewed-by: Juergen Gross
>
> Thanks. I'd like to wait for the x86 maintainer's opinion, and hence
> won't add your R-b unless you tell me that's fine either way, or
> unless they too would prefer resulting code cleanliness over patch
> readability.
I'm fine with the braces kept, too.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Tree is open again
Committers, as 4.10 has been released, feel free to commit patches again. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [GIT PULL] xen: fixes for 4.15-rc4
Linus, Please git pull the following tag: git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git for-linus-4.15-rc4-tag xen: fixes for 4.15-rc4 It contains two minor fixes for running as Xen dom0: - when built as 32 bit kernel on large machines the Xen LAPIC emulation should report a rather modern LAPIC in order to support enough APIC-Ids - The Xen LAPIC emulation is needed for dom0 only, so build it only for kernels supporting to run as Xen dom0 Thanks. Juergen arch/x86/xen/apic.c | 2 +- drivers/xen/Kconfig | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) Jan Beulich (2): x86/Xen: don't report ancient LAPIC version xen: XEN_ACPI_PROCESSOR is Dom0-only ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/balloon: Mark unallocated host memory as UNUSABLE
On 12/12/17 23:51, Boris Ostrovsky wrote:
> Commit f5775e0b6116 ("x86/xen: discard RAM regions above the maximum
> reservation") left host memory not assigned to dom0 as available for
> memory hotplug.
>
> Unfortunately this also meant that those regions could be used by
> others. Specifically, commit fa564ad96366 ("x86/PCI: Enable a 64bit BAR
> on AMD Family 15h (Models 00-1f, 30-3f, 60-7f)") may try to map those
> addresses as MMIO.
>
> To prevent this mark unallocated host memory as E820_TYPE_UNUSABLE (thus
> effectively reverting f5775e0b6116) and keep track of that region as
> a hostmem resource that can be used for the hotplug.
>
> Signed-off-by: Boris Ostrovsky
> ---
> I don't see /proc/meminfo reporting the hotplugged memory (although
> internal data such as max_pfn is updated properly). Need to look at
> hotplug code some more. But then I didn't see meminfo changing with
> existing code either.
>
>
> arch/x86/xen/enlighten.c | 69
>
> arch/x86/xen/setup.c | 6 ++---
> drivers/xen/balloon.c| 65 ++---
> 3 files changed, 127 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index d669e9d..19223b9 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -3,6 +3,7 @@
>
> #include
> #include
> +#include
>
> #include
> #include
> @@ -331,3 +332,71 @@ void xen_arch_unregister_cpu(int num)
> }
> EXPORT_SYMBOL(xen_arch_unregister_cpu);
> #endif
> +
> +#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
> +void __init arch_xen_balloon_init(struct resource *hostmem_resource)
> +{
> + struct xen_memory_map memmap;
> + int rc, i, last_guest_ram;
i and last_guest_ram should be unsigned int
> + unsigned long max_addr = max_pfn << PAGE_SHIFT;
This might overflow for 32 bit builds
> + struct e820_table *xen_e820_table;
> + struct e820_entry *entry;
> + struct resource *res = NULL;
> +
> + if (!xen_initial_domain())
> + return;
> +
> + xen_e820_table = kzalloc(sizeof(*xen_e820_table), GFP_KERNEL);
> + if (!xen_e820_table) {
> + pr_warn("%s: Out of memory\n", __func__);
> + return;
> + }
> +
> + memmap.nr_entries = ARRAY_SIZE(xen_e820_table->entries);
> + set_xen_guest_handle(memmap.buffer, xen_e820_table->entries);
> + rc = HYPERVISOR_memory_op(XENMEM_machine_memory_map, &memmap);
> + if (rc) {
> + pr_warn("%s: Can't read host e820 (%d)\n", __func__, rc);
> + goto out;
> + }
> +
> + last_guest_ram = i = 0;
> + while (xen_e820_table->entries[i].addr < max_addr) {
> + if (xen_e820_table->entries[i].type == E820_TYPE_RAM)
> + last_guest_ram = i;
> + i++;
Check for i < memmap.nr_entries?
> + }
> +
> + entry = &xen_e820_table->entries[last_guest_ram];
> + if (max_addr >= entry->addr + entry->size)
> + goto out; /* No unallocated host RAM. */
> +
> + hostmem_resource->start = max_addr;
> + hostmem_resource->end = entry->addr + entry->size;
> + for (; i < memmap.nr_entries; i++) {
> + entry = &xen_e820_table->entries[i];
> + if (entry->type == E820_TYPE_RAM)
Shouldn't that be != ?
> + continue;
> +
> + res = kzalloc(sizeof(*res), GFP_KERNEL);
> + if (!res) {
> + pr_warn("%s: Out of memory\n", __func__);
Don't print error message in case of allocation failures.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/balloon: Mark unallocated host memory as UNUSABLE
On 15/12/17 15:24, Boris Ostrovsky wrote:
>
>>> +
>>> + hostmem_resource->start = max_addr;
>>> + hostmem_resource->end = entry->addr + entry->size;
>>> + for (; i < memmap.nr_entries; i++) {
>>> + entry = &xen_e820_table->entries[i];
>>> + if (entry->type == E820_TYPE_RAM)
>> Shouldn't that be != ?
>
> No, the idea here is to populate hostmem_resource with ranges already
> taken by things other than RAM, leaving memory regions as available for
> the balloon hotplug. This will allow us to use allocate_resource(),
> which searches for a free range, in the balloon driver.
But why says the comment "Host memory not allocated to dom0" then?
And why are you trying to allocate from this resource in case of
hotplugging memory (and fall back to iomem_resource in case of
failure)?
Either the comment is completely wrong and I don't understand the
logic here, or your code is wrong.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen release cycle revisited
On 14/12/17 14:13, Juergen Gross wrote: > On 14/12/17 13:43, Julien Grall wrote: >> >> >> On 14/12/17 11:38, Juergen Gross wrote: >>> On 14/12/17 12:28, Julien Grall wrote: >>>> >>>> >>>> On 14/12/17 07:56, Juergen Gross wrote: >>>>> Hi all, >>>> >>>> Hi Juergen, >>>> >>>> I would recommend to CC committers on that thread, so your thread don't >>>> get lost in the xen-devel meanders :). >>>> >>>>> with 4.10 more or less finished it is time to plan for the next release >>>>> 4.11. Since 4.7 we are using a 6 month release cycle [1] targeting to >>>>> release in June and December. >>>>> >>>>> While this worked reasonably well for 4.7, 4.8 and 4.9 we had some >>>>> difficulties with 4.10: bad luck with security patch timing shifted the >>>>> 4.10 release more towards mid of December. Doing thorough testing of >>>>> the >>>>> latest security patches and trying to release at least 10 days before >>>>> Christmas seemed to be almost mutually exclusive goals. >>>>> >>>>> So what do we learn from this experience? >>>>> >>>>> 1. Should we think about other planned release dates (e.g. May and >>>>> November - would that collide with any holiday season)? >>>>> >>>>> 2. Shouldn't we have tried to include the latest security patches in >>>>> 4.10, resulting in the need for 4.10.1 at once? >>>> >>>> I am not sure to understand this questions here. >>> >>> Hmm, yes, this is somehow garbled. >>> >>> Next try: >>> >>> 2. Should we have released 4.10 without those late security patches, >>> resulting in the need for 4.10.1 at once? >> >> We were not ready to release on the 2nd December. This would have put >> the release date too close to XSAs published date. The risk was that the >> security issues announcement would overshadow the release announcement. > > Okay. So for me it seems as if a planned release early December is the > main problem: either the release slips no more than 2 weeks or it will > slip for more than 5 weeks. > > Having only 2 weeks of spare time is a major risk. What I'd like to suggest is to move the target release dates to early May and November. Or would this create a conflict with any holiday season we care about? In order not to have a very short release cycle for 4.11 I'd do the shift in two steps (so 4.11 target release date mid of May, 4.12 early November). What do you think? Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/balloon: Mark unallocated host memory as UNUSABLE
On 15/12/17 15:58, Boris Ostrovsky wrote:
> On 12/15/2017 09:47 AM, Juergen Gross wrote:
>> On 15/12/17 15:24, Boris Ostrovsky wrote:
>>>>> +
>>>>> + hostmem_resource->start = max_addr;
>>>>> + hostmem_resource->end = entry->addr + entry->size;
>>>>> + for (; i < memmap.nr_entries; i++) {
>>>>> + entry = &xen_e820_table->entries[i];
>>>>> + if (entry->type == E820_TYPE_RAM)
>>>> Shouldn't that be != ?
>>> No, the idea here is to populate hostmem_resource with ranges already
>>> taken by things other than RAM, leaving memory regions as available for
>>> the balloon hotplug. This will allow us to use allocate_resource(),
>>> which searches for a free range, in the balloon driver.
>> But why says the comment "Host memory not allocated to dom0" then?
>
> hostmem_resource is created starting from the end of dom0 RAM and ending
> at the end of host RAM. If these two are the same (or if the former is
> larger, which I don't think is possible) then there is nothing to do, as
> the hostmem_resource will be empty. That's what the comment is referring
> to.
>
>> And why are you trying to allocate from this resource in case of
>> hotplugging memory (and fall back to iomem_resource in case of
>> failure)?
>
> Because that area (end of dom0 RAM through end of host RAM) is not going
> to be used by anyone else and thus is available. That was the idea
> behind f5775e0b6116.
Aah, now I've got it.
Could you please add a comment like:
/* Mark non-RAM regions as not available. */
above the test? That would have helped me. :-)
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH v3 2/2] KVM: x86: Allow Qemu/KVM to use PVH entry point
On 13/12/17 00:42, Maran Wilson wrote:
> For certain applications it is desirable to rapidly boot a KVM virtual
> machine. In cases where legacy hardware and software support within the
> guest is not needed, Qemu should be able to boot directly into the
> uncompressed Linux kernel binary without the need to run firmware.
>
> There already exists an ABI to allow this for Xen PVH guests and the ABI
> is supported by Linux and FreeBSD:
>
>https://xenbits.xen.org/docs/unstable/misc/pvh.html
>
> This patch enables Qemu to use that same entry point for booting KVM
> guests.
I'm fine with the general idea.
I'm wondering whether you really want to require CONFIG_XEN for the
KVM case, though.
Wouldn't it be better to rename arch/x86/xen/enlighten_pvh.c to
arch/x86/pvh.c and arch/x86/xen/xen-pvh.S to arch/x86/pvh-head.S,
put both under CONFIG_PVH umbrella and select this from CONFIG_XEN_PVH
and KVM_PVH (or what you like to call it)?
In the two moved source files you can make Xen/KVM-specific parts
optional via their CONFIG_ options.
And you might want to add an own ELF note for the KVM case?
> ---
> arch/x86/xen/enlighten_pvh.c | 51
> +++-
> 1 file changed, 36 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
> index 98ab176..12f3716 100644
> --- a/arch/x86/xen/enlighten_pvh.c
> +++ b/arch/x86/xen/enlighten_pvh.c
> @@ -31,21 +31,38 @@ static void xen_pvh_arch_setup(void)
> acpi_irq_model = ACPI_IRQ_MODEL_PLATFORM;
> }
>
> -static void __init init_pvh_bootparams(void)
> +static void __init init_pvh_bootparams(bool xen_guest)
> {
> struct xen_memory_map memmap;
> int rc;
>
> memset(&pvh_bootparams, 0, sizeof(pvh_bootparams));
>
> - memmap.nr_entries = ARRAY_SIZE(pvh_bootparams.e820_table);
> - set_xen_guest_handle(memmap.buffer, pvh_bootparams.e820_table);
> - rc = HYPERVISOR_memory_op(XENMEM_memory_map, &memmap);
> - if (rc) {
> - xen_raw_printk("XENMEM_memory_map failed (%d)\n", rc);
> + if ((pvh_start_info.version > 0) && (pvh_start_info.memmap_entries)) {
> + struct hvm_memmap_table_entry *ep;
> + int i;
> +
> + ep = __va(pvh_start_info.memmap_paddr);
> + pvh_bootparams.e820_entries = pvh_start_info.memmap_entries;
> +
> + for (i = 0; i < pvh_bootparams.e820_entries ; i++, ep++) {
> + pvh_bootparams.e820_table[i].addr = ep->addr;
> + pvh_bootparams.e820_table[i].size = ep->size;
> + pvh_bootparams.e820_table[i].type = ep->type;
> + }
> + } else if (xen_guest) {
> + memmap.nr_entries = ARRAY_SIZE(pvh_bootparams.e820_table);
> + set_xen_guest_handle(memmap.buffer, pvh_bootparams.e820_table);
> + rc = HYPERVISOR_memory_op(XENMEM_memory_map, &memmap);
> + if (rc) {
> + xen_raw_printk("XENMEM_memory_map failed (%d)\n", rc);
> + BUG();
> + }
> + pvh_bootparams.e820_entries = memmap.nr_entries;
> + } else {
> + xen_raw_printk("Error: Could not find memory map\n");
xen_raw_printk() without being a Xen guest?
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] swiotlb-zen: Convert to use macro
On 01.09.19 21:28, Souptick Joarder wrote: Rather than using static int max_dma_bits, this can be coverted to use as macro. Signed-off-by: Souptick Joarder s/zen/xen/ in the patch title, other than that: Reviewed-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 3/3] xen/sched: add minimalistic idle scheduler for free cpus
On 02.09.19 13:06, Jan Beulich wrote:
On 27.08.2019 14:40, Juergen Gross wrote:
On 27.08.19 14:37, Andrew Cooper wrote:
On 27/08/2019 11:59, Juergen Gross wrote:
+static void *
+sched_idle_alloc_vdata(const struct scheduler *ops, struct vcpu *v,
+ void *dd)
+{
+/* Any non-NULL pointer is fine here. */
+return (void *)1UL;
As an observation, the vdata interface (and others, if applicable) could
do with being updated to use ERR_PTR(), just as done in c/s 340edc390
One of the items for my scheduler cleanup patches.
Passing 1 back here is rather dangerous.
Not really. vdata is scheduler specific, and the idle-scheduler doesn't
use it.
But maybe handing back e.g. ZERO_BLOCK_PTR would still be better?
Not the least because of avoiding an open-coded cast?
Yes. I wasn't aware of its existance.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] xen: add macro for defining variable length array in public headers
Several public headers of the hypervisor contain structures with
variable length arrays. In order to be usable with different compilers
those definitions are depending on the compiler type and the standard
supported by the compiler.
In order to avoid open coding the different variants in each header
add a common macro for that purpose in xen.h.
This at once corrects most of the definitions which miss one case
leading to not defining the array at all.
Signed-off-by: Juergen Gross
---
V2:
- rename macro (Jan Beulich)
---
xen/include/public/arch-x86/hvm/save.h | 8 +---
xen/include/public/arch-x86/pmu.h | 12 ++--
xen/include/public/argo.h | 18 +++---
xen/include/public/physdev.h | 6 +-
xen/include/public/version.h | 7 ++-
xen/include/public/xen-compat.h| 2 ++
xen/include/public/xen.h | 9 +
7 files changed, 20 insertions(+), 42 deletions(-)
diff --git a/xen/include/public/arch-x86/hvm/save.h
b/xen/include/public/arch-x86/hvm/save.h
index 8344aa471f..bb8fa7c12f 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -632,13 +632,7 @@ struct hvm_msr {
uint32_t index;
uint32_t _rsvd;
uint64_t val;
-#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
-} msr[];
-#elif defined(__GNUC__)
-} msr[0];
-#else
-} msr[1 /* variable size */];
-#endif
+} msr[XEN_FLEX_ARRAY_DIM];
};
#define CPU_MSR_CODE 20
diff --git a/xen/include/public/arch-x86/pmu.h
b/xen/include/public/arch-x86/pmu.h
index 68ebf121d0..c421cb7a4a 100644
--- a/xen/include/public/arch-x86/pmu.h
+++ b/xen/include/public/arch-x86/pmu.h
@@ -35,11 +35,7 @@ struct xen_pmu_amd_ctxt {
uint32_t ctrls;
/* Counter MSRs */
-#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
-uint64_t regs[];
-#elif defined(__GNUC__)
-uint64_t regs[0];
-#endif
+uint64_t regs[XEN_FLEX_ARRAY_DIM];
};
typedef struct xen_pmu_amd_ctxt xen_pmu_amd_ctxt_t;
DEFINE_XEN_GUEST_HANDLE(xen_pmu_amd_ctxt_t);
@@ -71,11 +67,7 @@ struct xen_pmu_intel_ctxt {
uint64_t debugctl;
/* Fixed and architectural counter MSRs */
-#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
-uint64_t regs[];
-#elif defined(__GNUC__)
-uint64_t regs[0];
-#endif
+uint64_t regs[XEN_FLEX_ARRAY_DIM];
};
typedef struct xen_pmu_intel_ctxt xen_pmu_intel_ctxt_t;
DEFINE_XEN_GUEST_HANDLE(xen_pmu_intel_ctxt_t);
diff --git a/xen/include/public/argo.h b/xen/include/public/argo.h
index cc603d395d..6b645f34e6 100644
--- a/xen/include/public/argo.h
+++ b/xen/include/public/argo.h
@@ -82,11 +82,7 @@ typedef struct xen_argo_ring
* multiple of the message slot size.
*/
uint8_t reserved[56];
-#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
-uint8_t ring[];
-#elif defined(__GNUC__)
-uint8_t ring[0];
-#endif
+uint8_t ring[XEN_FLEX_ARRAY_DIM];
} xen_argo_ring_t;
typedef struct xen_argo_register_ring
@@ -136,11 +132,7 @@ typedef struct xen_argo_ring_data
{
uint32_t nent;
uint32_t pad;
-#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
-struct xen_argo_ring_data_ent data[];
-#elif defined(__GNUC__)
-struct xen_argo_ring_data_ent data[0];
-#endif
+struct xen_argo_ring_data_ent data[XEN_FLEX_ARRAY_DIM];
} xen_argo_ring_data_t;
struct xen_argo_ring_message_header
@@ -148,11 +140,7 @@ struct xen_argo_ring_message_header
uint32_t len;
struct xen_argo_addr source;
uint32_t message_type;
-#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
-uint8_t data[];
-#elif defined(__GNUC__)
-uint8_t data[0];
-#endif
+uint8_t data[XEN_FLEX_ARRAY_DIM];
};
/*
diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
index b6faf8350c..aedb71d678 100644
--- a/xen/include/public/physdev.h
+++ b/xen/include/public/physdev.h
@@ -300,11 +300,7 @@ struct physdev_pci_device_add {
* First element ([0]) is PXM domain associated with the device (if
* XEN_PCI_DEV_PXM is set)
*/
-#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
-uint32_t optarr[];
-#elif defined(__GNUC__)
-uint32_t optarr[0];
-#endif
+uint32_t optarr[XEN_FLEX_ARRAY_DIM];
};
typedef struct physdev_pci_device_add physdev_pci_device_add_t;
DEFINE_XEN_GUEST_HANDLE(physdev_pci_device_add_t);
diff --git a/xen/include/public/version.h b/xen/include/public/version.h
index 7063e8ca55..17a81e23cd 100644
--- a/xen/include/public/version.h
+++ b/xen/include/public/version.h
@@ -95,11 +95,8 @@ typedef char xen_commandline_t[1024];
#define XENVER_build_id 10
struct xen_build_id {
uint32_tlen; /* IN: size of buf[]. */
-#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
-unsigned char buf[];
-#elif defined(__GNUC__)
-unsigned ch
[Xen-devel] [PATCH] tools/libs: put common Makefile parts into new libs.mk
The Makefile below tools/libs have a lot in common. Put those common parts into a new libs.mk and include that from the specific Makefiles. Signed-off-by: Juergen Gross --- tools/libs/call/Makefile | 86 ++- tools/libs/devicemodel/Makefile | 88 ++-- tools/libs/evtchn/Makefile| 86 ++- tools/libs/foreignmemory/Makefile | 86 ++- tools/libs/gnttab/Makefile| 86 ++- tools/libs/libs.mk| 95 +++ tools/libs/toolcore/Makefile | 85 +-- tools/libs/toollog/Makefile | 84 +- 8 files changed, 114 insertions(+), 582 deletions(-) create mode 100644 tools/libs/libs.mk diff --git a/tools/libs/call/Makefile b/tools/libs/call/Makefile index 6291e6dfe7..d30476add6 100644 --- a/tools/libs/call/Makefile +++ b/tools/libs/call/Makefile @@ -3,11 +3,8 @@ include $(XEN_ROOT)/tools/Rules.mk MAJOR= 1 MINOR= 2 -SHLIB_LDFLAGS += -Wl,--version-script=libxencall.map - -CFLAGS += -Werror -Wmissing-prototypes -CFLAGS += -I./include $(CFLAGS_xeninclude) -CFLAGS += $(CFLAGS_libxentoollog) $(CFLAGS_libxentoolcore) +LIBNAME := call +USELIBS := toollog toolcore SRCS-y += core.c buffer.c SRCS-$(CONFIG_Linux) += linux.c @@ -16,84 +13,7 @@ SRCS-$(CONFIG_SunOS) += solaris.c SRCS-$(CONFIG_NetBSD) += netbsd.c SRCS-$(CONFIG_MiniOS) += minios.c -LIB_OBJS := $(patsubst %.c,%.o,$(SRCS-y)) -PIC_OBJS := $(patsubst %.c,%.opic,$(SRCS-y)) - -LIB := libxencall.a -ifneq ($(nosharedlibs),y) -LIB += libxencall.so -endif - -PKG_CONFIG := xencall.pc -PKG_CONFIG_VERSION := $(MAJOR).$(MINOR) - -ifneq ($(CONFIG_LIBXC_MINIOS),y) -PKG_CONFIG_INST := $(PKG_CONFIG) -$(PKG_CONFIG_INST): PKG_CONFIG_PREFIX = $(prefix) -$(PKG_CONFIG_INST): PKG_CONFIG_INCDIR = $(includedir) -$(PKG_CONFIG_INST): PKG_CONFIG_LIBDIR = $(libdir) -endif - -PKG_CONFIG_LOCAL := $(foreach pc,$(PKG_CONFIG),$(PKG_CONFIG_DIR)/$(pc)) +include ../libs.mk -$(PKG_CONFIG_LOCAL): PKG_CONFIG_PREFIX = $(XEN_ROOT) $(PKG_CONFIG_LOCAL): PKG_CONFIG_INCDIR = $(XEN_LIBXENCALL)/include -$(PKG_CONFIG_LOCAL): PKG_CONFIG_LIBDIR = $(CURDIR) $(PKG_CONFIG_LOCAL): PKG_CONFIG_CFLAGS_LOCAL = $(CFLAGS_xeninclude) - -.PHONY: all -all: build - -.PHONY: build -build: - $(MAKE) libs - -.PHONY: libs -libs: headers.chk $(LIB) $(PKG_CONFIG_INST) $(PKG_CONFIG_LOCAL) - -headers.chk: $(wildcard include/*.h) - -libxencall.a: $(LIB_OBJS) - $(AR) rc $@ $^ - -libxencall.so: libxencall.so.$(MAJOR) - $(SYMLINK_SHLIB) $< $@ -libxencall.so.$(MAJOR): libxencall.so.$(MAJOR).$(MINOR) - $(SYMLINK_SHLIB) $< $@ - -libxencall.so.$(MAJOR).$(MINOR): $(PIC_OBJS) libxencall.map - $(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxencall.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $(PIC_OBJS) $(LDLIBS_libxentoollog) $(LDLIBS_libxentoolcore) $(APPEND_LDFLAGS) - -.PHONY: install -install: build - $(INSTALL_DIR) $(DESTDIR)$(libdir) - $(INSTALL_DIR) $(DESTDIR)$(includedir) - $(INSTALL_SHLIB) libxencall.so.$(MAJOR).$(MINOR) $(DESTDIR)$(libdir) - $(INSTALL_DATA) libxencall.a $(DESTDIR)$(libdir) - $(SYMLINK_SHLIB) libxencall.so.$(MAJOR).$(MINOR) $(DESTDIR)$(libdir)/libxencall.so.$(MAJOR) - $(SYMLINK_SHLIB) libxencall.so.$(MAJOR) $(DESTDIR)$(libdir)/libxencall.so - $(INSTALL_DATA) include/xencall.h $(DESTDIR)$(includedir) - $(INSTALL_DATA) xencall.pc $(DESTDIR)$(PKG_INSTALLDIR) - -.PHONY: uninstall -uninstall: - rm -f $(DESTDIR)$(PKG_INSTALLDIR)/xencall.pc - rm -f $(DESTDIR)$(includedir)/xencall.h - rm -f $(DESTDIR)$(libdir)/libxencall.so - rm -f $(DESTDIR)$(libdir)/libxencall.so.$(MAJOR) - rm -f $(DESTDIR)$(libdir)/libxencall.so.$(MAJOR).$(MINOR) - rm -f $(DESTDIR)$(libdir)/libxencall.a - -.PHONY: TAGS -TAGS: - etags -t *.c *.h - -.PHONY: clean -clean: - rm -rf *.rpm $(LIB) *~ $(DEPS_RM) $(LIB_OBJS) $(PIC_OBJS) - rm -f libxencall.so.$(MAJOR).$(MINOR) libxencall.so.$(MAJOR) - rm -f headers.chk - rm -f xencall.pc - -.PHONY: distclean -distclean: clean diff --git a/tools/libs/devicemodel/Makefile b/tools/libs/devicemodel/Makefile index 73cff6dbc4..702ec24673 100644 --- a/tools/libs/devicemodel/Makefile +++ b/tools/libs/devicemodel/Makefile @@ -3,13 +3,8 @@ include $(XEN_ROOT)/tools/Rules.mk MAJOR= 1 MINOR= 3 -SHLIB_LDFLAGS += -Wl,--version-script=libxendevicemodel.map - -CFLAGS += -Werror -Wmissing-prototypes -CFLAGS += -I./include $(CFLAGS_xeninclude) -CFLAGS += $(CFLAGS_libxentoollog) -CFLAGS += $(CFLAGS_libxentoolcore) -CFLAGS += $(CFLAGS_libxencall) +LIBNAME := devicemodel +USELIBS := toollog toolcore call SRCS-y += core.c SRCS-$(CONFIG_Linux) += linux.c @@ -18,84 +13,7 @@ SRCS-$(CONFIG
Re: [Xen-devel] [PATCH v3 1/4] xen: use common output function in debugtrace
On 03.09.19 12:00, Jan Beulich wrote: On 28.08.2019 10:00, Juergen Gross wrote: Today dumping the debugtrace buffers is done via sercon_puts(), while direct printing of trace entries (after toggling output to the console) is using serial_puts(). Use sercon_puts() in both cases, as the difference between both is not really making sense. No matter that I like this change, I'm not convinced it's correct: There are two differences between the functions, neither of which I could qualify as "not really making sense": Why is it obvious that it makes no sense for the debugtrace output to bypass one or both of serial_steal_fn() and pv_console_puts()? If you're convinced of this, please provide the "why"-s behind the sentence above. Okay, I'll use: Use sercon_puts() in both cases, to be consistent between dumping the buffer when switching debugtrace to console output and when printing a debugtrace entry directly to console. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/4] xen: refactor debugtrace data
On 03.09.19 12:16, Jan Beulich wrote:
On 28.08.2019 10:00, Juergen Gross wrote:
As a preparation for per-cpu buffers do a little refactoring of the
debugtrace data: put the needed buffer admin data into the buffer as
it will be needed for each buffer.
While at it switch debugtrace_send_to_console and debugtrace_used to
bool and delete an empty line.
Signed-off-by: Juergen Gross
---
xen/common/debugtrace.c | 62 -
1 file changed, 35 insertions(+), 27 deletions(-)
diff --git a/xen/common/debugtrace.c b/xen/common/debugtrace.c
index a2fab0d72c..7a9e4fb99f 100644
--- a/xen/common/debugtrace.c
+++ b/xen/common/debugtrace.c
@@ -15,33 +15,39 @@
#include
/* Send output direct to console, or buffer it? */
-static volatile int debugtrace_send_to_console;
+static volatile bool debugtrace_send_to_console;
-static char*debugtrace_buf; /* Debug-trace buffer */
-static unsigned int debugtrace_prd; /* Producer index */
-static unsigned int debugtrace_kilobytes = 128, debugtrace_bytes;
-static unsigned int debugtrace_used;
+struct debugtrace_data_s {
Is the _s suffix really needed for some reason?
No, not really. I can drop it.
+unsigned long bytes; /* Size of buffer. */
+unsigned long prd; /* Producer index. */
Seeing that you switch from int to long here - are you really
fancying these buffers to go beyond 4GiB in size?
I didn't want to rule it out on a multi-TB machine. :-)
+char buf[];
+};
+
+static struct debugtrace_data_s *debtr_data;
How about s/debtr/dt/ or s/debtr/debugtrace/ ?
Yes, dt seems to be fine.
+static unsigned int debugtrace_kilobytes = 128;
Since you touch this anyway, add __initdata? Maybe also move it
next to its integer_param()?
This is modified in patch 4 again, and there I need it in the running
system for support of cpu hotplug with per-cpu buffers.
+static bool debugtrace_used;
static DEFINE_SPINLOCK(debugtrace_lock);
integer_param("debugtrace", debugtrace_kilobytes);
static void debugtrace_dump_worker(void)
{
-if ( (debugtrace_bytes == 0) || !debugtrace_used )
+if ( !debtr_data || !debugtrace_used )
return;
Why don't you get rid of the left side of the || altogether?
In patch 4 I do. :-)
I can drop it here already.
@@ -165,12 +171,14 @@ static int __init debugtrace_init(void)
return 0;
order = get_order_from_bytes(bytes);
-debugtrace_buf = alloc_xenheap_pages(order, 0);
-ASSERT(debugtrace_buf != NULL);
+data = alloc_xenheap_pages(order, 0);
+if ( !data )
+return -ENOMEM;
-memset(debugtrace_buf, '\0', bytes);
+memset(data, '\0', bytes);
-debugtrace_bytes = bytes;
+data->bytes = bytes - sizeof(*data);
+debtr_data = data;
The allocation may end up being almost twice as big as what gets
actually used this way. Wouldn't it make sense to re-calculate
"bytes" from "order"?
Yes, you are right.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 4/4] xen: add per-cpu buffer option to debugtrace
On 03.09.19 12:51, Jan Beulich wrote:
On 28.08.2019 10:00, Juergen Gross wrote:
@@ -24,32 +25,62 @@ struct debugtrace_data_s {
};
static struct debugtrace_data_s *debtr_data;
+static DEFINE_PER_CPU(struct debugtrace_data_s *, debtr_cpu_data);
-static unsigned int debugtrace_kilobytes = 128;
+static unsigned int debugtrace_bytes = 128 << 10;
And after patch 3 this is now left as "unsigned int"?
Good catch. :-)
+static bool debugtrace_per_cpu;
static bool debugtrace_used;
static DEFINE_SPINLOCK(debugtrace_lock);
-integer_param("debugtrace", debugtrace_kilobytes);
-static void debugtrace_dump_worker(void)
+static int __init debugtrace_parse_param(const char *s)
+{
+if ( !strncmp(s, "cpu:", 4) )
+{
+debugtrace_per_cpu = true;
+s += 4;
+}
+debugtrace_bytes = parse_size_and_unit(s, NULL);
Stray double blank.
Yes. Will remove one.
+return 0;
+}
+custom_param("debugtrace", debugtrace_parse_param);
+
+static void debugtrace_dump_buffer(struct debugtrace_data_s *data,
+ const char *which)
{
-if ( !debtr_data || !debugtrace_used )
+if ( !data )
return;
-printk("debugtrace_dump() starting\n");
+printk("debugtrace_dump() %s buffer starting\n", which);
/* Print oldest portion of the ring. */
-ASSERT(debtr_data->buf[debtr_data->bytes - 1] == 0);
-if ( debtr_data->buf[debtr_data->prd] != '\0' )
-console_serial_puts(&debtr_data->buf[debtr_data->prd],
-debtr_data->bytes - debtr_data->prd - 1);
+ASSERT(data->buf[data->bytes - 1] == 0);
+if ( data->buf[data->prd] != '\0' )
+console_serial_puts(&data->buf[data->prd], data->bytes - data->prd -
1);
Seeing this getting changed yet another time I now really wonder if
this nul termination is really still needed now that a size is being
passed into the actual output function. If you got rid of this in an
early prereq patch, the subsequent (to that one) ones would shrink.
Yes.
Furthermore I can't help thinking that said change to pass the size
into the actual output functions actually broke the logic here: By
memset()-ing the buffer to zero, output on a subsequent invocation
would have been suitably truncated (in fact, until prd had wrapped,
I think it would have got truncated more than intended). Julien,
can you please look into this apparent regression?
I can do that. Resetting prd to 0 when clearing the buffer is
required here.
@@ -156,33 +195,70 @@ static void debugtrace_key(unsigned char key)
debugtrace_toggle();
}
-static int __init debugtrace_init(void)
+static void debugtrace_alloc_buffer(struct debugtrace_data_s **ptr)
{
int order;
-unsigned long kbytes, bytes;
struct debugtrace_data_s *data;
-/* Round size down to next power of two. */
-while ( (kbytes = (debugtrace_kilobytes & (debugtrace_kilobytes-1))) != 0 )
-debugtrace_kilobytes = kbytes;
-
-bytes = debugtrace_kilobytes << 10;
-if ( bytes == 0 )
-return 0;
+if ( debugtrace_bytes < PAGE_SIZE || *ptr )
Why the check against PAGE_SIZE?
With recaclulating debugtrace_bytes this can be dropped.
+return;
-order = get_order_from_bytes(bytes);
+order = get_order_from_bytes(debugtrace_bytes);
data = alloc_xenheap_pages(order, 0);
if ( !data )
-return -ENOMEM;
+{
+printk("failed to allocate debugtrace buffer\n");
Perhaps better to also indicate which/whose buffer?
Hmm, I'm not sure this is really required. I can add it if you want, but
as a user of debugtrace I'd be only interested in the information
whether I can expect all trace entries to be seen or not.
+return;
+}
+
+memset(data, '\0', debugtrace_bytes);
+data->bytes = debugtrace_bytes - sizeof(*data);
+
+*ptr = data;
+}
+
+static int debugtrace_cpu_callback(struct notifier_block *nfb,
+ unsigned long action, void *hcpu)
+{
+unsigned int cpu = (unsigned long)hcpu;
+
+/* Buffers are only ever allocated, never freed. */
+switch ( action )
+{
+case CPU_UP_PREPARE:
+debugtrace_alloc_buffer(&per_cpu(debtr_cpu_data, cpu));
+break;
+default:
+break;
There no apparent need for "default:" here; quite possibly this
could be if() instead of switch() anyway.
Fine with me.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] arm: xen: mm: use __GPF_DMA32 for arm64
On 02.09.19 17:57, Christoph Hellwig wrote: On Fri, Aug 30, 2019 at 07:40:42PM -0700, Stefano Stabellini wrote: + Juergen, Boris On Fri, 30 Aug 2019, Christoph Hellwig wrote: Can we take a step back and figure out what we want to do here? AFAICS this function allocates memory for the swiotlb-xen buffer, and that means it must be <= 32-bit addressable to satisfy the DMA API guarantees. That means we generally want to use GFP_DMA32 everywhere that exists, but on systems with odd zones we might want to dip into GFP_DMA. This also means swiotlb-xen doesn't actually do the right thing on x86 at the moment. So shouldn't we just have one common routine in swiotlb-xen.c that checks if we have CONFIG_ZONE_DMA32 set, then try GFP_DMA32, and if not check if CONFIG_ZONE_DMA is set and then try that, else default to GFP_KERNEL? Yes, for ARM/ARM64 it makes a lot of sense given that dom0 is 1:1 mapped (pseudo-physical == physical). I'll let Juergen and Boris comment on the x86 side of things, but on x86 PV Dom0 is not 1:1 mapped so GFP_DMA32 is probably not meaningful. But is it actually harmful? If the GFP_DMA32 doesn't hurt we could just use it there. Or if that seems to ugly we can make the dma flags dependents on a XEN_1TO1_MAPPED config option set by arm/arm64. I'd rather have it only if needed. Especially on X86 PV dom0 I'd like to avoid GFP_DMA32 as memory below 4GB (guest physical) might be rather scarce in some configurations. I think X86 PVH dom0 should need GFP_DMA32, too, as the limit is related to the address as communicated to the device (before being translated by the IOMMU), right? This would mean on a X86 kernel configured to support PV and PVH the test for setting GFP_DMA32 can't depend on a config option alone, it needs to be dynamic. BTW, for PV guests the DMA address width is handled via xen_create_contiguous_region(). Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 1/4] xen: use common output function in debugtrace
On 03.09.19 13:47, Jan Beulich wrote: On 03.09.2019 12:22, Juergen Gross wrote: On 03.09.19 12:00, Jan Beulich wrote: On 28.08.2019 10:00, Juergen Gross wrote: Today dumping the debugtrace buffers is done via sercon_puts(), while direct printing of trace entries (after toggling output to the console) is using serial_puts(). Use sercon_puts() in both cases, as the difference between both is not really making sense. No matter that I like this change, I'm not convinced it's correct: There are two differences between the functions, neither of which I could qualify as "not really making sense": Why is it obvious that it makes no sense for the debugtrace output to bypass one or both of serial_steal_fn() and pv_console_puts()? If you're convinced of this, please provide the "why"-s behind the sentence above. Okay, I'll use: Use sercon_puts() in both cases, to be consistent between dumping the buffer when switching debugtrace to console output and when printing a debugtrace entry directly to console. Well, this is better as an explanation indeed, but it still doesn't make clear whether it wasn't in fact wanted for there to be a difference in where output gets sent. I may believe that bypassing pv_console_puts() wasn't intended, but the steal function bypass had been there in 3.2 already, so may well have been on purpose. There are two users of console_steal(): suspend handling - I believe it is a good idea to not use the serial interface while it is potentially uninitialized. gdb support - Why should that be special? Not treating the output data appropriate to the attached debugger will be worse than encapsulating it in a way the debugger can handle. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 4/4] xen: add per-cpu buffer option to debugtrace
On 03.09.19 14:01, Jan Beulich wrote:
On 03.09.2019 13:10, Juergen Gross wrote:
On 03.09.19 12:51, Jan Beulich wrote:
On 28.08.2019 10:00, Juergen Gross wrote:
+static void debugtrace_dump_buffer(struct debugtrace_data_s *data,
+ const char *which)
{
-if ( !debtr_data || !debugtrace_used )
+if ( !data )
return;
-printk("debugtrace_dump() starting\n");
+printk("debugtrace_dump() %s buffer starting\n", which);
/* Print oldest portion of the ring. */
-ASSERT(debtr_data->buf[debtr_data->bytes - 1] == 0);
-if ( debtr_data->buf[debtr_data->prd] != '\0' )
-console_serial_puts(&debtr_data->buf[debtr_data->prd],
-debtr_data->bytes - debtr_data->prd - 1);
+ASSERT(data->buf[data->bytes - 1] == 0);
+if ( data->buf[data->prd] != '\0' )
+console_serial_puts(&data->buf[data->prd], data->bytes - data->prd -
1);
Seeing this getting changed yet another time I now really wonder if
this nul termination is really still needed now that a size is being
passed into the actual output function. If you got rid of this in an
early prereq patch, the subsequent (to that one) ones would shrink.
Yes.
Furthermore I can't help thinking that said change to pass the size
into the actual output functions actually broke the logic here: By
memset()-ing the buffer to zero, output on a subsequent invocation
would have been suitably truncated (in fact, until prd had wrapped,
I think it would have got truncated more than intended). Julien,
can you please look into this apparent regression?
I can do that. Resetting prd to 0 when clearing the buffer is
required here.
I'm afraid it's not this simple: Doing so will confuse
debugtrace_printk() - consider the function then storing the
previously latched last_prd into debugtrace_prd.
Sure, this has to be handled (and it is still easy to do so).
-order = get_order_from_bytes(bytes);
+order = get_order_from_bytes(debugtrace_bytes);
data = alloc_xenheap_pages(order, 0);
if ( !data )
-return -ENOMEM;
+{
+printk("failed to allocate debugtrace buffer\n");
Perhaps better to also indicate which/whose buffer?
Hmm, I'm not sure this is really required. I can add it if you want, but
as a user of debugtrace I'd be only interested in the information
whether I can expect all trace entries to be seen or not.
Well, if the allocation fails for a CPU, it's not impossible for
the CPU bringup to then also fail. Subsequent to this the system
would then still provide an almost complete set of debugtrace
entries (ones issued by subsequent bringup actions would be
missing), _despite_ this log message.
You seem to want the information, so I can add it.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 1/4] xen: use common output function in debugtrace
On 03.09.19 14:09, Jan Beulich wrote: On 03.09.2019 13:58, Juergen Gross wrote: On 03.09.19 13:47, Jan Beulich wrote: On 03.09.2019 12:22, Juergen Gross wrote: On 03.09.19 12:00, Jan Beulich wrote: On 28.08.2019 10:00, Juergen Gross wrote: Today dumping the debugtrace buffers is done via sercon_puts(), while direct printing of trace entries (after toggling output to the console) is using serial_puts(). Use sercon_puts() in both cases, as the difference between both is not really making sense. No matter that I like this change, I'm not convinced it's correct: There are two differences between the functions, neither of which I could qualify as "not really making sense": Why is it obvious that it makes no sense for the debugtrace output to bypass one or both of serial_steal_fn() and pv_console_puts()? If you're convinced of this, please provide the "why"-s behind the sentence above. Okay, I'll use: Use sercon_puts() in both cases, to be consistent between dumping the buffer when switching debugtrace to console output and when printing a debugtrace entry directly to console. Well, this is better as an explanation indeed, but it still doesn't make clear whether it wasn't in fact wanted for there to be a difference in where output gets sent. I may believe that bypassing pv_console_puts() wasn't intended, but the steal function bypass had been there in 3.2 already, so may well have been on purpose. There are two users of console_steal(): suspend handling - I believe it is a good idea to not use the serial interface while it is potentially uninitialized. I agree here. gdb support - Why should that be special? Not treating the output data appropriate to the attached debugger will be worse than encapsulating it in a way the debugger can handle. I'm not sure here - it may well have been intentional to avoid cluttering the debugger console. But won't using serial_puts() clutter the debugger console? "normal" printk() output seem to be handled in a special way with gdb active, while just using serial_puts() with attached gdb is looking at least suspicious to me. There seems to be a special format, e.g. text output of the hypervisor is prepended with 'O' and the data is sent in hex representation. I can't imagine just sending some ASCII text is the desired approach for debugtrace output. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/4] xen: refactor debugtrace data
On 03.09.19 13:50, Jan Beulich wrote: On 03.09.2019 12:31, Juergen Gross wrote: On 03.09.19 12:16, Jan Beulich wrote: On 28.08.2019 10:00, Juergen Gross wrote: +static unsigned int debugtrace_kilobytes = 128; Since you touch this anyway, add __initdata? Maybe also move it next to its integer_param()? This is modified in patch 4 again, and there I need it in the running system for support of cpu hotplug with per-cpu buffers. Right, I've meanwhile noticed this. Hence it's fine to keep as is. @@ -165,12 +171,14 @@ static int __init debugtrace_init(void) return 0; order = get_order_from_bytes(bytes); -debugtrace_buf = alloc_xenheap_pages(order, 0); -ASSERT(debugtrace_buf != NULL); +data = alloc_xenheap_pages(order, 0); +if ( !data ) +return -ENOMEM; -memset(debugtrace_buf, '\0', bytes); +memset(data, '\0', bytes); -debugtrace_bytes = bytes; +data->bytes = bytes - sizeof(*data); +debtr_data = data; The allocation may end up being almost twice as big as what gets actually used this way. Wouldn't it make sense to re-calculate "bytes" from "order"? Yes, you are right. Actually I wasn't, which I did notice seeing the relevant piece of code getting touched in patch 4: while ( (kbytes = (debugtrace_kilobytes & (debugtrace_kilobytes-1))) != 0 ) debugtrace_kilobytes = kbytes; For kbytes < 4 you still were right. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 1/5] xen/spinlocks: in debug builds store cpu holding the lock
On 03.09.19 16:11, Jan Beulich wrote:
On 29.08.2019 12:18, Juergen Gross wrote:
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -6,14 +6,21 @@
#include
#ifndef NDEBUG
-struct lock_debug {
-s16 irq_safe; /* +1: IRQ-safe; 0: not IRQ-safe; -1: don't know yet */
+union lock_debug {
+uint16_t val;
+#define LOCK_DEBUG_INITVAL 0x
+struct {
+uint16_t cpu:12;
I'm afraid I have one more request: The literal 12 in struct spinlock is
sitting right next to the SPINLOCK_NO_CPU definition, making it pretty
unlikely that both author and reviewer of a change wouldn't notice one
getting changed without adjustment to the other. The literal 12 here,
though, is sufficiently remote to that other place, so there needs to be
a connection established. Best I can think of right away is to have a
#define for the 12, move SPINLOCK_NO_CPU's definition next to it, and
use the new constant in both places.
Can do.
+uint16_t pad:2;
While at it, would you mind making this an unnamed field?
NP. I guess the "2" should then be replaced to depend on the added
#define above.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/5] xen: print lock profile info in panic()
On 03.09.19 16:22, Jan Beulich wrote: On 29.08.2019 12:18, Juergen Gross wrote: V2: - rename CONFIG_LOCK_PROFILE to CONFIG_DEBUG_LOCK_PROFILE (Jan Beulich) - move .lockprofile.data section to init area in linker scripts How can this be correct, when you don't change lock_prof_init() at all? The function builds a linked list from the entries in the section, and then hands the head of this list to _lock_profile_register_struct(). I guess I must be missing something. Anyway, everything else here looks good to me. The .lockprofile.data section contains only the pointers to the elements put into the linked list. And those pointers are no longer needed afterwards. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] xen: modify several static locks to unique names
On 03.09.19 16:53, Jan Beulich wrote:
On 29.08.2019 12:18, Juergen Gross wrote:
In order to have unique names when doing lock profiling several local
locks "lock" need to be renamed.
But these are all named simply "lock" for a good reason, including
because they're all function scope symbols (and typically the
functions are all sufficiently short). The issue stems from the
dual use of "name" in
#define _LOCK_PROFILE(name) { 0, #name, &name, 0, 0, 0, 0, 0 }
so I'd rather suggest making this a derivation of a new
#define _LOCK_PROFILE_NAME(lock, name) { 0, #name, &lock, 0, 0, 0, 0, 0 }
if there's no other (transparent) way of disambiguating the names.
This will require to use a different DEFINE_SPINLOCK() variant, so e.g.
DEFINE_SPINLOCK_LOCAL(), which will then include the needed "static" and
add "@" to the lock profiling name. Is this okay?
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] xen: modify several static locks to unique names
On 03.09.19 17:09, Jan Beulich wrote:
On 03.09.2019 17:03, Juergen Gross wrote:
On 03.09.19 16:53, Jan Beulich wrote:
On 29.08.2019 12:18, Juergen Gross wrote:
In order to have unique names when doing lock profiling several local
locks "lock" need to be renamed.
But these are all named simply "lock" for a good reason, including
because they're all function scope symbols (and typically the
functions are all sufficiently short). The issue stems from the
dual use of "name" in
#define _LOCK_PROFILE(name) { 0, #name, &name, 0, 0, 0, 0, 0 }
so I'd rather suggest making this a derivation of a new
#define _LOCK_PROFILE_NAME(lock, name) { 0, #name, &lock, 0, 0, 0, 0, 0 }
if there's no other (transparent) way of disambiguating the names.
This will require to use a different DEFINE_SPINLOCK() variant, so e.g.
DEFINE_SPINLOCK_LOCAL(), which will then include the needed "static" and
add "@" to the lock profiling name. Is this okay?
To be frank - not really. I dislike both, and would hence prefer to
stick to what there is currently, until someone invents a transparent
way to disambiguate these. I'm sorry for being unhelpful here.
NP with me. It was Andrew who asked for a way to differentiate between
multiple locks. I'm not depending in any way on this patch.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] xen: modify several static locks to unique names
On 03.09.19 17:09, Jan Beulich wrote:
On 03.09.2019 17:03, Juergen Gross wrote:
On 03.09.19 16:53, Jan Beulich wrote:
On 29.08.2019 12:18, Juergen Gross wrote:
In order to have unique names when doing lock profiling several local
locks "lock" need to be renamed.
But these are all named simply "lock" for a good reason, including
because they're all function scope symbols (and typically the
functions are all sufficiently short). The issue stems from the
dual use of "name" in
#define _LOCK_PROFILE(name) { 0, #name, &name, 0, 0, 0, 0, 0 }
so I'd rather suggest making this a derivation of a new
#define _LOCK_PROFILE_NAME(lock, name) { 0, #name, &lock, 0, 0, 0, 0, 0 }
if there's no other (transparent) way of disambiguating the names.
This will require to use a different DEFINE_SPINLOCK() variant, so e.g.
DEFINE_SPINLOCK_LOCAL(), which will then include the needed "static" and
add "@" to the lock profiling name. Is this okay?
To be frank - not really. I dislike both, and would hence prefer to
stick to what there is currently, until someone invents a transparent
way to disambiguate these. I'm sorry for being unhelpful here.
I think I have found a way: I could add __FILE__ and __LINE__ data to
struct lock_profile. In lock_prof_init() I could look for non-unique
lock names and mark those to be printed with the __FILE__ and __LINE__
data added to the names.
Would you be fine with this approach?
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 4/5] xen: modify lock profiling interface
On 03.09.19 16:46, Jan Beulich wrote:
On 29.08.2019 12:18, Juergen Gross wrote:
Today adding locks located in a struct to lock profiling requires a
unique type index for each structure. This makes it hard to add any
new structure as the related sysctl interface needs to be changed, too.
Instead of using an index the already existing struct name specified
in lock_profile_register_struct() can be used as an identifier instead.
Modify the sysctl interface to use the struct name instead of the type
index and adapt the related coding accordingly. Instead of an array of
struct anchors for lock profiling we now use a linked list for that
purpose. Use the special idx value -1 for cases where the idx isn't
relevant (global locks) and shouldn't be printed.
Just to make explicit what was implied by my v1 reply: I can see why
you want to do this, but I'm not really a friend of string literals
in the hypercall interface, when binary values can also do. So to
me this looks to be a move in the wrong direction. Therefore, while
I'm fine reviewing such a change, I'm not very likely to eventually
ack it.
I'll write two example patches for adding support of lock profiling in a
new structure, one with patch 4 of this series applied and one for the
interface without that patch. This should make clear why I'm really in
favor of this patch.
@@ -465,31 +466,70 @@ int spinlock_profile_control(struct
xen_sysctl_lockprof_op *pc)
return rc;
}
-void _lock_profile_register_struct(
-int32_t type, struct lock_profile_qhead *qhead, int32_t idx, char *name)
+static struct lock_profile_anc *find_prof_anc(const char *name)
{
-qhead->idx = idx;
+struct lock_profile_anc *anc;
+
+for ( anc = lock_profile_ancs; anc; anc = anc->next )
+if ( !strcmp(anc->name, name) )
+return anc;
+return NULL;
+}
Blank line before main return statement please.
Yes.
+void _lock_profile_register_struct(struct lock_profile_qhead *qhead,
+ int32_t idx, const char *name)
+{
+struct lock_profile_anc *anc;
+
spin_lock(&lock_profile_lock);
-qhead->head_q = lock_profile_ancs[type].head_q;
-lock_profile_ancs[type].head_q = qhead;
-lock_profile_ancs[type].name = name;
+
+anc = find_prof_anc(name);
+if ( !anc )
+{
+anc = xzalloc(struct lock_profile_anc);
Hmm, another allocation with a lock held. We try to avoid such as
much as possible, and it doesn't look overly difficult to avoid it
here.
I'll modify it.
+if ( !anc )
+goto out;
+anc->name = name;
+anc->next = lock_profile_ancs;
+lock_profile_ancs = anc;
+}
+
+qhead->idx = idx;
+qhead->head_q = anc->head_q;
+anc->head_q = qhead;
+
+ out:
spin_unlock(&lock_profile_lock);
}
-void _lock_profile_deregister_struct(
-int32_t type, struct lock_profile_qhead *qhead)
+void _lock_profile_deregister_struct(struct lock_profile_qhead *qhead,
+ const char *name)
{
+struct lock_profile_anc *anc;
struct lock_profile_qhead **q;
+struct lock_profile *elem;
spin_lock(&lock_profile_lock);
-for ( q = &lock_profile_ancs[type].head_q; *q; q = &(*q)->head_q )
+
+anc = find_prof_anc(name);
+if ( anc )
{
-if ( *q == qhead )
+for ( q = &anc->head_q; *q; q = &(*q)->head_q )
{
-*q = qhead->head_q;
-break;
+if ( *q == qhead )
+{
+*q = qhead->head_q;
+while ( qhead->elem_q )
+{
+elem = qhead->elem_q;
+qhead->elem_q = elem->next;
+xfree(elem);
Which assumes the global list would never get handed here. Probably
fine.
I can add an ASSERT() to make this very clear.
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -50,19 +50,24 @@ union lock_debug { };
with ptr being the main structure pointer and lock the spinlock field
+ It should be noted that this will need to allocate memory, so interrupts
+ must be enabled.
+
- each structure has to be added to profiling with
- lock_profile_register_struct(type, ptr, idx, print);
+ lock_profile_register_struct(ptr, idx, print);
with:
-type: something like LOCKPROF_TYPE_PERDOM
ptr: pointer to the structure
idx: index of that structure, e.g. domid
print: descriptive string like "domain"
+ It should be noted that this will might need to allocate memory, so
Nit: "will" or "might", but not both.
Indeed.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] xen: modify several static locks to unique names
On 04.09.19 10:40, Jan Beulich wrote:
On 04.09.2019 10:25, Juergen Gross wrote:
On 03.09.19 17:09, Jan Beulich wrote:
On 03.09.2019 17:03, Juergen Gross wrote:
On 03.09.19 16:53, Jan Beulich wrote:
On 29.08.2019 12:18, Juergen Gross wrote:
In order to have unique names when doing lock profiling several local
locks "lock" need to be renamed.
But these are all named simply "lock" for a good reason, including
because they're all function scope symbols (and typically the
functions are all sufficiently short). The issue stems from the
dual use of "name" in
#define _LOCK_PROFILE(name) { 0, #name, &name, 0, 0, 0, 0, 0 }
so I'd rather suggest making this a derivation of a new
#define _LOCK_PROFILE_NAME(lock, name) { 0, #name, &lock, 0, 0, 0, 0, 0 }
if there's no other (transparent) way of disambiguating the names.
This will require to use a different DEFINE_SPINLOCK() variant, so e.g.
DEFINE_SPINLOCK_LOCAL(), which will then include the needed "static" and
add "@" to the lock profiling name. Is this okay?
To be frank - not really. I dislike both, and would hence prefer to
stick to what there is currently, until someone invents a transparent
way to disambiguate these. I'm sorry for being unhelpful here.
I think I have found a way: I could add __FILE__ and __LINE__ data to
struct lock_profile. In lock_prof_init() I could look for non-unique
lock names and mark those to be printed with the __FILE__ and __LINE__
data added to the names.
Would you be fine with this approach?
I would be, but I'm afraid Andrew won't (as with any new uses of __LINE__).
I wonder if __func__ or __FUNCTION__ could be used - the main question is
how they behave when used outside of a function. If either would be NULL
(rather than triggering a diagnostic), it might be usable here. Of course
this would be fragile if just based on observed (rather than documented)
behavior.
With gcc 7.4.1 it fails:
/home/gross/xen/xen/include/xen/spinlock.h:80:41: error: ‘__func__’ is
not defined outside of function scope [-Werror]
#define _LOCK_PROFILE(name) { 0, #name, __func__, &name, 0, 0, 0, 0, 0 }
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] xen: modify several static locks to unique names
On 04.09.19 10:58, Andrew Cooper wrote:
On 04/09/2019 09:40, Jan Beulich wrote:
On 04.09.2019 10:25, Juergen Gross wrote:
On 03.09.19 17:09, Jan Beulich wrote:
On 03.09.2019 17:03, Juergen Gross wrote:
On 03.09.19 16:53, Jan Beulich wrote:
On 29.08.2019 12:18, Juergen Gross wrote:
In order to have unique names when doing lock profiling several local
locks "lock" need to be renamed.
But these are all named simply "lock" for a good reason, including
because they're all function scope symbols (and typically the
functions are all sufficiently short). The issue stems from the
dual use of "name" in
#define _LOCK_PROFILE(name) { 0, #name, &name, 0, 0, 0, 0, 0 }
so I'd rather suggest making this a derivation of a new
#define _LOCK_PROFILE_NAME(lock, name) { 0, #name, &lock, 0, 0, 0, 0, 0 }
if there's no other (transparent) way of disambiguating the names.
This will require to use a different DEFINE_SPINLOCK() variant, so e.g.
DEFINE_SPINLOCK_LOCAL(), which will then include the needed "static" and
add "@" to the lock profiling name. Is this okay?
To be frank - not really. I dislike both, and would hence prefer to
stick to what there is currently, until someone invents a transparent
way to disambiguate these. I'm sorry for being unhelpful here.
I think I have found a way: I could add __FILE__ and __LINE__ data to
struct lock_profile. In lock_prof_init() I could look for non-unique
lock names and mark those to be printed with the __FILE__ and __LINE__
data added to the names.
Would you be fine with this approach?
I would be, but I'm afraid Andrew won't (as with any new uses of __LINE__).
The ok-ness of using __LINE__ is inversely proportional to the
likelihood of developing a livepatch for this particular build of Xen,
and what additional patching delta it would cause through unrelated changes.
Not related to this patch, but to __LINE__ and livepatching: have you
considered to use the "#line" directive to avoid unrelated diffs?
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] xen: modify several static locks to unique names
On 04.09.19 11:15, Andrew Cooper wrote:
On 04/09/2019 10:11, Juergen Gross wrote:
On 04.09.19 10:58, Andrew Cooper wrote:
On 04/09/2019 09:40, Jan Beulich wrote:
On 04.09.2019 10:25, Juergen Gross wrote:
On 03.09.19 17:09, Jan Beulich wrote:
On 03.09.2019 17:03, Juergen Gross wrote:
On 03.09.19 16:53, Jan Beulich wrote:
On 29.08.2019 12:18, Juergen Gross wrote:
In order to have unique names when doing lock profiling several
local
locks "lock" need to be renamed.
But these are all named simply "lock" for a good reason, including
because they're all function scope symbols (and typically the
functions are all sufficiently short). The issue stems from the
dual use of "name" in
#define _LOCK_PROFILE(name) { 0, #name, &name, 0, 0, 0, 0, 0 }
so I'd rather suggest making this a derivation of a new
#define _LOCK_PROFILE_NAME(lock, name) { 0, #name, &lock, 0, 0,
0, 0, 0 }
if there's no other (transparent) way of disambiguating the names.
This will require to use a different DEFINE_SPINLOCK() variant,
so e.g.
DEFINE_SPINLOCK_LOCAL(), which will then include the needed
"static" and
add "@" to the lock profiling name. Is this okay?
To be frank - not really. I dislike both, and would hence prefer to
stick to what there is currently, until someone invents a transparent
way to disambiguate these. I'm sorry for being unhelpful here.
I think I have found a way: I could add __FILE__ and __LINE__ data to
struct lock_profile. In lock_prof_init() I could look for non-unique
lock names and mark those to be printed with the __FILE__ and __LINE__
data added to the names.
Would you be fine with this approach?
I would be, but I'm afraid Andrew won't (as with any new uses of
__LINE__).
The ok-ness of using __LINE__ is inversely proportional to the
likelihood of developing a livepatch for this particular build of Xen,
and what additional patching delta it would cause through unrelated
changes.
Not related to this patch, but to __LINE__ and livepatching: have you
considered to use the "#line" directive to avoid unrelated diffs?
There are ways to play with __LINE__, yes. #line was brought up in the
original discussion.
As a thought experiment, how would you expect this to be used to
simplify a livepatch?
It should be rather strait forward to write a tool adding #line
directives to a patch recovering the previous line numbers in the
code following a modification which added or removed lines.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] xen: modify several static locks to unique names
On 04.09.19 10:51, Jan Beulich wrote:
On 04.09.2019 10:47, Juergen Gross wrote:
On 04.09.19 10:40, Jan Beulich wrote:
On 04.09.2019 10:25, Juergen Gross wrote:
On 03.09.19 17:09, Jan Beulich wrote:
On 03.09.2019 17:03, Juergen Gross wrote:
On 03.09.19 16:53, Jan Beulich wrote:
On 29.08.2019 12:18, Juergen Gross wrote:
In order to have unique names when doing lock profiling several local
locks "lock" need to be renamed.
But these are all named simply "lock" for a good reason, including
because they're all function scope symbols (and typically the
functions are all sufficiently short). The issue stems from the
dual use of "name" in
#define _LOCK_PROFILE(name) { 0, #name, &name, 0, 0, 0, 0, 0 }
so I'd rather suggest making this a derivation of a new
#define _LOCK_PROFILE_NAME(lock, name) { 0, #name, &lock, 0, 0, 0, 0, 0 }
if there's no other (transparent) way of disambiguating the names.
This will require to use a different DEFINE_SPINLOCK() variant, so e.g.
DEFINE_SPINLOCK_LOCAL(), which will then include the needed "static" and
add "@" to the lock profiling name. Is this okay?
To be frank - not really. I dislike both, and would hence prefer to
stick to what there is currently, until someone invents a transparent
way to disambiguate these. I'm sorry for being unhelpful here.
I think I have found a way: I could add __FILE__ and __LINE__ data to
struct lock_profile. In lock_prof_init() I could look for non-unique
lock names and mark those to be printed with the __FILE__ and __LINE__
data added to the names.
Would you be fine with this approach?
I would be, but I'm afraid Andrew won't (as with any new uses of __LINE__).
I wonder if __func__ or __FUNCTION__ could be used - the main question is
how they behave when used outside of a function. If either would be NULL
(rather than triggering a diagnostic), it might be usable here. Of course
this would be fragile if just based on observed (rather than documented)
behavior.
With gcc 7.4.1 it fails:
/home/gross/xen/xen/include/xen/spinlock.h:80:41: error: ‘__func__’ is
not defined outside of function scope [-Werror]
#define _LOCK_PROFILE(name) { 0, #name, __func__, &name, 0, 0, 0, 0, 0 }
Right, as I was afraid of. But __PRETTY_FUNCTION__ looks to be suitable
(as per the gcc doc, and as per there being clear indications that clang
also supports this).
Yes, this is working. Will modify the patch.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 0/4] xen: debugtrace cleanup and per-cpu buffer support
Another debugtrace enhancement I needed for core scheduling debugging, plus some cleanup. Changes in V4: - replaced patch 1 (original one was committed, new one requested by Jan Beulich) - several comments by Jan addressed Changes in V3: - rebase to current staging Changes in V2: - added new patch 1 (preparing the move of debugtrace coding) - patch 4 (v1 patch 3): avoid leaking buffer Juergen Gross (4): xen: fix debugtrace clearing xen: move debugtrace coding to common/debugtrace.c xen: refactor debugtrace data xen: add per-cpu buffer option to debugtrace docs/misc/xen-command-line.pandoc | 7 +- xen/common/Makefile | 1 + xen/common/debugtrace.c | 264 ++ xen/drivers/char/console.c| 178 + 4 files changed, 270 insertions(+), 180 deletions(-) create mode 100644 xen/common/debugtrace.c -- 2.16.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 4/4] xen: add per-cpu buffer option to debugtrace
debugtrace is normally writing trace entries into a single trace
buffer. There are cases where this is not optimal, e.g. when hunting
a bug which requires writing lots of trace entries and one cpu is
stuck. This will result in other cpus filling the trace buffer and
finally overwriting the interesting trace entries of the hanging cpu.
In order to be able to debug such situations add the capability to use
per-cpu trace buffers. This can be selected by specifying the
debugtrace boot parameter with the modifier "cpu:", like:
debugtrace=cpu:16
At the same time switch the parsing function to accept size modifiers
(e.g. 4M or 1G).
Printing out the trace entries is done for each buffer in order to
minimize the effort needed during printing. As each entry is prefixed
with its sequence number sorting the entries can easily be done when
analyzing them.
Signed-off-by: Juergen Gross
---
V2:
- only allocate buffer if not already done so
V4:
- unsigned int -> unsigned long (Jan Beulich)
- replace check for bytes < PAGE_SIZE by !bytes (Jan Beulich)
- print info which buffer allocation failed (Jan Beulich)
- replace switch by if in cpu notifier handler (Jan Beulich)
---
docs/misc/xen-command-line.pandoc | 7 +-
xen/common/debugtrace.c | 149 --
2 files changed, 116 insertions(+), 40 deletions(-)
diff --git a/docs/misc/xen-command-line.pandoc
b/docs/misc/xen-command-line.pandoc
index 7c72e31032..832797e2e2 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -644,12 +644,13 @@ over the PCI busses sequentially) or by PCI device (must
be on segment 0).
Limits the number lines printed in Xen stack traces.
### debugtrace
-> `= `
+> `= [cpu:]`
> Default: `128`
-Specify the size of the console debug trace buffer in KiB. The debug
-trace feature is only enabled in debugging builds of Xen.
+Specify the size of the console debug trace buffer. By specifying `cpu:`
+additionally a trace buffer of the specified size is allocated per cpu.
+The debug trace feature is only enabled in debugging builds of Xen.
### dma_bits
> `= `
diff --git a/xen/common/debugtrace.c b/xen/common/debugtrace.c
index 8fa0e7fb0e..449406b9b6 100644
--- a/xen/common/debugtrace.c
+++ b/xen/common/debugtrace.c
@@ -6,6 +6,7 @@
#include
+#include
#include
#include
#include
@@ -25,33 +26,63 @@ struct debugtrace_data {
};
static struct debugtrace_data *dt_data;
+static DEFINE_PER_CPU(struct debugtrace_data *, dt_cpu_data);
-static unsigned int debugtrace_kilobytes = 128;
+static unsigned long debugtrace_bytes = 128 << 10;
+static bool debugtrace_per_cpu;
static bool debugtrace_used;
static DEFINE_SPINLOCK(debugtrace_lock);
-integer_param("debugtrace", debugtrace_kilobytes);
-static void debugtrace_dump_worker(void)
+static int __init debugtrace_parse_param(const char *s)
{
-if ( !debugtrace_used )
+if ( !strncmp(s, "cpu:", 4) )
+{
+debugtrace_per_cpu = true;
+s += 4;
+}
+debugtrace_bytes = parse_size_and_unit(s, NULL);
+return 0;
+}
+custom_param("debugtrace", debugtrace_parse_param);
+
+static void debugtrace_dump_buffer(struct debugtrace_data *data,
+ const char *which)
+{
+if ( !data )
return;
-printk("debugtrace_dump() starting\n");
+printk("debugtrace_dump() %s buffer starting\n", which);
/* Print oldest portion of the ring. */
-if ( dt_data->buf[dt_data->prd] != '\0' )
-console_serial_puts(&dt_data->buf[dt_data->prd],
-dt_data->bytes - dt_data->prd);
+if ( data->buf[data->prd] != '\0' )
+console_serial_puts(&data->buf[data->prd], data->bytes - data->prd);
/* Print youngest portion of the ring. */
-dt_data->buf[dt_data->prd] = '\0';
-console_serial_puts(&dt_data->buf[0], dt_data->prd);
+data->buf[data->prd] = '\0';
+console_serial_puts(&data->buf[0], data->prd);
-memset(dt_data->buf, '\0', dt_data->bytes);
-dt_data->prd = 0;
-dt_data->prd_last = 0;
+memset(data->buf, '\0', data->bytes);
+data->prd = 0;
+data->prd_last = 0;
-printk("debugtrace_dump() finished\n");
+printk("debugtrace_dump() %s buffer finished\n", which);
+}
+
+static void debugtrace_dump_worker(void)
+{
+unsigned int cpu;
+char buf[16];
+
+if ( !debugtrace_used )
+return;
+
+debugtrace_dump_buffer(dt_data, "global");
+
+for ( cpu = 0; cpu < nr_cpu_ids; cpu++ )
+{
+snprintf(buf, sizeof(buf), "cpu %u", cpu);
+debugtrace_dump_buffer(per_cpu(dt_cpu_data, cpu), buf);
+}
}
static void debugtrace_toggle(void)
@@ -91,27
[Xen-devel] [PATCH v4 3/4] xen: refactor debugtrace data
As a preparation for per-cpu buffers do a little refactoring of the
debugtrace data: put the needed buffer admin data into the buffer as
it will be needed for each buffer.
While at it switch debugtrace_send_to_console and debugtrace_used to
bool and delete an empty line.
Signed-off-by: Juergen Gross
---
V4:
- renamed struct debugtrace_data_s (Jan Beulich)
- renamed debtr_data (Jan Beulich)
- remove unneeded condition (Jan Beulich)
- recalc debugtrace_bytes (Jan Beulich)
---
xen/common/debugtrace.c | 65 -
1 file changed, 37 insertions(+), 28 deletions(-)
diff --git a/xen/common/debugtrace.c b/xen/common/debugtrace.c
index 506aee9418..8fa0e7fb0e 100644
--- a/xen/common/debugtrace.c
+++ b/xen/common/debugtrace.c
@@ -15,35 +15,41 @@
#include
/* Send output direct to console, or buffer it? */
-static volatile int debugtrace_send_to_console;
+static volatile bool debugtrace_send_to_console;
-static char*debugtrace_buf; /* Debug-trace buffer */
-static unsigned int debugtrace_prd; /* Producer index */
-static unsigned int debugtrace_prd_last;
-static unsigned int debugtrace_kilobytes = 128, debugtrace_bytes;
-static unsigned int debugtrace_used;
+struct debugtrace_data {
+unsigned long bytes; /* Size of buffer. */
+unsigned long prd; /* Producer index. */
+unsigned long prd_last;
+char buf[];
+};
+
+static struct debugtrace_data *dt_data;
+
+static unsigned int debugtrace_kilobytes = 128;
+static bool debugtrace_used;
static DEFINE_SPINLOCK(debugtrace_lock);
integer_param("debugtrace", debugtrace_kilobytes);
static void debugtrace_dump_worker(void)
{
-if ( (debugtrace_bytes == 0) || !debugtrace_used )
+if ( !debugtrace_used )
return;
printk("debugtrace_dump() starting\n");
/* Print oldest portion of the ring. */
-if ( debugtrace_buf[debugtrace_prd] != '\0' )
-console_serial_puts(&debugtrace_buf[debugtrace_prd],
-debugtrace_bytes - debugtrace_prd);
+if ( dt_data->buf[dt_data->prd] != '\0' )
+console_serial_puts(&dt_data->buf[dt_data->prd],
+dt_data->bytes - dt_data->prd);
/* Print youngest portion of the ring. */
-debugtrace_buf[debugtrace_prd] = '\0';
-console_serial_puts(&debugtrace_buf[0], debugtrace_prd);
+dt_data->buf[dt_data->prd] = '\0';
+console_serial_puts(&dt_data->buf[0], dt_data->prd);
-memset(debugtrace_buf, '\0', debugtrace_bytes);
-debugtrace_prd = 0;
-debugtrace_prd_last = 0;
+memset(dt_data->buf, '\0', dt_data->bytes);
+dt_data->prd = 0;
+dt_data->prd_last = 0;
printk("debugtrace_dump() finished\n");
}
@@ -68,7 +74,6 @@ static void debugtrace_toggle(void)
spin_unlock_irqrestore(&debugtrace_lock, flags);
watchdog_enable();
-
}
void debugtrace_dump(void)
@@ -90,9 +95,9 @@ static void debugtrace_add_to_buf(char *buf)
for ( p = buf; *p != '\0'; p++ )
{
-debugtrace_buf[debugtrace_prd++] = *p;
-if ( debugtrace_prd == debugtrace_bytes )
-debugtrace_prd = 0;
+dt_data->buf[dt_data->prd++] = *p;
+if ( dt_data->prd == dt_data->bytes )
+dt_data->prd = 0;
}
}
@@ -106,10 +111,10 @@ void debugtrace_printk(const char *fmt, ...)
unsigned long flags;
unsigned int nr;
-if ( debugtrace_bytes == 0 )
+if ( !dt_data )
return;
-debugtrace_used = 1;
+debugtrace_used = true;
spin_lock_irqsave(&debugtrace_lock, flags);
@@ -128,14 +133,14 @@ void debugtrace_printk(const char *fmt, ...)
{
if ( strcmp(buf, last_buf) )
{
-debugtrace_prd_last = debugtrace_prd;
+dt_data->prd_last = dt_data->prd;
last_count = ++count;
safe_strcpy(last_buf, buf);
snprintf(cntbuf, sizeof(cntbuf), "%u ", count);
}
else
{
-debugtrace_prd = debugtrace_prd_last;
+dt_data->prd = dt_data->prd_last;
snprintf(cntbuf, sizeof(cntbuf), "%u-%u ", last_count, ++count);
}
debugtrace_add_to_buf(cntbuf);
@@ -153,7 +158,8 @@ static void debugtrace_key(unsigned char key)
static int __init debugtrace_init(void)
{
int order;
-unsigned int kbytes, bytes;
+unsigned long kbytes, bytes;
+struct debugtrace_data *data;
/* Round size down to next power of two. */
while ( (kbytes = (debugtrace_kilobytes & (debugtrace_kilobytes-1))) != 0 )
@@ -164,12 +170,15 @@ static int __init debugtrace_init(void)
return 0;
order = get_order_from_bytes(bytes);
-debugtrace_buf = alloc_xenheap_pages(order, 0);
-ASSERT(debugtrace_buf != NULL);
+data
[Xen-devel] [PATCH v4 1/4] xen: fix debugtrace clearing
After dumping the debugtrace buffer it is cleared. This results in some
entries not being printed in case the buffer is dumped again before
having wrapped.
While at it remove the trailing zero byte in the buffer as it is no
longer needed. Commit b5e6e1ee8da59f introduced passing the number of
chars to be printed in the related interfaces, so the trailing 0 byte
is no longer required.
Signed-off-by: Juergen Gross
---
xen/drivers/char/console.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index f49c6f29a8..62477a9728 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1171,6 +1171,7 @@ static volatile int debugtrace_send_to_console;
static char*debugtrace_buf; /* Debug-trace buffer */
static unsigned int debugtrace_prd; /* Producer index */
+static unsigned int debugtrace_prd_last;
static unsigned int debugtrace_kilobytes = 128, debugtrace_bytes;
static unsigned int debugtrace_used;
static DEFINE_SPINLOCK(debugtrace_lock);
@@ -1184,16 +1185,17 @@ static void debugtrace_dump_worker(void)
printk("debugtrace_dump() starting\n");
/* Print oldest portion of the ring. */
-ASSERT(debugtrace_buf[debugtrace_bytes - 1] == 0);
if ( debugtrace_buf[debugtrace_prd] != '\0' )
console_serial_puts(&debugtrace_buf[debugtrace_prd],
-debugtrace_bytes - debugtrace_prd - 1);
+debugtrace_bytes - debugtrace_prd);
/* Print youngest portion of the ring. */
debugtrace_buf[debugtrace_prd] = '\0';
console_serial_puts(&debugtrace_buf[0], debugtrace_prd);
memset(debugtrace_buf, '\0', debugtrace_bytes);
+debugtrace_prd = 0;
+debugtrace_prd_last = 0;
printk("debugtrace_dump() finished\n");
}
@@ -1241,8 +1243,7 @@ static void debugtrace_add_to_buf(char *buf)
for ( p = buf; *p != '\0'; p++ )
{
debugtrace_buf[debugtrace_prd++] = *p;
-/* Always leave a nul byte at the end of the buffer. */
-if ( debugtrace_prd == (debugtrace_bytes - 1) )
+if ( debugtrace_prd == debugtrace_bytes )
debugtrace_prd = 0;
}
}
@@ -1250,7 +1251,7 @@ static void debugtrace_add_to_buf(char *buf)
void debugtrace_printk(const char *fmt, ...)
{
static char buf[1024], last_buf[1024];
-static unsigned int count, last_count, last_prd;
+static unsigned int count, last_count;
char cntbuf[24];
va_list args;
@@ -1264,8 +1265,6 @@ void debugtrace_printk(const char *fmt, ...)
spin_lock_irqsave(&debugtrace_lock, flags);
-ASSERT(debugtrace_buf[debugtrace_bytes - 1] == 0);
-
va_start(args, fmt);
nr = vscnprintf(buf, sizeof(buf), fmt, args);
va_end(args);
@@ -1281,14 +1280,14 @@ void debugtrace_printk(const char *fmt, ...)
{
if ( strcmp(buf, last_buf) )
{
-last_prd = debugtrace_prd;
+debugtrace_prd_last = debugtrace_prd;
last_count = ++count;
safe_strcpy(last_buf, buf);
snprintf(cntbuf, sizeof(cntbuf), "%u ", count);
}
else
{
-debugtrace_prd = last_prd;
+debugtrace_prd = debugtrace_prd_last;
snprintf(cntbuf, sizeof(cntbuf), "%u-%u ", last_count, ++count);
}
debugtrace_add_to_buf(cntbuf);
--
2.16.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 2/4] xen: move debugtrace coding to common/debugtrace.c
Instead of living in drivers/char/console.c move the debugtrace
related coding to a new file common/debugtrace.c
No functional change, code movement only.
Signed-off-by: Juergen Gross
Acked-by: Jan Beulich
---
xen/common/Makefile| 1 +
xen/common/debugtrace.c| 180 +
xen/drivers/char/console.c | 177 +---
3 files changed, 182 insertions(+), 176 deletions(-)
create mode 100644 xen/common/debugtrace.c
diff --git a/xen/common/Makefile b/xen/common/Makefile
index eddda5daa6..62b34e69e9 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -4,6 +4,7 @@ obj-y += bsearch.o
obj-$(CONFIG_CORE_PARKING) += core_parking.o
obj-y += cpu.o
obj-y += cpupool.o
+obj-$(CONFIG_DEBUG_TRACE) += debugtrace.o
obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
obj-y += domctl.o
obj-y += domain.o
diff --git a/xen/common/debugtrace.c b/xen/common/debugtrace.c
new file mode 100644
index 00..506aee9418
--- /dev/null
+++ b/xen/common/debugtrace.c
@@ -0,0 +1,180 @@
+/**
+ * debugtrace.c
+ *
+ * Debugtrace for Xen
+ */
+
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* Send output direct to console, or buffer it? */
+static volatile int debugtrace_send_to_console;
+
+static char*debugtrace_buf; /* Debug-trace buffer */
+static unsigned int debugtrace_prd; /* Producer index */
+static unsigned int debugtrace_prd_last;
+static unsigned int debugtrace_kilobytes = 128, debugtrace_bytes;
+static unsigned int debugtrace_used;
+static DEFINE_SPINLOCK(debugtrace_lock);
+integer_param("debugtrace", debugtrace_kilobytes);
+
+static void debugtrace_dump_worker(void)
+{
+if ( (debugtrace_bytes == 0) || !debugtrace_used )
+return;
+
+printk("debugtrace_dump() starting\n");
+
+/* Print oldest portion of the ring. */
+if ( debugtrace_buf[debugtrace_prd] != '\0' )
+console_serial_puts(&debugtrace_buf[debugtrace_prd],
+debugtrace_bytes - debugtrace_prd);
+
+/* Print youngest portion of the ring. */
+debugtrace_buf[debugtrace_prd] = '\0';
+console_serial_puts(&debugtrace_buf[0], debugtrace_prd);
+
+memset(debugtrace_buf, '\0', debugtrace_bytes);
+debugtrace_prd = 0;
+debugtrace_prd_last = 0;
+
+printk("debugtrace_dump() finished\n");
+}
+
+static void debugtrace_toggle(void)
+{
+unsigned long flags;
+
+watchdog_disable();
+spin_lock_irqsave(&debugtrace_lock, flags);
+
+/*
+ * Dump the buffer *before* toggling, in case the act of dumping the
+ * buffer itself causes more printk() invocations.
+ */
+printk("debugtrace_printk now writing to %s.\n",
+ !debugtrace_send_to_console ? "console": "buffer");
+if ( !debugtrace_send_to_console )
+debugtrace_dump_worker();
+
+debugtrace_send_to_console = !debugtrace_send_to_console;
+
+spin_unlock_irqrestore(&debugtrace_lock, flags);
+watchdog_enable();
+
+}
+
+void debugtrace_dump(void)
+{
+unsigned long flags;
+
+watchdog_disable();
+spin_lock_irqsave(&debugtrace_lock, flags);
+
+debugtrace_dump_worker();
+
+spin_unlock_irqrestore(&debugtrace_lock, flags);
+watchdog_enable();
+}
+
+static void debugtrace_add_to_buf(char *buf)
+{
+char *p;
+
+for ( p = buf; *p != '\0'; p++ )
+{
+debugtrace_buf[debugtrace_prd++] = *p;
+if ( debugtrace_prd == debugtrace_bytes )
+debugtrace_prd = 0;
+}
+}
+
+void debugtrace_printk(const char *fmt, ...)
+{
+static char buf[1024], last_buf[1024];
+static unsigned int count, last_count;
+
+char cntbuf[24];
+va_list args;
+unsigned long flags;
+unsigned int nr;
+
+if ( debugtrace_bytes == 0 )
+return;
+
+debugtrace_used = 1;
+
+spin_lock_irqsave(&debugtrace_lock, flags);
+
+va_start(args, fmt);
+nr = vsnprintf(buf, sizeof(buf), fmt, args);
+va_end(args);
+
+if ( debugtrace_send_to_console )
+{
+unsigned int n = scnprintf(cntbuf, sizeof(cntbuf), "%u ", ++count);
+
+console_serial_puts(cntbuf, n);
+console_serial_puts(buf, nr);
+}
+else
+{
+if ( strcmp(buf, last_buf) )
+{
+debugtrace_prd_last = debugtrace_prd;
+last_count = ++count;
+safe_strcpy(last_buf, buf);
+snprintf(cntbuf, sizeof(cntbuf), "%u ", count);
+}
+else
+{
+debugtrace_prd = debugtrace_prd_last;
+snprintf(cntbuf, sizeof(cntbuf), "%u-%u ", last_count, ++count);
+}
+debugtrace_add_to_buf(cntbuf);
+debugtrace_add_to_buf(buf);
+}
+
+spin_unlock_irqr
