Re: [PATCH] x86/pv: Flush TLB in response to paging structure changes
On 21.10.2020 17:39, Andrew Cooper wrote:
> On 21/10/2020 14:56, Jan Beulich wrote:
>> On 21.10.2020 15:07, Andrew Cooper wrote:
>>> @@ -4037,6 +4037,9 @@ long do_mmu_update(
>>> break;
>>> rc = mod_l2_entry(va, l2e_from_intpte(req.val), mfn,
>>>cmd == MMU_PT_UPDATE_PRESERVE_AD, v);
>>> +/* Paging structure maybe changed. Flush linear
>>> range. */
>>> +if ( !rc )
>>> +flush_flags_all |= FLUSH_TLB;
>>> break;
>>>
>>> case PGT_l3_page_table:
>>> @@ -4044,6 +4047,9 @@ long do_mmu_update(
>>> break;
>>> rc = mod_l3_entry(va, l3e_from_intpte(req.val), mfn,
>>>cmd == MMU_PT_UPDATE_PRESERVE_AD, v);
>>> +/* Paging structure maybe changed. Flush linear
>>> range. */
>>> +if ( !rc )
>>> +flush_flags_all |= FLUSH_TLB;
>>> break;
>>>
>>> case PGT_l4_page_table:
>>> @@ -4051,27 +4057,28 @@ long do_mmu_update(
>>> break;
>>> rc = mod_l4_entry(va, l4e_from_intpte(req.val), mfn,
>>>cmd == MMU_PT_UPDATE_PRESERVE_AD, v);
>>> -if ( !rc && pt_owner->arch.pv.xpti )
>>> +/* Paging structure maybe changed. Flush linear
>>> range. */
>>> +if ( !rc )
>>> {
>>> -bool local_in_use = false;
>>> +bool local_in_use = mfn_eq(
>>> +pagetable_get_mfn(curr->arch.guest_table),
>>> mfn);
>>>
>>> -if (
>>> mfn_eq(pagetable_get_mfn(curr->arch.guest_table),
>>> -mfn) )
>>> -{
>>> -local_in_use = true;
>>> -get_cpu_info()->root_pgt_changed = true;
>>> -}
>>> +flush_flags_all |= FLUSH_TLB;
>>> +
>>> +if ( local_in_use )
>>> +flush_flags_local |= FLUSH_TLB |
>>> FLUSH_ROOT_PGTBL;
>> Aiui here (and in the code consuming the flags) you build upon
>> flush_flags_local, when not zero, always being a superset of
>> flush_flags_all. I think this is a trap to fall into when later
>> wanting to change this code, but as per below this won't hold
>> anymore anyway, I think. Hence here I think you want to set
>> FLUSH_TLB unconditionally, and above for L3 and L2 you want to
>> set it in both variables. Or, if I'm wrong below, a comment to
>> that effect may help people avoid falling into this trap.
>>
>> An alternative would be to have
>>
>> flush_flags_local |= (flush_flags_all & FLUSH_TLB);
>>
>> before doing the actual flush.
>
> Honestly, this is what I meant by stating that the asymmetry is a total
> mess.
>
> I originally named all 'remote', but this is even less accurate, it may
> still contain the current cpu.
>
> Our matrix of complexity:
>
> * FLUSH_TLB for L2+ structure changes
> * FLUSH_TLB_GLOBAL/FLUSH_ROOT_PGTBL for XPTI
>
> with optimisations to skip GLOBAL/ROOT_PGTBL on the local CPU if none of
> the updates hit the L4-in-use, and to skip the remote if we hold all
> references on the L4.
>
> Everything is complicated because pt_owner may not be current, for
> toolstack operations constructing a new domain.
Which is a case I'm wondering why we flush in the first place.
The L4 under construction can't be in use yet, and hence
updates to it shouldn't need syncing. Otoh
pt_owner->dirty_cpumask obviously is empty at that point, i.e.
special casing this likely isn't really worth it.
>>> @@ -4173,18 +4180,36 @@ long do_mmu_update(
>>> if ( va )
>>> unmap_domain_page(va);
>>>
>>> -if ( sync_guest )
>>> +/*
>>> + * Flushing needs to occur for one of several reasons.
>>> + *
>>> + * 1) An update to an L2 or higher occured. This potentially changes
>>> the
>>> + *pagetable structure, requiring a flush of the linear range.
>>> + * 2) An update to an L4 occured, and XPTI is enabled. All CPUs
>>> running
>>> + *on a copy of this L4 need refreshing.
>>> + */
>>> +if ( flush_flags_all || flush_flags_local )
>> Minor remark: At least in x86 code it is more efficient to use
>> | instead of || in such cases, to avoid relying on the compiler
>> to carry out this small optimization.
>
> This transformation should not be recommended in general. There are
> cases, including this one, where it is at best, no effect, and at worst,
> wrong.
>
> I don't care about people using ancient compilers. They've got far
> bigger (== more impactful) problems than (the absence of) this
> transformation, and the TLB flush will dwarf anythin
Re: [PATCH] x86/mm: avoid playing with directmap when self-snoop can be relied upon
On 21.10.2020 17:23, Roger Pau Monné wrote: > On Tue, Oct 20, 2020 at 03:51:18PM +0200, Jan Beulich wrote: >> The set of systems affected by XSA-345 would have been smaller is we had >> this in place already: When the processor is capable of dealing with >> mismatched cacheability, there's no extra work we need to carry out. >> >> Signed-off-by: Jan Beulich > > Acked-by: Roger Pau Monné Thanks. > I guess it's not worth using the alternative framework to patch this > up at boot in order to avoid the call in the first place? It being non-trivial (afaict) in cases like this one makes me think that the price to do so would be higher than the gain to be had. But I might be wrong ... Jan
Re: [PATCH v2 1/8] evtchn: avoid race in get_xen_consumer()
On 21.10.2020 17:46, Roger Pau Monné wrote:
> On Tue, Oct 20, 2020 at 04:08:13PM +0200, Jan Beulich wrote:
>> There's no global lock around the updating of this global piece of data.
>> Make use of cmpxchgptr() to avoid two entities racing with their
>> updates.
>>
>> While touching the functionality, mark xen_consumers[] read-mostly (or
>> else the if() condition could use the result of cmpxchgptr(), writing to
>> the slot unconditionally).
>
> I'm not sure I get this, likely related to the comment I have below.
This is about the alternative case of invoking cmpxchgptr()
without the if() around it. On x86 this would mean always
writing the field, even if the designated value is already in
place.
>> --- a/xen/common/event_channel.c
>> +++ b/xen/common/event_channel.c
>> @@ -57,7 +57,8 @@
>> * with a pointer, we stash them dynamically in a small lookup array which
>> * can be indexed by a small integer.
>> */
>> -static xen_event_channel_notification_t xen_consumers[NR_XEN_CONSUMERS];
>> +static xen_event_channel_notification_t __read_mostly
>> +xen_consumers[NR_XEN_CONSUMERS];
>>
>> /* Default notification action: wake up from wait_on_xen_event_channel(). */
>> static void default_xen_notification_fn(struct vcpu *v, unsigned int port)
>> @@ -80,8 +81,9 @@ static uint8_t get_xen_consumer(xen_even
>>
>> for ( i = 0; i < ARRAY_SIZE(xen_consumers); i++ )
>> {
>> +/* Use cmpxchgptr() in lieu of a global lock. */
>> if ( xen_consumers[i] == NULL )
>> -xen_consumers[i] = fn;
>> +cmpxchgptr(&xen_consumers[i], NULL, fn);
>> if ( xen_consumers[i] == fn )
>> break;
>
> I think you could join it as:
>
> if ( !xen_consumers[i] &&
> !cmpxchgptr(&xen_consumers[i], NULL, fn) )
> break;
>
> As cmpxchgptr will return the previous value of &xen_consumers[i]?
But then you also have to check whether the returned value is
fn (or retain the 2nd if()).
Jan
[PATCH 3/5] xen/events: only register debug interrupt for 2-level events
xen_debug_interrupt() is specific to 2-level event handling. So don't
register it with fifo event handling being active.
Signed-off-by: Juergen Gross
---
arch/x86/xen/smp.c | 19 +++
arch/x86/xen/xen-ops.h | 2 ++
drivers/xen/events/events_base.c | 6 --
3 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 2097fa0ebdb5..b544e511b3c2 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -88,14 +88,17 @@ int xen_smp_intr_init(unsigned int cpu)
per_cpu(xen_callfunc_irq, cpu).irq = rc;
per_cpu(xen_callfunc_irq, cpu).name = callfunc_name;
- debug_name = kasprintf(GFP_KERNEL, "debug%d", cpu);
- rc = bind_virq_to_irqhandler(VIRQ_DEBUG, cpu, xen_debug_interrupt,
-IRQF_PERCPU | IRQF_NOBALANCING,
-debug_name, NULL);
- if (rc < 0)
- goto fail;
- per_cpu(xen_debug_irq, cpu).irq = rc;
- per_cpu(xen_debug_irq, cpu).name = debug_name;
+ if (!fifo_events) {
+ debug_name = kasprintf(GFP_KERNEL, "debug%d", cpu);
+ rc = bind_virq_to_irqhandler(VIRQ_DEBUG, cpu,
+xen_debug_interrupt,
+IRQF_PERCPU | IRQF_NOBALANCING,
+debug_name, NULL);
+ if (rc < 0)
+ goto fail;
+ per_cpu(xen_debug_irq, cpu).irq = rc;
+ per_cpu(xen_debug_irq, cpu).name = debug_name;
+ }
callfunc_name = kasprintf(GFP_KERNEL, "callfuncsingle%d", cpu);
rc = bind_ipi_to_irqhandler(XEN_CALL_FUNCTION_SINGLE_VECTOR,
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 45d556f71858..e444c78b6e2b 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -29,6 +29,8 @@ extern struct start_info *xen_start_info;
extern struct shared_info xen_dummy_shared_info;
extern struct shared_info *HYPERVISOR_shared_info;
+extern bool fifo_events;
+
void xen_setup_mfn_list_list(void);
void xen_build_mfn_list_list(void);
void xen_setup_machphys_mapping(void);
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 1c25580c7691..bb18cce4db06 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -2050,7 +2050,7 @@ void xen_setup_callback_vector(void) {}
static inline void xen_alloc_callback_vector(void) {}
#endif
-static bool fifo_events = true;
+bool fifo_events = true;
module_param(fifo_events, bool, 0);
static int xen_evtchn_cpu_prepare(unsigned int cpu)
@@ -2082,8 +2082,10 @@ void __init xen_init_IRQ(void)
if (fifo_events)
ret = xen_evtchn_fifo_init();
- if (ret < 0)
+ if (ret < 0) {
xen_evtchn_2l_init();
+ fifo_events = false;
+ }
xen_cpu_init_eoi(smp_processor_id());
--
2.26.2
[PATCH 2/5] xen/events: make struct irq_info private to events_base.c
The struct irq_info of Xen's event handling is used only for two
evtchn_ops functions outside of events_base.c. Those two functions
can easily be switched to avoid that usage.
This allows to make struct irq_info and its related access functions
private to events_base.c.
Signed-off-by: Juergen Gross
---
drivers/xen/events/events_2l.c | 7 +--
drivers/xen/events/events_base.c | 63 ++---
drivers/xen/events/events_fifo.c | 6 +--
drivers/xen/events/events_internal.h | 70
4 files changed, 73 insertions(+), 73 deletions(-)
diff --git a/drivers/xen/events/events_2l.c b/drivers/xen/events/events_2l.c
index fe5ad0e89cd8..da87f3a1e351 100644
--- a/drivers/xen/events/events_2l.c
+++ b/drivers/xen/events/events_2l.c
@@ -47,10 +47,11 @@ static unsigned evtchn_2l_max_channels(void)
return EVTCHN_2L_NR_CHANNELS;
}
-static void evtchn_2l_bind_to_cpu(struct irq_info *info, unsigned cpu)
+static void evtchn_2l_bind_to_cpu(evtchn_port_t evtchn, unsigned int cpu,
+ unsigned int old_cpu)
{
- clear_bit(info->evtchn, BM(per_cpu(cpu_evtchn_mask, info->cpu)));
- set_bit(info->evtchn, BM(per_cpu(cpu_evtchn_mask, cpu)));
+ clear_bit(evtchn, BM(per_cpu(cpu_evtchn_mask, old_cpu)));
+ set_bit(evtchn, BM(per_cpu(cpu_evtchn_mask, cpu)));
}
static void evtchn_2l_clear_pending(evtchn_port_t port)
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 436682db41c5..1c25580c7691 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -70,6 +70,57 @@
#undef MODULE_PARAM_PREFIX
#define MODULE_PARAM_PREFIX "xen."
+/* Interrupt types. */
+enum xen_irq_type {
+ IRQT_UNBOUND = 0,
+ IRQT_PIRQ,
+ IRQT_VIRQ,
+ IRQT_IPI,
+ IRQT_EVTCHN
+};
+
+/*
+ * Packed IRQ information:
+ * type - enum xen_irq_type
+ * event channel - irq->event channel mapping
+ * cpu - cpu this event channel is bound to
+ * index - type-specific information:
+ *PIRQ - vector, with MSB being "needs EIO", or physical IRQ of the HVM
+ * guest, or GSI (real passthrough IRQ) of the device.
+ *VIRQ - virq number
+ *IPI - IPI vector
+ *EVTCHN -
+ */
+struct irq_info {
+ struct list_head list;
+ struct list_head eoi_list;
+ short refcnt;
+ short spurious_cnt;
+ enum xen_irq_type type; /* type */
+ unsigned irq;
+ evtchn_port_t evtchn; /* event channel */
+ unsigned short cpu; /* cpu bound */
+ unsigned short eoi_cpu; /* EOI must happen on this cpu-1 */
+ unsigned int irq_epoch; /* If eoi_cpu valid: irq_epoch of event */
+ u64 eoi_time; /* Time in jiffies when to EOI. */
+
+ union {
+ unsigned short virq;
+ enum ipi_vector ipi;
+ struct {
+ unsigned short pirq;
+ unsigned short gsi;
+ unsigned char vector;
+ unsigned char flags;
+ uint16_t domid;
+ } pirq;
+ } u;
+};
+
+#define PIRQ_NEEDS_EOI (1 << 0)
+#define PIRQ_SHAREABLE (1 << 1)
+#define PIRQ_MSI_GROUP (1 << 2)
+
static uint __read_mostly event_loop_timeout = 2;
module_param(event_loop_timeout, uint, 0644);
@@ -110,7 +161,7 @@ static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0
... NR_VIRQS-1] = -1};
/* IRQ <-> IPI mapping */
static DEFINE_PER_CPU(int [XEN_NR_IPIS], ipi_to_irq) = {[0 ... XEN_NR_IPIS-1]
= -1};
-int **evtchn_to_irq;
+static int **evtchn_to_irq;
#ifdef CONFIG_X86
static unsigned long *pirq_eoi_map;
#endif
@@ -190,7 +241,7 @@ int get_evtchn_to_irq(evtchn_port_t evtchn)
}
/* Get info for IRQ */
-struct irq_info *info_for_irq(unsigned irq)
+static struct irq_info *info_for_irq(unsigned irq)
{
if (irq < nr_legacy_irqs())
return legacy_info_ptrs[irq];
@@ -228,7 +279,7 @@ static int xen_irq_info_common_setup(struct irq_info *info,
irq_clear_status_flags(irq, IRQ_NOREQUEST|IRQ_NOAUTOEN);
- return xen_evtchn_port_setup(info);
+ return xen_evtchn_port_setup(evtchn);
}
static int xen_irq_info_evtchn_setup(unsigned irq,
@@ -351,7 +402,7 @@ static enum xen_irq_type type_from_irq(unsigned irq)
return info_for_irq(irq)->type;
}
-unsigned cpu_from_irq(unsigned irq)
+static unsigned cpu_from_irq(unsigned irq)
{
return info_for_irq(irq)->cpu;
}
@@ -391,7 +442,7 @@ static void bind_evtchn_to_cpu(evtchn_port_t evtchn,
unsigned int cpu)
#ifdef CONFIG_SMP
cpumask_copy(irq_get_affinity_mask(irq), cpumask_of(cpu));
#endif
- xen_evtchn_port_bind_to_cpu(info, cpu);
+ xen_evtchn_port_bind_to_cpu(evtchn, cpu, info->cpu);
info->cpu = cpu;
}
@@ -745,7 +796,7 @@ static unsigned int __startup_pirq(unsigned int irq)
info->evtchn = evtchn;
bind_evtchn_to_cpu(evtchn, 0);
- rc = xen_evt
Re: [PATCH] xen/acpi: Don't fail if SPCR table is absent
On 22.10.2020 00:12, Elliott Mitchell wrote:
> --- a/xen/arch/arm/acpi/domain_build.c
> +++ b/xen/arch/arm/acpi/domain_build.c
> @@ -42,17 +42,18 @@ static int __init acpi_iomem_deny_access(struct domain *d)
> status = acpi_get_table(ACPI_SIG_SPCR, 0,
> (struct acpi_table_header **)&spcr);
>
> -if ( ACPI_FAILURE(status) )
> +if ( ACPI_SUCCESS(status) )
> {
> -printk("Failed to get SPCR table\n");
> -return -EINVAL;
> +mfn = spcr->serial_port.address >> PAGE_SHIFT;
> +/* Deny MMIO access for UART */
> +rc = iomem_deny_access(d, mfn, mfn + 1);
> +if ( rc )
> +return rc;
> +}
> +else
> +{
> +printk("Failed to get SPCR table, Xen console may be unavailable\n");
> }
Nit: While I see you've got Stefano's R-b already, I Xen we typically
omit the braces here.
Jan
[PATCH 0/5] xen: event handling cleanup
Do some cleanups in Xen event handling code. Juergen Gross (5): xen: remove no longer used functions xen/events: make struct irq_info private to events_base.c xen/events: only register debug interrupt for 2-level events xen/events: unmask a fifo event channel only if it was masked Documentation: add xen.fifo_events kernel parameter description .../admin-guide/kernel-parameters.txt | 7 ++ arch/x86/xen/smp.c| 19 ++-- arch/x86/xen/xen-ops.h| 2 + drivers/xen/events/events_2l.c| 7 +- drivers/xen/events/events_base.c | 90 +-- drivers/xen/events/events_fifo.c | 9 +- drivers/xen/events/events_internal.h | 70 ++- include/xen/events.h | 8 -- 8 files changed, 100 insertions(+), 112 deletions(-) -- 2.26.2
[PATCH 4/5] xen/events: unmask a fifo event channel only if it was masked
Unmasking an event channel with fifo events channels being used can
require a hypercall to be made, so try to avoid that by checking
whether the event channel was really masked.
Suggested-by: Jan Beulich
Signed-off-by: Juergen Gross
---
drivers/xen/events/events_fifo.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/xen/events/events_fifo.c b/drivers/xen/events/events_fifo.c
index 243e7b6d7b96..f60c5a9ec833 100644
--- a/drivers/xen/events/events_fifo.c
+++ b/drivers/xen/events/events_fifo.c
@@ -236,6 +236,9 @@ static bool clear_masked_cond(volatile event_word_t *word)
w = *word;
+ if (!(w & (1 << EVTCHN_FIFO_MASKED)))
+ return true;
+
do {
if (w & (1 << EVTCHN_FIFO_PENDING))
return false;
--
2.26.2
[PATCH 5/5] Documentation: add xen.fifo_events kernel parameter description
The kernel boot parameter xen.fifo_events isn't listed in Documentation/admin-guide/kernel-parameters.txt. Add it. Signed-off-by: Juergen Gross --- Documentation/admin-guide/kernel-parameters.txt | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 02d4adbf98d2..526d65d8573a 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -5978,6 +5978,13 @@ After which time (jiffies) the event handling loop should start to delay EOI handling. Default is 2. + xen.fifo_events=[XEN] + Boolean parameter to disable using fifo event handling + even if available. Normally fifo event handling is + preferred over the 2-level event handling, as it is + fairer and the number of possible event channels is + much higher. Default is on (use fifo events). + nopv= [X86,XEN,KVM,HYPER_V,VMWARE] Disables the PV optimizations forcing the guest to run as generic guest with no PV drivers. Currently support -- 2.26.2
[PATCH 1/5] xen: remove no longer used functions
With the switch to the lateeoi model for interdomain event channels
some functions are no longer in use. Remove them.
Suggested-by: Jan Beulich
Signed-off-by: Juergen Gross
---
drivers/xen/events/events_base.c | 21 -
include/xen/events.h | 8
2 files changed, 29 deletions(-)
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index cc317739e786..436682db41c5 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1145,14 +1145,6 @@ static int bind_interdomain_evtchn_to_irq_chip(unsigned
int remote_domain,
chip);
}
-int bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
- evtchn_port_t remote_port)
-{
- return bind_interdomain_evtchn_to_irq_chip(remote_domain, remote_port,
- &xen_dynamic_chip);
-}
-EXPORT_SYMBOL_GPL(bind_interdomain_evtchn_to_irq);
-
int bind_interdomain_evtchn_to_irq_lateeoi(unsigned int remote_domain,
evtchn_port_t remote_port)
{
@@ -1320,19 +1312,6 @@ static int bind_interdomain_evtchn_to_irqhandler_chip(
return irq;
}
-int bind_interdomain_evtchn_to_irqhandler(unsigned int remote_domain,
- evtchn_port_t remote_port,
- irq_handler_t handler,
- unsigned long irqflags,
- const char *devname,
- void *dev_id)
-{
- return bind_interdomain_evtchn_to_irqhandler_chip(remote_domain,
- remote_port, handler, irqflags, devname,
- dev_id, &xen_dynamic_chip);
-}
-EXPORT_SYMBOL_GPL(bind_interdomain_evtchn_to_irqhandler);
-
int bind_interdomain_evtchn_to_irqhandler_lateeoi(unsigned int remote_domain,
evtchn_port_t remote_port,
irq_handler_t handler,
diff --git a/include/xen/events.h b/include/xen/events.h
index 3b8155c2ea03..8ec418e30c7f 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -35,16 +35,8 @@ int bind_ipi_to_irqhandler(enum ipi_vector ipi,
unsigned long irqflags,
const char *devname,
void *dev_id);
-int bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
- evtchn_port_t remote_port);
int bind_interdomain_evtchn_to_irq_lateeoi(unsigned int remote_domain,
evtchn_port_t remote_port);
-int bind_interdomain_evtchn_to_irqhandler(unsigned int remote_domain,
- evtchn_port_t remote_port,
- irq_handler_t handler,
- unsigned long irqflags,
- const char *devname,
- void *dev_id);
int bind_interdomain_evtchn_to_irqhandler_lateeoi(unsigned int remote_domain,
evtchn_port_t remote_port,
irq_handler_t handler,
--
2.26.2
Re: [xen-unstable test] 156050: regressions - FAIL
On 22.10.2020 07:58, osstest service owner wrote:
> flight 156050 xen-unstable real [real]
> flight 156084 xen-unstable real-retest [real]
> http://logs.test-lab.xenproject.org/osstest/logs/156050/
> http://logs.test-lab.xenproject.org/osstest/logs/156084/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail
> REGR. vs. 156013
Continuing from my reply to the earlier flight yesterday I'm
meanwhile even more puzzled because 4.12 and earlier have had
pushes, i.e. the tests were successful there that have been
failing for 4.13 and newer. It's further suspicious (to me)
that in each case it's just one of the qemu{u,t} tests which
fails, while its sibling is successful. This may mean a
dependency on the particular hardware we're running on, but
again I wouldn't be able to make a connection of such
behavior to the commits under test.
Jan
Re: [xen-unstable test] 156050: regressions - FAIL
On 22.10.2020 09:46, Jan Beulich wrote:
> On 22.10.2020 07:58, osstest service owner wrote:
>> flight 156050 xen-unstable real [real]
>> flight 156084 xen-unstable real-retest [real]
>> http://logs.test-lab.xenproject.org/osstest/logs/156050/
>> http://logs.test-lab.xenproject.org/osstest/logs/156084/
>>
>> Regressions :-(
>>
>> Tests which did not succeed and are blocking,
>> including tests which could not be run:
>> test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail
>> REGR. vs. 156013
>
> Continuing from my reply to the earlier flight yesterday I'm
> meanwhile even more puzzled because 4.12 and earlier have had
> pushes, i.e. the tests were successful there that have been
> failing for 4.13 and newer. It's further suspicious (to me)
> that in each case it's just one of the qemu{u,t} tests which
> fails, while its sibling is successful. This may mean a
> dependency on the particular hardware we're running on, but
> again I wouldn't be able to make a connection of such
> behavior to the commits under test.
Actually, yesterday's 4.14 and 4.13 flights speak against a
hardware dependency: Their failing vs successful qemu{u,t}
each ran on respectively the same host.
For now I'm lost; I'd appreciate others to take a look.
Jan
Re: [PATCH 3/5] xen/events: only register debug interrupt for 2-level events
On 22.10.2020 09:42, Juergen Gross wrote:
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -2050,7 +2050,7 @@ void xen_setup_callback_vector(void) {}
> static inline void xen_alloc_callback_vector(void) {}
> #endif
>
> -static bool fifo_events = true;
> +bool fifo_events = true;
When making this non-static, perhaps better to also prefix it with
xen_?
Jan
Re: [PATCH 4/5] xen/events: unmask a fifo event channel only if it was masked
On 22.10.2020 09:42, Juergen Gross wrote:
> --- a/drivers/xen/events/events_fifo.c
> +++ b/drivers/xen/events/events_fifo.c
> @@ -236,6 +236,9 @@ static bool clear_masked_cond(volatile event_word_t *word)
>
> w = *word;
>
> + if (!(w & (1 << EVTCHN_FIFO_MASKED)))
> + return true;
Maybe better move this ...
> do {
> if (w & (1 << EVTCHN_FIFO_PENDING))
> return false;
>
... into the loop, above this check?
Jan
Re: [PATCH 0/5] xen: event handling cleanup
On 22.10.2020 09:42, Juergen Gross wrote: > Do some cleanups in Xen event handling code. > > Juergen Gross (5): > xen: remove no longer used functions > xen/events: make struct irq_info private to events_base.c > xen/events: only register debug interrupt for 2-level events > xen/events: unmask a fifo event channel only if it was masked > Documentation: add xen.fifo_events kernel parameter description With the two remarks to individual patches suitably taken care of one way or another Reviewed-by: Jan Beulich Jan
Re: [PATCH v5 10/10] drm/fb_helper: Support framebuffers in I/O memory
On Tue, Oct 20, 2020 at 02:20:46PM +0200, Thomas Zimmermann wrote:
> At least sparc64 requires I/O-specific access to framebuffers. This
> patch updates the fbdev console accordingly.
>
> For drivers with direct access to the framebuffer memory, the callback
> functions in struct fb_ops test for the type of memory and call the rsp
> fb_sys_ of fb_cfb_ functions. Read and write operations are implemented
> internally by DRM's fbdev helper.
>
> For drivers that employ a shadow buffer, fbdev's blit function retrieves
> the framebuffer address as struct dma_buf_map, and uses dma_buf_map
> interfaces to access the buffer.
>
> The bochs driver on sparc64 uses a workaround to flag the framebuffer as
> I/O memory and avoid a HW exception. With the introduction of struct
> dma_buf_map, this is not required any longer. The patch removes the rsp
> code from both, bochs and fbdev.
>
> v5:
> * implement fb_read/fb_write internally (Daniel, Sam)
> v4:
> * move dma_buf_map changes into separate patch (Daniel)
> * TODO list: comment on fbdev updates (Daniel)
>
> Signed-off-by: Thomas Zimmermann
> Tested-by: Sam Ravnborg
> ---
> Documentation/gpu/todo.rst| 19 ++-
> drivers/gpu/drm/bochs/bochs_kms.c | 1 -
> drivers/gpu/drm/drm_fb_helper.c | 227 --
> include/drm/drm_mode_config.h | 12 --
> 4 files changed, 230 insertions(+), 29 deletions(-)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 7e6fc3c04add..638b7f704339 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -197,13 +197,28 @@ Convert drivers to use drm_fbdev_generic_setup()
>
>
> Most drivers can use drm_fbdev_generic_setup(). Driver have to implement
> -atomic modesetting and GEM vmap support. Current generic fbdev emulation
> -expects the framebuffer in system memory (or system-like memory).
> +atomic modesetting and GEM vmap support. Historically, generic fbdev
> emulation
> +expected the framebuffer in system memory or system-like memory. By employing
> +struct dma_buf_map, drivers with frambuffers in I/O memory can be supported
> +as well.
>
> Contact: Maintainer of the driver you plan to convert
>
> Level: Intermediate
>
> +Reimplement functions in drm_fbdev_fb_ops without fbdev
> +---
> +
> +A number of callback functions in drm_fbdev_fb_ops could benefit from
> +being rewritten without dependencies on the fbdev module. Some of the
> +helpers could further benefit from using struct dma_buf_map instead of
> +raw pointers.
> +
> +Contact: Thomas Zimmermann , Daniel Vetter
> +
> +Level: Advanced
> +
> +
> drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup
> -
>
> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
> b/drivers/gpu/drm/bochs/bochs_kms.c
> index 13d0d04c4457..853081d186d5 100644
> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> @@ -151,7 +151,6 @@ int bochs_kms_init(struct bochs_device *bochs)
> bochs->dev->mode_config.preferred_depth = 24;
> bochs->dev->mode_config.prefer_shadow = 0;
> bochs->dev->mode_config.prefer_shadow_fbdev = 1;
> - bochs->dev->mode_config.fbdev_use_iomem = true;
> bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
>
> bochs->dev->mode_config.funcs = &bochs_mode_funcs;
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 6212cd7cde1d..1d3180841778 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -372,24 +372,22 @@ static void drm_fb_helper_resume_worker(struct
> work_struct *work)
> }
>
> static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
> - struct drm_clip_rect *clip)
> + struct drm_clip_rect *clip,
> + struct dma_buf_map *dst)
> {
> struct drm_framebuffer *fb = fb_helper->fb;
> unsigned int cpp = fb->format->cpp[0];
> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> void *src = fb_helper->fbdev->screen_buffer + offset;
> - void *dst = fb_helper->buffer->map.vaddr + offset;
> size_t len = (clip->x2 - clip->x1) * cpp;
> unsigned int y;
>
> - for (y = clip->y1; y < clip->y2; y++) {
> - if (!fb_helper->dev->mode_config.fbdev_use_iomem)
> - memcpy(dst, src, len);
> - else
> - memcpy_toio((void __iomem *)dst, src, len);
> + dma_buf_map_incr(dst, offset); /* go to first pixel within clip rect */
>
> + for (y = clip->y1; y < clip->y2; y++) {
> + dma_buf_map_memcpy_to(dst, src, len);
> + dma_buf_map_incr(dst, fb->pitches[0]);
> src +
Re: [PATCH v2 1/8] evtchn: avoid race in get_xen_consumer()
On Thu, Oct 22, 2020 at 09:33:27AM +0200, Jan Beulich wrote:
> On 21.10.2020 17:46, Roger Pau Monné wrote:
> > On Tue, Oct 20, 2020 at 04:08:13PM +0200, Jan Beulich wrote:
> >> There's no global lock around the updating of this global piece of data.
> >> Make use of cmpxchgptr() to avoid two entities racing with their
> >> updates.
> >>
> >> While touching the functionality, mark xen_consumers[] read-mostly (or
> >> else the if() condition could use the result of cmpxchgptr(), writing to
> >> the slot unconditionally).
> >
> > I'm not sure I get this, likely related to the comment I have below.
>
> This is about the alternative case of invoking cmpxchgptr()
> without the if() around it. On x86 this would mean always
> writing the field, even if the designated value is already in
> place.
>
> >> --- a/xen/common/event_channel.c
> >> +++ b/xen/common/event_channel.c
> >> @@ -57,7 +57,8 @@
> >> * with a pointer, we stash them dynamically in a small lookup array which
> >> * can be indexed by a small integer.
> >> */
> >> -static xen_event_channel_notification_t xen_consumers[NR_XEN_CONSUMERS];
> >> +static xen_event_channel_notification_t __read_mostly
> >> +xen_consumers[NR_XEN_CONSUMERS];
> >>
> >> /* Default notification action: wake up from wait_on_xen_event_channel().
> >> */
> >> static void default_xen_notification_fn(struct vcpu *v, unsigned int port)
> >> @@ -80,8 +81,9 @@ static uint8_t get_xen_consumer(xen_even
> >>
> >> for ( i = 0; i < ARRAY_SIZE(xen_consumers); i++ )
> >> {
> >> +/* Use cmpxchgptr() in lieu of a global lock. */
> >> if ( xen_consumers[i] == NULL )
> >> -xen_consumers[i] = fn;
> >> +cmpxchgptr(&xen_consumers[i], NULL, fn);
> >> if ( xen_consumers[i] == fn )
> >> break;
> >
> > I think you could join it as:
> >
> > if ( !xen_consumers[i] &&
> > !cmpxchgptr(&xen_consumers[i], NULL, fn) )
> > break;
> >
> > As cmpxchgptr will return the previous value of &xen_consumers[i]?
>
> But then you also have to check whether the returned value is
> fn (or retain the 2nd if()).
__cmpxchg comment says that success of the operation is indicated when
the returned value equals the old value, so it's my understanding that
cmpxchgptr returning NULL would mean the exchange has succeed and that
xen_consumers[i] == fn?
Roger.
Re: XSM and the idle domain
On 22.10.2020 03:23, Daniel P. Smith wrote: > On 10/21/20 10:34 AM, Hongyan Xia wrote: >> Also, should idle domain be restricted? IMO the idle domain is Xen >> itself which mostly bootstraps the system and performs limited work >> when switched to, and is not something a user (either dom0 or domU) >> directly interacts with. I doubt XSM was designed to include the idle >> domain (although there is an ID allocated for it in the code), so I >> would say just exclude idle in all security policy checks. > > The idle domain is a limited, internal construct within the hypervisor > and should be constrained as part of the hypervisor, which is why its > domain id gets labeled with the same label as the hypervisor. For this > reason I would wholeheartedly disagree with exempting the idle domain id > from XSM hooks as that would effectively be saying the core hypervisor > should not be constrained. The purpose of the XSM hooks is to control > the flow of information in the system in a non-bypassable way. Codifying > bypasses completely subverts the security model behind XSM for which the > flask security server is dependent upon. While what you say may in general make sense, I have two questions: 1) When the idle domain is purely an internal construct of Xen, why does it need limiting in any way? In fact, if restricting it in a bad way, aren't you risking to prevent the system from functioning correctly? 2) LU is merely restoring the prior state of the system. This prior state was reached with security auditing as per the system's policy at the time. Why should there be anything denind in the process of re-establishing this same state? IOW can't XSM checking be globally disabled until the system is ready be run normally again? Please forgive if this sounds like rubbish to you - I may not have a good enough understanding of the abstract constraints involved here. Jan
Re: [PATCH v2 1/8] evtchn: avoid race in get_xen_consumer()
On 22.10.2020 10:11, Roger Pau Monné wrote:
> On Thu, Oct 22, 2020 at 09:33:27AM +0200, Jan Beulich wrote:
>> On 21.10.2020 17:46, Roger Pau Monné wrote:
>>> On Tue, Oct 20, 2020 at 04:08:13PM +0200, Jan Beulich wrote:
@@ -80,8 +81,9 @@ static uint8_t get_xen_consumer(xen_even
for ( i = 0; i < ARRAY_SIZE(xen_consumers); i++ )
{
+/* Use cmpxchgptr() in lieu of a global lock. */
if ( xen_consumers[i] == NULL )
-xen_consumers[i] = fn;
+cmpxchgptr(&xen_consumers[i], NULL, fn);
if ( xen_consumers[i] == fn )
break;
>>>
>>> I think you could join it as:
>>>
>>> if ( !xen_consumers[i] &&
>>> !cmpxchgptr(&xen_consumers[i], NULL, fn) )
>>> break;
>>>
>>> As cmpxchgptr will return the previous value of &xen_consumers[i]?
>>
>> But then you also have to check whether the returned value is
>> fn (or retain the 2nd if()).
>
> __cmpxchg comment says that success of the operation is indicated when
> the returned value equals the old value, so it's my understanding that
> cmpxchgptr returning NULL would mean the exchange has succeed and that
> xen_consumers[i] == fn?
Correct. But if xen_consumers[i] == fn before the call, the return
value will be fn. The cmpxchg() wasn't "successful" in this case
(it didn't update anything), but the state of the array slot is what
we want.
Jan
Re: [PATCH 3/5] xen/events: only register debug interrupt for 2-level events
On 22.10.20 09:54, Jan Beulich wrote:
On 22.10.2020 09:42, Juergen Gross wrote:
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -2050,7 +2050,7 @@ void xen_setup_callback_vector(void) {}
static inline void xen_alloc_callback_vector(void) {}
#endif
-static bool fifo_events = true;
+bool fifo_events = true;
When making this non-static, perhaps better to also prefix it with
xen_?
Fine with me.
Juergen
Re: [PATCH 4/5] xen/events: unmask a fifo event channel only if it was masked
On 22.10.20 09:55, Jan Beulich wrote:
On 22.10.2020 09:42, Juergen Gross wrote:
--- a/drivers/xen/events/events_fifo.c
+++ b/drivers/xen/events/events_fifo.c
@@ -236,6 +236,9 @@ static bool clear_masked_cond(volatile event_word_t *word)
w = *word;
+ if (!(w & (1 << EVTCHN_FIFO_MASKED)))
+ return true;
Maybe better move this ...
do {
if (w & (1 << EVTCHN_FIFO_PENDING))
return false;
... into the loop, above this check?
Yes, that should be better.
Juergen
Re: [PATCH v2 1/8] evtchn: avoid race in get_xen_consumer()
On Thu, Oct 22, 2020 at 10:15:45AM +0200, Jan Beulich wrote:
> On 22.10.2020 10:11, Roger Pau Monné wrote:
> > On Thu, Oct 22, 2020 at 09:33:27AM +0200, Jan Beulich wrote:
> >> On 21.10.2020 17:46, Roger Pau Monné wrote:
> >>> On Tue, Oct 20, 2020 at 04:08:13PM +0200, Jan Beulich wrote:
> @@ -80,8 +81,9 @@ static uint8_t get_xen_consumer(xen_even
>
> for ( i = 0; i < ARRAY_SIZE(xen_consumers); i++ )
> {
> +/* Use cmpxchgptr() in lieu of a global lock. */
> if ( xen_consumers[i] == NULL )
> -xen_consumers[i] = fn;
> +cmpxchgptr(&xen_consumers[i], NULL, fn);
> if ( xen_consumers[i] == fn )
> break;
> >>>
> >>> I think you could join it as:
> >>>
> >>> if ( !xen_consumers[i] &&
> >>> !cmpxchgptr(&xen_consumers[i], NULL, fn) )
> >>> break;
> >>>
> >>> As cmpxchgptr will return the previous value of &xen_consumers[i]?
> >>
> >> But then you also have to check whether the returned value is
> >> fn (or retain the 2nd if()).
> >
> > __cmpxchg comment says that success of the operation is indicated when
> > the returned value equals the old value, so it's my understanding that
> > cmpxchgptr returning NULL would mean the exchange has succeed and that
> > xen_consumers[i] == fn?
>
> Correct. But if xen_consumers[i] == fn before the call, the return
> value will be fn. The cmpxchg() wasn't "successful" in this case
> (it didn't update anything), but the state of the array slot is what
> we want.
Oh, I get it now. You don't want the same fn populating more than one
slot.
I assume the reads of xen_consumers are not using ACCESS_ONCE or
read_atomic because we rely on the compiler performing such reads as
single instructions?
Roger.
Re: [XEN PATCH v1] xen/arm : Add support for SMMUv3 driver
On 21/10/2020 12:25, Rahul Singh wrote: Hello Julien, Hi Rahul, On 20 Oct 2020, at 6:03 pm, Julien Grall wrote: Hi Rahul, Thank you for the contribution. Lets make sure this attempt to SMMUv3 support in Xen will be more successful than the other one :). Yes sure. I haven't reviewed the code yet, but I wanted to provide feedback on the commit message. On 20/10/2020 16:25, Rahul Singh wrote: Add support for ARM architected SMMUv3 implementations. It is based on the Linux SMMUv3 driver. Major differences between the Linux driver are as follows: 1. Only Stage-2 translation is supported as compared to the Linux driver that supports both Stage-1 and Stage-2 translations. 2. Use P2M page table instead of creating one as SMMUv3 has the capability to share the page tables with the CPU. 3. Tasklets is used in place of threaded IRQ's in Linux for event queue and priority queue IRQ handling. Tasklets are not a replacement for threaded IRQ. In particular, they will have priority over anything else (IOW nothing will run on the pCPU until they are done). Do you know why Linux is using thread. Is it because of long running operations? Yes you are right because of long running operations Linux is using the threaded IRQs. SMMUv3 reports fault/events bases on memory-based circular buffer queues not based on the register. As per my understanding, it is time-consuming to process the memory based queues in interrupt context because of that Linux is using threaded IRQ to process the faults/events from SMMU. I didn’t find any other solution in XEN in place of tasklet to defer the work, that’s why I used tasklet in XEN in replacement of threaded IRQs. If we do all work in interrupt context we will make XEN less responsive. So we need to make sure that Xen continue to receives interrupts, but we also need to make sure that a vCPU bound to the pCPU is also responsive. If you know another solution in XEN that will be used to defer the work in the interrupt please let me know I will try to use that. One of my work colleague encountered a similar problem recently. He had a long running tasklet and wanted to be broken down in smaller chunk. We decided to use a timer to reschedule the taslket in the future. This allows the scheduler to run other loads (e.g. vCPU) for some time. This is pretty hackish but I couldn't find a better solution as tasklet have high priority. Maybe the other will have a better idea. 4. Latest version of the Linux SMMUv3 code implements the commands queue access functions based on atomic operations implemented in Linux. Can you provide more details? I tried to port the latest version of the SMMUv3 code than I observed that in order to port that code I have to also port atomic operation implemented in Linux to XEN. As latest Linux code uses atomic operation to process the command queues (atomic_cond_read_relaxed(),atomic_long_cond_read_relaxed() , atomic_fetch_andnot_relaxed()) . Thank you for the explanation. I think it would be best to import the atomic helpers and use the latest code. This will ensure that we don't re-introduce bugs and also buy us some time before the Linux and Xen driver diverge again too much. Stefano, what do you think? Atomic functions used by the commands queue access functions is not implemented in XEN therefore we decided to port the earlier version of the code. Once the proper atomic operations will be available in XEN the driver can be updated. Signed-off-by: Rahul Singh --- xen/drivers/passthrough/Kconfig | 10 + xen/drivers/passthrough/arm/Makefile |1 + xen/drivers/passthrough/arm/smmu-v3.c | 2847 + 3 files changed, 2858 insertions(+) This is quite significant patch to review. Is there any way to get it split (maybe a verbatim Linux copy + Xen modification)? Yes, I understand this is a quite significant patch to review let me think to get it split. If it is ok for you to review this patch and provide your comments then it will great for us. I will try to have a look next week. Cheers, -- Julien Grall
Re: [PATCH v5 10/10] drm/fb_helper: Support framebuffers in I/O memory
Hi
On 22.10.20 10:05, Daniel Vetter wrote:
> On Tue, Oct 20, 2020 at 02:20:46PM +0200, Thomas Zimmermann wrote:
>> At least sparc64 requires I/O-specific access to framebuffers. This
>> patch updates the fbdev console accordingly.
>>
>> For drivers with direct access to the framebuffer memory, the callback
>> functions in struct fb_ops test for the type of memory and call the rsp
>> fb_sys_ of fb_cfb_ functions. Read and write operations are implemented
>> internally by DRM's fbdev helper.
>>
>> For drivers that employ a shadow buffer, fbdev's blit function retrieves
>> the framebuffer address as struct dma_buf_map, and uses dma_buf_map
>> interfaces to access the buffer.
>>
>> The bochs driver on sparc64 uses a workaround to flag the framebuffer as
>> I/O memory and avoid a HW exception. With the introduction of struct
>> dma_buf_map, this is not required any longer. The patch removes the rsp
>> code from both, bochs and fbdev.
>>
>> v5:
>> * implement fb_read/fb_write internally (Daniel, Sam)
>> v4:
>> * move dma_buf_map changes into separate patch (Daniel)
>> * TODO list: comment on fbdev updates (Daniel)
>>
>> Signed-off-by: Thomas Zimmermann
>> Tested-by: Sam Ravnborg
>> ---
>> Documentation/gpu/todo.rst| 19 ++-
>> drivers/gpu/drm/bochs/bochs_kms.c | 1 -
>> drivers/gpu/drm/drm_fb_helper.c | 227 --
>> include/drm/drm_mode_config.h | 12 --
>> 4 files changed, 230 insertions(+), 29 deletions(-)
>>
>> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
>> index 7e6fc3c04add..638b7f704339 100644
>> --- a/Documentation/gpu/todo.rst
>> +++ b/Documentation/gpu/todo.rst
>> @@ -197,13 +197,28 @@ Convert drivers to use drm_fbdev_generic_setup()
>>
>>
>> Most drivers can use drm_fbdev_generic_setup(). Driver have to implement
>> -atomic modesetting and GEM vmap support. Current generic fbdev emulation
>> -expects the framebuffer in system memory (or system-like memory).
>> +atomic modesetting and GEM vmap support. Historically, generic fbdev
>> emulation
>> +expected the framebuffer in system memory or system-like memory. By
>> employing
>> +struct dma_buf_map, drivers with frambuffers in I/O memory can be supported
>> +as well.
>>
>> Contact: Maintainer of the driver you plan to convert
>>
>> Level: Intermediate
>>
>> +Reimplement functions in drm_fbdev_fb_ops without fbdev
>> +---
>> +
>> +A number of callback functions in drm_fbdev_fb_ops could benefit from
>> +being rewritten without dependencies on the fbdev module. Some of the
>> +helpers could further benefit from using struct dma_buf_map instead of
>> +raw pointers.
>> +
>> +Contact: Thomas Zimmermann , Daniel Vetter
>> +
>> +Level: Advanced
>> +
>> +
>> drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup
>> -
>>
>> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
>> b/drivers/gpu/drm/bochs/bochs_kms.c
>> index 13d0d04c4457..853081d186d5 100644
>> --- a/drivers/gpu/drm/bochs/bochs_kms.c
>> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
>> @@ -151,7 +151,6 @@ int bochs_kms_init(struct bochs_device *bochs)
>> bochs->dev->mode_config.preferred_depth = 24;
>> bochs->dev->mode_config.prefer_shadow = 0;
>> bochs->dev->mode_config.prefer_shadow_fbdev = 1;
>> -bochs->dev->mode_config.fbdev_use_iomem = true;
>> bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
>>
>> bochs->dev->mode_config.funcs = &bochs_mode_funcs;
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index 6212cd7cde1d..1d3180841778 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -372,24 +372,22 @@ static void drm_fb_helper_resume_worker(struct
>> work_struct *work)
>> }
>>
>> static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
>> - struct drm_clip_rect *clip)
>> + struct drm_clip_rect *clip,
>> + struct dma_buf_map *dst)
>> {
>> struct drm_framebuffer *fb = fb_helper->fb;
>> unsigned int cpp = fb->format->cpp[0];
>> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
>> void *src = fb_helper->fbdev->screen_buffer + offset;
>> -void *dst = fb_helper->buffer->map.vaddr + offset;
>> size_t len = (clip->x2 - clip->x1) * cpp;
>> unsigned int y;
>>
>> -for (y = clip->y1; y < clip->y2; y++) {
>> -if (!fb_helper->dev->mode_config.fbdev_use_iomem)
>> -memcpy(dst, src, len);
>> -else
>> -memcpy_toio((void __iomem *)dst, src, len);
>> +dma_buf_map_incr(dst, offset); /* go to first pixel within clip rect */
>>
>> +for (y = clip->y1; y < clip->y2; y++)
Re: [PATCH v5 08/10] drm/gem: Store client buffer mappings as struct dma_buf_map
On Tue, Oct 20, 2020 at 02:20:44PM +0200, Thomas Zimmermann wrote:
> Kernel DRM clients now store their framebuffer address in an instance
> of struct dma_buf_map. Depending on the buffer's location, the address
> refers to system or I/O memory.
>
> Callers of drm_client_buffer_vmap() receive a copy of the value in
> the call's supplied arguments. It can be accessed and modified with
> dma_buf_map interfaces.
>
> Signed-off-by: Thomas Zimmermann
> Reviewed-by: Daniel Vetter
> Tested-by: Sam Ravnborg
> ---
> drivers/gpu/drm/drm_client.c| 34 +++--
> drivers/gpu/drm/drm_fb_helper.c | 23 +-
> include/drm/drm_client.h| 7 ---
> 3 files changed, 38 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index ac0082bed966..fe573acf1067 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -235,7 +235,7 @@ static void drm_client_buffer_delete(struct
> drm_client_buffer *buffer)
> {
> struct drm_device *dev = buffer->client->dev;
>
> - drm_gem_vunmap(buffer->gem, buffer->vaddr);
> + drm_gem_vunmap(buffer->gem, &buffer->map);
>
> if (buffer->gem)
> drm_gem_object_put(buffer->gem);
> @@ -291,25 +291,31 @@ drm_client_buffer_create(struct drm_client_dev *client,
> u32 width, u32 height, u
> /**
> * drm_client_buffer_vmap - Map DRM client buffer into address space
> * @buffer: DRM client buffer
> + * @map_copy: Returns the mapped memory's address
> *
> * This function maps a client buffer into kernel address space. If the
> - * buffer is already mapped, it returns the mapping's address.
> + * buffer is already mapped, it returns the existing mapping's address.
> *
> * Client buffer mappings are not ref'counted. Each call to
> * drm_client_buffer_vmap() should be followed by a call to
> * drm_client_buffer_vunmap(); or the client buffer should be mapped
> * throughout its lifetime.
> *
> + * The returned address is a copy of the internal value. In contrast to
> + * other vmap interfaces, you don't need it for the client's vunmap
> + * function. So you can modify it at will during blit and draw operations.
> + *
> * Returns:
> - * The mapped memory's address
> + * 0 on success, or a negative errno code otherwise.
> */
> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
> +int
> +drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map
> *map_copy)
> {
> - struct dma_buf_map map;
> + struct dma_buf_map *map = &buffer->map;
> int ret;
>
> - if (buffer->vaddr)
> - return buffer->vaddr;
> + if (dma_buf_map_is_set(map))
> + goto out;
>
> /*
>* FIXME: The dependency on GEM here isn't required, we could
> @@ -319,13 +325,14 @@ void *drm_client_buffer_vmap(struct drm_client_buffer
> *buffer)
>* fd_install step out of the driver backend hooks, to make that
>* final step optional for internal users.
>*/
> - ret = drm_gem_vmap(buffer->gem, &map);
> + ret = drm_gem_vmap(buffer->gem, map);
> if (ret)
> - return ERR_PTR(ret);
> + return ret;
>
> - buffer->vaddr = map.vaddr;
> +out:
> + *map_copy = *map;
>
> - return map.vaddr;
> + return 0;
> }
> EXPORT_SYMBOL(drm_client_buffer_vmap);
>
> @@ -339,10 +346,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
> */
> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
> {
> - struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buffer->vaddr);
> + struct dma_buf_map *map = &buffer->map;
>
> - drm_gem_vunmap(buffer->gem, &map);
> - buffer->vaddr = NULL;
> + drm_gem_vunmap(buffer->gem, map);
> }
> EXPORT_SYMBOL(drm_client_buffer_vunmap);
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index c2f72bb6afb1..6212cd7cde1d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -378,7 +378,7 @@ static void drm_fb_helper_dirty_blit_real(struct
> drm_fb_helper *fb_helper,
> unsigned int cpp = fb->format->cpp[0];
> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> void *src = fb_helper->fbdev->screen_buffer + offset;
> - void *dst = fb_helper->buffer->vaddr + offset;
> + void *dst = fb_helper->buffer->map.vaddr + offset;
> size_t len = (clip->x2 - clip->x1) * cpp;
> unsigned int y;
>
> @@ -400,7 +400,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
> *work)
> struct drm_clip_rect *clip = &helper->dirty_clip;
> struct drm_clip_rect clip_copy;
> unsigned long flags;
> - void *vaddr;
> + struct dma_buf_map map;
> + int ret;
>
> spin_lock_irqsave(&helper->dirty_lock, flags);
> clip_copy = *clip;
> @@ -413,8 +414,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
> *work)
>
>
Re: [PATCH v5 10/10] drm/fb_helper: Support framebuffers in I/O memory
On Thu, Oct 22, 2020 at 10:37:56AM +0200, Thomas Zimmermann wrote:
> Hi
>
> On 22.10.20 10:05, Daniel Vetter wrote:
> > On Tue, Oct 20, 2020 at 02:20:46PM +0200, Thomas Zimmermann wrote:
> >> At least sparc64 requires I/O-specific access to framebuffers. This
> >> patch updates the fbdev console accordingly.
> >>
> >> For drivers with direct access to the framebuffer memory, the callback
> >> functions in struct fb_ops test for the type of memory and call the rsp
> >> fb_sys_ of fb_cfb_ functions. Read and write operations are implemented
> >> internally by DRM's fbdev helper.
> >>
> >> For drivers that employ a shadow buffer, fbdev's blit function retrieves
> >> the framebuffer address as struct dma_buf_map, and uses dma_buf_map
> >> interfaces to access the buffer.
> >>
> >> The bochs driver on sparc64 uses a workaround to flag the framebuffer as
> >> I/O memory and avoid a HW exception. With the introduction of struct
> >> dma_buf_map, this is not required any longer. The patch removes the rsp
> >> code from both, bochs and fbdev.
> >>
> >> v5:
> >>* implement fb_read/fb_write internally (Daniel, Sam)
> >> v4:
> >>* move dma_buf_map changes into separate patch (Daniel)
> >>* TODO list: comment on fbdev updates (Daniel)
> >>
> >> Signed-off-by: Thomas Zimmermann
> >> Tested-by: Sam Ravnborg
> >> ---
> >> Documentation/gpu/todo.rst| 19 ++-
> >> drivers/gpu/drm/bochs/bochs_kms.c | 1 -
> >> drivers/gpu/drm/drm_fb_helper.c | 227 --
> >> include/drm/drm_mode_config.h | 12 --
> >> 4 files changed, 230 insertions(+), 29 deletions(-)
> >>
> >> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> >> index 7e6fc3c04add..638b7f704339 100644
> >> --- a/Documentation/gpu/todo.rst
> >> +++ b/Documentation/gpu/todo.rst
> >> @@ -197,13 +197,28 @@ Convert drivers to use drm_fbdev_generic_setup()
> >>
> >>
> >> Most drivers can use drm_fbdev_generic_setup(). Driver have to implement
> >> -atomic modesetting and GEM vmap support. Current generic fbdev emulation
> >> -expects the framebuffer in system memory (or system-like memory).
> >> +atomic modesetting and GEM vmap support. Historically, generic fbdev
> >> emulation
> >> +expected the framebuffer in system memory or system-like memory. By
> >> employing
> >> +struct dma_buf_map, drivers with frambuffers in I/O memory can be
> >> supported
> >> +as well.
> >>
> >> Contact: Maintainer of the driver you plan to convert
> >>
> >> Level: Intermediate
> >>
> >> +Reimplement functions in drm_fbdev_fb_ops without fbdev
> >> +---
> >> +
> >> +A number of callback functions in drm_fbdev_fb_ops could benefit from
> >> +being rewritten without dependencies on the fbdev module. Some of the
> >> +helpers could further benefit from using struct dma_buf_map instead of
> >> +raw pointers.
> >> +
> >> +Contact: Thomas Zimmermann , Daniel Vetter
> >> +
> >> +Level: Advanced
> >> +
> >> +
> >> drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup
> >> -
> >>
> >> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
> >> b/drivers/gpu/drm/bochs/bochs_kms.c
> >> index 13d0d04c4457..853081d186d5 100644
> >> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> >> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> >> @@ -151,7 +151,6 @@ int bochs_kms_init(struct bochs_device *bochs)
> >>bochs->dev->mode_config.preferred_depth = 24;
> >>bochs->dev->mode_config.prefer_shadow = 0;
> >>bochs->dev->mode_config.prefer_shadow_fbdev = 1;
> >> - bochs->dev->mode_config.fbdev_use_iomem = true;
> >>bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
> >>
> >>bochs->dev->mode_config.funcs = &bochs_mode_funcs;
> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> >> b/drivers/gpu/drm/drm_fb_helper.c
> >> index 6212cd7cde1d..1d3180841778 100644
> >> --- a/drivers/gpu/drm/drm_fb_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >> @@ -372,24 +372,22 @@ static void drm_fb_helper_resume_worker(struct
> >> work_struct *work)
> >> }
> >>
> >> static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
> >> -struct drm_clip_rect *clip)
> >> +struct drm_clip_rect *clip,
> >> +struct dma_buf_map *dst)
> >> {
> >>struct drm_framebuffer *fb = fb_helper->fb;
> >>unsigned int cpp = fb->format->cpp[0];
> >>size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> >>void *src = fb_helper->fbdev->screen_buffer + offset;
> >> - void *dst = fb_helper->buffer->map.vaddr + offset;
> >>size_t len = (clip->x2 - clip->x1) * cpp;
> >>unsigned int y;
> >>
> >> - for (y = clip->y1; y < clip->y2; y++) {
> >> - if (!fb_helper->dev->mode_config.fbdev_use_iomem)
> >> -
Re: [PATCH v2 1/8] evtchn: avoid race in get_xen_consumer()
On 22.10.2020 10:29, Roger Pau Monné wrote:
> On Thu, Oct 22, 2020 at 10:15:45AM +0200, Jan Beulich wrote:
>> On 22.10.2020 10:11, Roger Pau Monné wrote:
>>> On Thu, Oct 22, 2020 at 09:33:27AM +0200, Jan Beulich wrote:
On 21.10.2020 17:46, Roger Pau Monné wrote:
> On Tue, Oct 20, 2020 at 04:08:13PM +0200, Jan Beulich wrote:
>> @@ -80,8 +81,9 @@ static uint8_t get_xen_consumer(xen_even
>>
>> for ( i = 0; i < ARRAY_SIZE(xen_consumers); i++ )
>> {
>> +/* Use cmpxchgptr() in lieu of a global lock. */
>> if ( xen_consumers[i] == NULL )
>> -xen_consumers[i] = fn;
>> +cmpxchgptr(&xen_consumers[i], NULL, fn);
>> if ( xen_consumers[i] == fn )
>> break;
>
> I think you could join it as:
>
> if ( !xen_consumers[i] &&
> !cmpxchgptr(&xen_consumers[i], NULL, fn) )
> break;
>
> As cmpxchgptr will return the previous value of &xen_consumers[i]?
But then you also have to check whether the returned value is
fn (or retain the 2nd if()).
>>>
>>> __cmpxchg comment says that success of the operation is indicated when
>>> the returned value equals the old value, so it's my understanding that
>>> cmpxchgptr returning NULL would mean the exchange has succeed and that
>>> xen_consumers[i] == fn?
>>
>> Correct. But if xen_consumers[i] == fn before the call, the return
>> value will be fn. The cmpxchg() wasn't "successful" in this case
>> (it didn't update anything), but the state of the array slot is what
>> we want.
>
> Oh, I get it now. You don't want the same fn populating more than one
> slot.
FAOD it's not just "want", it's a strict requirement.
> I assume the reads of xen_consumers are not using ACCESS_ONCE or
> read_atomic because we rely on the compiler performing such reads as
> single instructions?
Yes, as in so many other places in the code base.
Jan
Re: [PATCH] xen/arm: Remove EXPERT dependancy
Hi, On 22/10/2020 02:43, Elliott Mitchell wrote: Linux requires UEFI support to be enabled on ARM64 devices. While many ARM64 devices lack ACPI, the writing seems to be on the wall of UEFI/ACPI potentially taking over. Some common devices may need ACPI table support. Presently I think it is worth removing the dependancy on CONFIG_EXPERT. The idea behind EXPERT is to gate any feature that is not considered to be stable/complete enough to be used in production. I don't consider the ACPI complete because the parsing of the IORT (used to discover SMMU and GICv3 ITS) is not there yet. I vaguely remember some issues on system using SMMU (e.g. Thunder-X) because Dom0 will try to use the IOMMU and this would break PV drivers. Therefore I think we at least want to consider to hide SMMUs from dom0 before removing EXPERT. Ideally, I would also like the feature to be tested in Osstest. The good news is Xen Project already has systems (e.g. Thunder-X, Softiron) that can supported ACPI. So it should hopefully be a matter to tell them to boot with ACPI rather than DT. Cheers, -- Julien Grall
Re: [PATCH v5 08/10] drm/gem: Store client buffer mappings as struct dma_buf_map
Hi
On 22.10.20 10:49, Daniel Vetter wrote:
> On Tue, Oct 20, 2020 at 02:20:44PM +0200, Thomas Zimmermann wrote:
>> Kernel DRM clients now store their framebuffer address in an instance
>> of struct dma_buf_map. Depending on the buffer's location, the address
>> refers to system or I/O memory.
>>
>> Callers of drm_client_buffer_vmap() receive a copy of the value in
>> the call's supplied arguments. It can be accessed and modified with
>> dma_buf_map interfaces.
>>
>> Signed-off-by: Thomas Zimmermann
>> Reviewed-by: Daniel Vetter
>> Tested-by: Sam Ravnborg
>> ---
>> drivers/gpu/drm/drm_client.c| 34 +++--
>> drivers/gpu/drm/drm_fb_helper.c | 23 +-
>> include/drm/drm_client.h| 7 ---
>> 3 files changed, 38 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
>> index ac0082bed966..fe573acf1067 100644
>> --- a/drivers/gpu/drm/drm_client.c
>> +++ b/drivers/gpu/drm/drm_client.c
>> @@ -235,7 +235,7 @@ static void drm_client_buffer_delete(struct
>> drm_client_buffer *buffer)
>> {
>> struct drm_device *dev = buffer->client->dev;
>>
>> -drm_gem_vunmap(buffer->gem, buffer->vaddr);
>> +drm_gem_vunmap(buffer->gem, &buffer->map);
>>
>> if (buffer->gem)
>> drm_gem_object_put(buffer->gem);
>> @@ -291,25 +291,31 @@ drm_client_buffer_create(struct drm_client_dev
>> *client, u32 width, u32 height, u
>> /**
>> * drm_client_buffer_vmap - Map DRM client buffer into address space
>> * @buffer: DRM client buffer
>> + * @map_copy: Returns the mapped memory's address
>> *
>> * This function maps a client buffer into kernel address space. If the
>> - * buffer is already mapped, it returns the mapping's address.
>> + * buffer is already mapped, it returns the existing mapping's address.
>> *
>> * Client buffer mappings are not ref'counted. Each call to
>> * drm_client_buffer_vmap() should be followed by a call to
>> * drm_client_buffer_vunmap(); or the client buffer should be mapped
>> * throughout its lifetime.
>> *
>> + * The returned address is a copy of the internal value. In contrast to
>> + * other vmap interfaces, you don't need it for the client's vunmap
>> + * function. So you can modify it at will during blit and draw operations.
>> + *
>> * Returns:
>> - * The mapped memory's address
>> + * 0 on success, or a negative errno code otherwise.
>> */
>> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
>> +int
>> +drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map
>> *map_copy)
>> {
>> -struct dma_buf_map map;
>> +struct dma_buf_map *map = &buffer->map;
>> int ret;
>>
>> -if (buffer->vaddr)
>> -return buffer->vaddr;
>> +if (dma_buf_map_is_set(map))
>> +goto out;
>>
>> /*
>> * FIXME: The dependency on GEM here isn't required, we could
>> @@ -319,13 +325,14 @@ void *drm_client_buffer_vmap(struct drm_client_buffer
>> *buffer)
>> * fd_install step out of the driver backend hooks, to make that
>> * final step optional for internal users.
>> */
>> -ret = drm_gem_vmap(buffer->gem, &map);
>> +ret = drm_gem_vmap(buffer->gem, map);
>> if (ret)
>> -return ERR_PTR(ret);
>> +return ret;
>>
>> -buffer->vaddr = map.vaddr;
>> +out:
>> +*map_copy = *map;
>>
>> -return map.vaddr;
>> +return 0;
>> }
>> EXPORT_SYMBOL(drm_client_buffer_vmap);
>>
>> @@ -339,10 +346,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
>> */
>> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
>> {
>> -struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buffer->vaddr);
>> +struct dma_buf_map *map = &buffer->map;
>>
>> -drm_gem_vunmap(buffer->gem, &map);
>> -buffer->vaddr = NULL;
>> +drm_gem_vunmap(buffer->gem, map);
>> }
>> EXPORT_SYMBOL(drm_client_buffer_vunmap);
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index c2f72bb6afb1..6212cd7cde1d 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -378,7 +378,7 @@ static void drm_fb_helper_dirty_blit_real(struct
>> drm_fb_helper *fb_helper,
>> unsigned int cpp = fb->format->cpp[0];
>> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
>> void *src = fb_helper->fbdev->screen_buffer + offset;
>> -void *dst = fb_helper->buffer->vaddr + offset;
>> +void *dst = fb_helper->buffer->map.vaddr + offset;
>> size_t len = (clip->x2 - clip->x1) * cpp;
>> unsigned int y;
>>
>> @@ -400,7 +400,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
>> *work)
>> struct drm_clip_rect *clip = &helper->dirty_clip;
>> struct drm_clip_rect clip_copy;
>> unsigned long flags;
>> -void *vaddr;
>> +struct dma_buf_map map;
>> +int ret;
>>
>> spin_lock_irqsave(&helper->dirty_lock, f
Re: [PATCH v2 1/8] evtchn: avoid race in get_xen_consumer()
On Tue, Oct 20, 2020 at 04:08:13PM +0200, Jan Beulich wrote: > There's no global lock around the updating of this global piece of data. > Make use of cmpxchgptr() to avoid two entities racing with their > updates. > > While touching the functionality, mark xen_consumers[] read-mostly (or > else the if() condition could use the result of cmpxchgptr(), writing to > the slot unconditionally). > > The use of cmpxchgptr() here points out (by way of clang warning about > it) that its original use of const was slightly wrong. Adjust the > placement, or else undefined behavior of const qualifying a function > type will result. > > Signed-off-by: Jan Beulich Reviewed-by: Roger Pau Monné Thanks, Roger.
Re: [PATCH] x86/alternative: don't call text_poke() in lazy TLB mode
On 09.10.20 16:42, Juergen Gross wrote:
When running in lazy TLB mode the currently active page tables might
be the ones of a previous process, e.g. when running a kernel thread.
This can be problematic in case kernel code is being modified via
text_poke() in a kernel thread, and on another processor exit_mmap()
is active for the process which was running on the first cpu before
the kernel thread.
As text_poke() is using a temporary address space and the former
address space (obtained via cpu_tlbstate.loaded_mm) is restored
afterwards, there is a race possible in case the cpu on which
exit_mmap() is running wants to make sure there are no stale
references to that address space on any cpu active (this e.g. is
required when running as a Xen PV guest, where this problem has been
observed and analyzed).
In order to avoid that, drop off TLB lazy mode before switching to the
temporary address space.
Fixes: cefa929c034eb5d ("x86/mm: Introduce temporary mm structs")
Signed-off-by: Juergen Gross
Can anyone look at this, please? It is fixing a real problem which has
been seen several times.
Juergen
---
arch/x86/kernel/alternative.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index cdaab30880b9..cd6be6f143e8 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -807,6 +807,15 @@ static inline temp_mm_state_t use_temporary_mm(struct
mm_struct *mm)
temp_mm_state_t temp_state;
lockdep_assert_irqs_disabled();
+
+ /*
+* Make sure not to be in TLB lazy mode, as otherwise we'll end up
+* with a stale address space WITHOUT being in lazy mode after
+* restoring the previous mm.
+*/
+ if (this_cpu_read(cpu_tlbstate.is_lazy))
+ leave_mm(smp_processor_id());
+
temp_state.mm = this_cpu_read(cpu_tlbstate.loaded_mm);
switch_mm_irqs_off(NULL, mm, current);
Re: [PATCH v2 1/8] evtchn: avoid race in get_xen_consumer()
On Thu, Oct 22, 2020 at 10:56:15AM +0200, Jan Beulich wrote:
> On 22.10.2020 10:29, Roger Pau Monné wrote:
> > On Thu, Oct 22, 2020 at 10:15:45AM +0200, Jan Beulich wrote:
> >> On 22.10.2020 10:11, Roger Pau Monné wrote:
> >>> On Thu, Oct 22, 2020 at 09:33:27AM +0200, Jan Beulich wrote:
> On 21.10.2020 17:46, Roger Pau Monné wrote:
> > On Tue, Oct 20, 2020 at 04:08:13PM +0200, Jan Beulich wrote:
> >> @@ -80,8 +81,9 @@ static uint8_t get_xen_consumer(xen_even
> >>
> >> for ( i = 0; i < ARRAY_SIZE(xen_consumers); i++ )
> >> {
> >> +/* Use cmpxchgptr() in lieu of a global lock. */
> >> if ( xen_consumers[i] == NULL )
> >> -xen_consumers[i] = fn;
> >> +cmpxchgptr(&xen_consumers[i], NULL, fn);
> >> if ( xen_consumers[i] == fn )
> >> break;
> >
> > I think you could join it as:
> >
> > if ( !xen_consumers[i] &&
> > !cmpxchgptr(&xen_consumers[i], NULL, fn) )
> > break;
> >
> > As cmpxchgptr will return the previous value of &xen_consumers[i]?
>
> But then you also have to check whether the returned value is
> fn (or retain the 2nd if()).
> >>>
> >>> __cmpxchg comment says that success of the operation is indicated when
> >>> the returned value equals the old value, so it's my understanding that
> >>> cmpxchgptr returning NULL would mean the exchange has succeed and that
> >>> xen_consumers[i] == fn?
> >>
> >> Correct. But if xen_consumers[i] == fn before the call, the return
> >> value will be fn. The cmpxchg() wasn't "successful" in this case
> >> (it didn't update anything), but the state of the array slot is what
> >> we want.
> >
> > Oh, I get it now. You don't want the same fn populating more than one
> > slot.
>
> FAOD it's not just "want", it's a strict requirement.
I wouldn't mind having a comment to that effect in the function, but I
won't insist.
Thanks, Roger.
[PATCH v2 4/5] xen/events: unmask a fifo event channel only if it was masked
Unmasking an event channel with fifo events channels being used can
require a hypercall to be made, so try to avoid that by checking
whether the event channel was really masked.
Suggested-by: Jan Beulich
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
---
V2:
- move test for already unmasked into loop (Jan Beulich)
---
drivers/xen/events/events_fifo.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/xen/events/events_fifo.c b/drivers/xen/events/events_fifo.c
index 243e7b6d7b96..b234f1766810 100644
--- a/drivers/xen/events/events_fifo.c
+++ b/drivers/xen/events/events_fifo.c
@@ -237,6 +237,9 @@ static bool clear_masked_cond(volatile event_word_t *word)
w = *word;
do {
+ if (!(w & (1 << EVTCHN_FIFO_MASKED)))
+ return true;
+
if (w & (1 << EVTCHN_FIFO_PENDING))
return false;
--
2.26.2
[PATCH v2 3/5] xen/events: only register debug interrupt for 2-level events
xen_debug_interrupt() is specific to 2-level event handling. So don't
register it with fifo event handling being active.
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
---
V2:
- rename fifo_events variable to xen_fifo_events (Jan Beulich)
---
arch/x86/xen/smp.c | 19 +++
arch/x86/xen/xen-ops.h | 2 ++
drivers/xen/events/events_base.c | 10 ++
3 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 2097fa0ebdb5..c1b2f764b29a 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -88,14 +88,17 @@ int xen_smp_intr_init(unsigned int cpu)
per_cpu(xen_callfunc_irq, cpu).irq = rc;
per_cpu(xen_callfunc_irq, cpu).name = callfunc_name;
- debug_name = kasprintf(GFP_KERNEL, "debug%d", cpu);
- rc = bind_virq_to_irqhandler(VIRQ_DEBUG, cpu, xen_debug_interrupt,
-IRQF_PERCPU | IRQF_NOBALANCING,
-debug_name, NULL);
- if (rc < 0)
- goto fail;
- per_cpu(xen_debug_irq, cpu).irq = rc;
- per_cpu(xen_debug_irq, cpu).name = debug_name;
+ if (!xen_fifo_events) {
+ debug_name = kasprintf(GFP_KERNEL, "debug%d", cpu);
+ rc = bind_virq_to_irqhandler(VIRQ_DEBUG, cpu,
+xen_debug_interrupt,
+IRQF_PERCPU | IRQF_NOBALANCING,
+debug_name, NULL);
+ if (rc < 0)
+ goto fail;
+ per_cpu(xen_debug_irq, cpu).irq = rc;
+ per_cpu(xen_debug_irq, cpu).name = debug_name;
+ }
callfunc_name = kasprintf(GFP_KERNEL, "callfuncsingle%d", cpu);
rc = bind_ipi_to_irqhandler(XEN_CALL_FUNCTION_SINGLE_VECTOR,
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 45d556f71858..9546c3384c75 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -29,6 +29,8 @@ extern struct start_info *xen_start_info;
extern struct shared_info xen_dummy_shared_info;
extern struct shared_info *HYPERVISOR_shared_info;
+extern bool xen_fifo_events;
+
void xen_setup_mfn_list_list(void);
void xen_build_mfn_list_list(void);
void xen_setup_machphys_mapping(void);
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 1c25580c7691..6038c4c35db5 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -2050,8 +2050,8 @@ void xen_setup_callback_vector(void) {}
static inline void xen_alloc_callback_vector(void) {}
#endif
-static bool fifo_events = true;
-module_param(fifo_events, bool, 0);
+bool xen_fifo_events = true;
+module_param_named(fifo_events, xen_fifo_events, bool, 0);
static int xen_evtchn_cpu_prepare(unsigned int cpu)
{
@@ -2080,10 +2080,12 @@ void __init xen_init_IRQ(void)
int ret = -EINVAL;
evtchn_port_t evtchn;
- if (fifo_events)
+ if (xen_fifo_events)
ret = xen_evtchn_fifo_init();
- if (ret < 0)
+ if (ret < 0) {
xen_evtchn_2l_init();
+ xen_fifo_events = false;
+ }
xen_cpu_init_eoi(smp_processor_id());
--
2.26.2
[PATCH v2 0/5] xen: event handling cleanup
Do some cleanups in Xen event handling code. Changes in V2: - addressed comments Juergen Gross (5): xen: remove no longer used functions xen/events: make struct irq_info private to events_base.c xen/events: only register debug interrupt for 2-level events xen/events: unmask a fifo event channel only if it was masked Documentation: add xen.fifo_events kernel parameter description .../admin-guide/kernel-parameters.txt | 7 ++ arch/x86/xen/smp.c| 19 ++-- arch/x86/xen/xen-ops.h| 2 + drivers/xen/events/events_2l.c| 7 +- drivers/xen/events/events_base.c | 94 +-- drivers/xen/events/events_fifo.c | 9 +- drivers/xen/events/events_internal.h | 70 ++ include/xen/events.h | 8 -- 8 files changed, 102 insertions(+), 114 deletions(-) -- 2.26.2
[PATCH v2 2/5] xen/events: make struct irq_info private to events_base.c
The struct irq_info of Xen's event handling is used only for two
evtchn_ops functions outside of events_base.c. Those two functions
can easily be switched to avoid that usage.
This allows to make struct irq_info and its related access functions
private to events_base.c.
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
---
drivers/xen/events/events_2l.c | 7 +--
drivers/xen/events/events_base.c | 63 ++---
drivers/xen/events/events_fifo.c | 6 +--
drivers/xen/events/events_internal.h | 70
4 files changed, 73 insertions(+), 73 deletions(-)
diff --git a/drivers/xen/events/events_2l.c b/drivers/xen/events/events_2l.c
index fe5ad0e89cd8..da87f3a1e351 100644
--- a/drivers/xen/events/events_2l.c
+++ b/drivers/xen/events/events_2l.c
@@ -47,10 +47,11 @@ static unsigned evtchn_2l_max_channels(void)
return EVTCHN_2L_NR_CHANNELS;
}
-static void evtchn_2l_bind_to_cpu(struct irq_info *info, unsigned cpu)
+static void evtchn_2l_bind_to_cpu(evtchn_port_t evtchn, unsigned int cpu,
+ unsigned int old_cpu)
{
- clear_bit(info->evtchn, BM(per_cpu(cpu_evtchn_mask, info->cpu)));
- set_bit(info->evtchn, BM(per_cpu(cpu_evtchn_mask, cpu)));
+ clear_bit(evtchn, BM(per_cpu(cpu_evtchn_mask, old_cpu)));
+ set_bit(evtchn, BM(per_cpu(cpu_evtchn_mask, cpu)));
}
static void evtchn_2l_clear_pending(evtchn_port_t port)
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 436682db41c5..1c25580c7691 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -70,6 +70,57 @@
#undef MODULE_PARAM_PREFIX
#define MODULE_PARAM_PREFIX "xen."
+/* Interrupt types. */
+enum xen_irq_type {
+ IRQT_UNBOUND = 0,
+ IRQT_PIRQ,
+ IRQT_VIRQ,
+ IRQT_IPI,
+ IRQT_EVTCHN
+};
+
+/*
+ * Packed IRQ information:
+ * type - enum xen_irq_type
+ * event channel - irq->event channel mapping
+ * cpu - cpu this event channel is bound to
+ * index - type-specific information:
+ *PIRQ - vector, with MSB being "needs EIO", or physical IRQ of the HVM
+ * guest, or GSI (real passthrough IRQ) of the device.
+ *VIRQ - virq number
+ *IPI - IPI vector
+ *EVTCHN -
+ */
+struct irq_info {
+ struct list_head list;
+ struct list_head eoi_list;
+ short refcnt;
+ short spurious_cnt;
+ enum xen_irq_type type; /* type */
+ unsigned irq;
+ evtchn_port_t evtchn; /* event channel */
+ unsigned short cpu; /* cpu bound */
+ unsigned short eoi_cpu; /* EOI must happen on this cpu-1 */
+ unsigned int irq_epoch; /* If eoi_cpu valid: irq_epoch of event */
+ u64 eoi_time; /* Time in jiffies when to EOI. */
+
+ union {
+ unsigned short virq;
+ enum ipi_vector ipi;
+ struct {
+ unsigned short pirq;
+ unsigned short gsi;
+ unsigned char vector;
+ unsigned char flags;
+ uint16_t domid;
+ } pirq;
+ } u;
+};
+
+#define PIRQ_NEEDS_EOI (1 << 0)
+#define PIRQ_SHAREABLE (1 << 1)
+#define PIRQ_MSI_GROUP (1 << 2)
+
static uint __read_mostly event_loop_timeout = 2;
module_param(event_loop_timeout, uint, 0644);
@@ -110,7 +161,7 @@ static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0
... NR_VIRQS-1] = -1};
/* IRQ <-> IPI mapping */
static DEFINE_PER_CPU(int [XEN_NR_IPIS], ipi_to_irq) = {[0 ... XEN_NR_IPIS-1]
= -1};
-int **evtchn_to_irq;
+static int **evtchn_to_irq;
#ifdef CONFIG_X86
static unsigned long *pirq_eoi_map;
#endif
@@ -190,7 +241,7 @@ int get_evtchn_to_irq(evtchn_port_t evtchn)
}
/* Get info for IRQ */
-struct irq_info *info_for_irq(unsigned irq)
+static struct irq_info *info_for_irq(unsigned irq)
{
if (irq < nr_legacy_irqs())
return legacy_info_ptrs[irq];
@@ -228,7 +279,7 @@ static int xen_irq_info_common_setup(struct irq_info *info,
irq_clear_status_flags(irq, IRQ_NOREQUEST|IRQ_NOAUTOEN);
- return xen_evtchn_port_setup(info);
+ return xen_evtchn_port_setup(evtchn);
}
static int xen_irq_info_evtchn_setup(unsigned irq,
@@ -351,7 +402,7 @@ static enum xen_irq_type type_from_irq(unsigned irq)
return info_for_irq(irq)->type;
}
-unsigned cpu_from_irq(unsigned irq)
+static unsigned cpu_from_irq(unsigned irq)
{
return info_for_irq(irq)->cpu;
}
@@ -391,7 +442,7 @@ static void bind_evtchn_to_cpu(evtchn_port_t evtchn,
unsigned int cpu)
#ifdef CONFIG_SMP
cpumask_copy(irq_get_affinity_mask(irq), cpumask_of(cpu));
#endif
- xen_evtchn_port_bind_to_cpu(info, cpu);
+ xen_evtchn_port_bind_to_cpu(evtchn, cpu, info->cpu);
info->cpu = cpu;
}
@@ -745,7 +796,7 @@ static unsigned int __startup_pirq(unsigned int irq)
info->evtchn = evtchn;
bind_evtchn_to_cpu(evtchn,
[PATCH v2 1/5] xen: remove no longer used functions
With the switch to the lateeoi model for interdomain event channels
some functions are no longer in use. Remove them.
Suggested-by: Jan Beulich
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
---
drivers/xen/events/events_base.c | 21 -
include/xen/events.h | 8
2 files changed, 29 deletions(-)
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index cc317739e786..436682db41c5 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1145,14 +1145,6 @@ static int bind_interdomain_evtchn_to_irq_chip(unsigned
int remote_domain,
chip);
}
-int bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
- evtchn_port_t remote_port)
-{
- return bind_interdomain_evtchn_to_irq_chip(remote_domain, remote_port,
- &xen_dynamic_chip);
-}
-EXPORT_SYMBOL_GPL(bind_interdomain_evtchn_to_irq);
-
int bind_interdomain_evtchn_to_irq_lateeoi(unsigned int remote_domain,
evtchn_port_t remote_port)
{
@@ -1320,19 +1312,6 @@ static int bind_interdomain_evtchn_to_irqhandler_chip(
return irq;
}
-int bind_interdomain_evtchn_to_irqhandler(unsigned int remote_domain,
- evtchn_port_t remote_port,
- irq_handler_t handler,
- unsigned long irqflags,
- const char *devname,
- void *dev_id)
-{
- return bind_interdomain_evtchn_to_irqhandler_chip(remote_domain,
- remote_port, handler, irqflags, devname,
- dev_id, &xen_dynamic_chip);
-}
-EXPORT_SYMBOL_GPL(bind_interdomain_evtchn_to_irqhandler);
-
int bind_interdomain_evtchn_to_irqhandler_lateeoi(unsigned int remote_domain,
evtchn_port_t remote_port,
irq_handler_t handler,
diff --git a/include/xen/events.h b/include/xen/events.h
index 3b8155c2ea03..8ec418e30c7f 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -35,16 +35,8 @@ int bind_ipi_to_irqhandler(enum ipi_vector ipi,
unsigned long irqflags,
const char *devname,
void *dev_id);
-int bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
- evtchn_port_t remote_port);
int bind_interdomain_evtchn_to_irq_lateeoi(unsigned int remote_domain,
evtchn_port_t remote_port);
-int bind_interdomain_evtchn_to_irqhandler(unsigned int remote_domain,
- evtchn_port_t remote_port,
- irq_handler_t handler,
- unsigned long irqflags,
- const char *devname,
- void *dev_id);
int bind_interdomain_evtchn_to_irqhandler_lateeoi(unsigned int remote_domain,
evtchn_port_t remote_port,
irq_handler_t handler,
--
2.26.2
[PATCH v2 5/5] Documentation: add xen.fifo_events kernel parameter description
The kernel boot parameter xen.fifo_events isn't listed in Documentation/admin-guide/kernel-parameters.txt. Add it. Signed-off-by: Juergen Gross Reviewed-by: Jan Beulich --- Documentation/admin-guide/kernel-parameters.txt | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 02d4adbf98d2..526d65d8573a 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -5978,6 +5978,13 @@ After which time (jiffies) the event handling loop should start to delay EOI handling. Default is 2. + xen.fifo_events=[XEN] + Boolean parameter to disable using fifo event handling + even if available. Normally fifo event handling is + preferred over the 2-level event handling, as it is + fairer and the number of possible event channels is + much higher. Default is on (use fifo events). + nopv= [X86,XEN,KVM,HYPER_V,VMWARE] Disables the PV optimizations forcing the guest to run as generic guest with no PV drivers. Currently support -- 2.26.2
[xen-4.13-testing test] 156054: tolerable FAIL - PUSHED
flight 156054 xen-4.13-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/156054/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 155377 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 155377 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 155377 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 155377 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 155377 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 155377 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 155377 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 155377 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 155377 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 155377 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 155377 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: xen dc38c1103cfdc643860e10c1b9e925dac83332dc baseline version: xen 8e7e5857a203c9d9df7733fd68768555c7e76839 Last test of basis 155377 2020-10-03 13:48:36 Z 18 days Testing same since 156030 2020-10-20 13:06:12 Z1 days2 attempts People who touched revisions under test: Andrew Cooper Chen Yu George Dunlap Hongyan Xia Igor Druzhinin Jan Beulich Julien Grall Rafael J. Wysocki Roger Pau Monné Wei Liu jobs: build-amd64-xsm pass build-arm64-xsm
Re: [PATCH v5 08/10] drm/gem: Store client buffer mappings as struct dma_buf_map
On Thu, Oct 22, 2020 at 11:18 AM Thomas Zimmermann wrote:
>
> Hi
>
> On 22.10.20 10:49, Daniel Vetter wrote:
> > On Tue, Oct 20, 2020 at 02:20:44PM +0200, Thomas Zimmermann wrote:
> >> Kernel DRM clients now store their framebuffer address in an instance
> >> of struct dma_buf_map. Depending on the buffer's location, the address
> >> refers to system or I/O memory.
> >>
> >> Callers of drm_client_buffer_vmap() receive a copy of the value in
> >> the call's supplied arguments. It can be accessed and modified with
> >> dma_buf_map interfaces.
> >>
> >> Signed-off-by: Thomas Zimmermann
> >> Reviewed-by: Daniel Vetter
> >> Tested-by: Sam Ravnborg
> >> ---
> >> drivers/gpu/drm/drm_client.c| 34 +++--
> >> drivers/gpu/drm/drm_fb_helper.c | 23 +-
> >> include/drm/drm_client.h| 7 ---
> >> 3 files changed, 38 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> >> index ac0082bed966..fe573acf1067 100644
> >> --- a/drivers/gpu/drm/drm_client.c
> >> +++ b/drivers/gpu/drm/drm_client.c
> >> @@ -235,7 +235,7 @@ static void drm_client_buffer_delete(struct
> >> drm_client_buffer *buffer)
> >> {
> >> struct drm_device *dev = buffer->client->dev;
> >>
> >> -drm_gem_vunmap(buffer->gem, buffer->vaddr);
> >> +drm_gem_vunmap(buffer->gem, &buffer->map);
> >>
> >> if (buffer->gem)
> >> drm_gem_object_put(buffer->gem);
> >> @@ -291,25 +291,31 @@ drm_client_buffer_create(struct drm_client_dev
> >> *client, u32 width, u32 height, u
> >> /**
> >> * drm_client_buffer_vmap - Map DRM client buffer into address space
> >> * @buffer: DRM client buffer
> >> + * @map_copy: Returns the mapped memory's address
> >> *
> >> * This function maps a client buffer into kernel address space. If the
> >> - * buffer is already mapped, it returns the mapping's address.
> >> + * buffer is already mapped, it returns the existing mapping's address.
> >> *
> >> * Client buffer mappings are not ref'counted. Each call to
> >> * drm_client_buffer_vmap() should be followed by a call to
> >> * drm_client_buffer_vunmap(); or the client buffer should be mapped
> >> * throughout its lifetime.
> >> *
> >> + * The returned address is a copy of the internal value. In contrast to
> >> + * other vmap interfaces, you don't need it for the client's vunmap
> >> + * function. So you can modify it at will during blit and draw operations.
> >> + *
> >> * Returns:
> >> - * The mapped memory's address
> >> + * 0 on success, or a negative errno code otherwise.
> >> */
> >> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
> >> +int
> >> +drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct
> >> dma_buf_map *map_copy)
> >> {
> >> -struct dma_buf_map map;
> >> +struct dma_buf_map *map = &buffer->map;
> >> int ret;
> >>
> >> -if (buffer->vaddr)
> >> -return buffer->vaddr;
> >> +if (dma_buf_map_is_set(map))
> >> +goto out;
> >>
> >> /*
> >> * FIXME: The dependency on GEM here isn't required, we could
> >> @@ -319,13 +325,14 @@ void *drm_client_buffer_vmap(struct
> >> drm_client_buffer *buffer)
> >> * fd_install step out of the driver backend hooks, to make that
> >> * final step optional for internal users.
> >> */
> >> -ret = drm_gem_vmap(buffer->gem, &map);
> >> +ret = drm_gem_vmap(buffer->gem, map);
> >> if (ret)
> >> -return ERR_PTR(ret);
> >> +return ret;
> >>
> >> -buffer->vaddr = map.vaddr;
> >> +out:
> >> +*map_copy = *map;
> >>
> >> -return map.vaddr;
> >> +return 0;
> >> }
> >> EXPORT_SYMBOL(drm_client_buffer_vmap);
> >>
> >> @@ -339,10 +346,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
> >> */
> >> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
> >> {
> >> -struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buffer->vaddr);
> >> +struct dma_buf_map *map = &buffer->map;
> >>
> >> -drm_gem_vunmap(buffer->gem, &map);
> >> -buffer->vaddr = NULL;
> >> +drm_gem_vunmap(buffer->gem, map);
> >> }
> >> EXPORT_SYMBOL(drm_client_buffer_vunmap);
> >>
> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> >> b/drivers/gpu/drm/drm_fb_helper.c
> >> index c2f72bb6afb1..6212cd7cde1d 100644
> >> --- a/drivers/gpu/drm/drm_fb_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >> @@ -378,7 +378,7 @@ static void drm_fb_helper_dirty_blit_real(struct
> >> drm_fb_helper *fb_helper,
> >> unsigned int cpp = fb->format->cpp[0];
> >> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> >> void *src = fb_helper->fbdev->screen_buffer + offset;
> >> -void *dst = fb_helper->buffer->vaddr + offset;
> >> +void *dst = fb_helper->buffer->map.vaddr + offset;
> >> size_t len = (clip->x2 - clip->x1) * cpp;
> >> unsigned int y;
> >>
> >> @@ -400,7 +400,8 @@ static void drm_fb_helper_dir
Re: [PATCH v2 3/8] evtchn: rename and adjust guest_enabled_event()
On Tue, Oct 20, 2020 at 04:09:16PM +0200, Jan Beulich wrote:
> The function isn't about an "event" in general, but about a vIRQ. The
> function also failed to honor global vIRQ-s, instead assuming the caller
> would pass vCPU 0 in such a case.
>
> While at it also adjust the
> - types the function uses,
> - single user to make use of domain_vcpu().
>
> Signed-off-by: Jan Beulich
Reviewed-by: Roger Pau Monné
> ---
> v2: New.
>
> --- a/xen/arch/x86/cpu/mcheck/vmce.h
> +++ b/xen/arch/x86/cpu/mcheck/vmce.h
> @@ -5,9 +5,9 @@
>
> int vmce_init(struct cpuinfo_x86 *c);
>
> -#define dom0_vmce_enabled() (hardware_domain && hardware_domain->max_vcpus \
> -&& hardware_domain->vcpu[0] \
> -&& guest_enabled_event(hardware_domain->vcpu[0], VIRQ_MCA))
> +#define dom0_vmce_enabled() \
> +(hardware_domain && \
> + evtchn_virq_enabled(domain_vcpu(hardware_domain, 0), VIRQ_MCA))
>
> int unmmap_broken_page(struct domain *d, mfn_t mfn, unsigned long gfn);
>
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -778,9 +778,15 @@ out:
> return ret;
> }
>
> -int guest_enabled_event(struct vcpu *v, uint32_t virq)
> +bool evtchn_virq_enabled(const struct vcpu *v, unsigned int virq)
> {
> -return ((v != NULL) && (v->virq_to_evtchn[virq] != 0));
> +if ( !v )
Not sure it's worth adding a check that virq < NR_VIRQS here just to
be on the safe side...
> +return false;
> +
> +if ( virq_is_global(virq) && v->vcpu_id )
...as virq_is_global has an ASSERT to that extend (but that would be a
nop on release builds).
Thanks, Roger.
Re: [PATCH v2 3/5] xen/events: only register debug interrupt for 2-level events
On 22.10.2020 11:49, Juergen Gross wrote:
> @@ -2080,10 +2080,12 @@ void __init xen_init_IRQ(void)
> int ret = -EINVAL;
> evtchn_port_t evtchn;
>
> - if (fifo_events)
> + if (xen_fifo_events)
> ret = xen_evtchn_fifo_init();
> - if (ret < 0)
> + if (ret < 0) {
> xen_evtchn_2l_init();
> + xen_fifo_events = false;
> + }
Another note: While it may not matter right here, maybe better
first set the variable and the call the function?
Jan
Re: [PATCH] x86/alternative: don't call text_poke() in lazy TLB mode
On Thu, Oct 22, 2020 at 11:24:39AM +0200, Jürgen Groß wrote:
> On 09.10.20 16:42, Juergen Gross wrote:
> > When running in lazy TLB mode the currently active page tables might
> > be the ones of a previous process, e.g. when running a kernel thread.
> >
> > This can be problematic in case kernel code is being modified via
> > text_poke() in a kernel thread, and on another processor exit_mmap()
> > is active for the process which was running on the first cpu before
> > the kernel thread.
> >
> > As text_poke() is using a temporary address space and the former
> > address space (obtained via cpu_tlbstate.loaded_mm) is restored
> > afterwards, there is a race possible in case the cpu on which
> > exit_mmap() is running wants to make sure there are no stale
> > references to that address space on any cpu active (this e.g. is
> > required when running as a Xen PV guest, where this problem has been
> > observed and analyzed).
> >
> > In order to avoid that, drop off TLB lazy mode before switching to the
> > temporary address space.
> >
> > Fixes: cefa929c034eb5d ("x86/mm: Introduce temporary mm structs")
> > Signed-off-by: Juergen Gross
>
> Can anyone look at this, please? It is fixing a real problem which has
> been seen several times.
As it happens I picked it up yesterday, just pushed it out for you.
Thanks!
Re: [PATCH] x86/alternative: don't call text_poke() in lazy TLB mode
On 22.10.20 12:45, Peter Zijlstra wrote:
On Thu, Oct 22, 2020 at 11:24:39AM +0200, Jürgen Groß wrote:
On 09.10.20 16:42, Juergen Gross wrote:
When running in lazy TLB mode the currently active page tables might
be the ones of a previous process, e.g. when running a kernel thread.
This can be problematic in case kernel code is being modified via
text_poke() in a kernel thread, and on another processor exit_mmap()
is active for the process which was running on the first cpu before
the kernel thread.
As text_poke() is using a temporary address space and the former
address space (obtained via cpu_tlbstate.loaded_mm) is restored
afterwards, there is a race possible in case the cpu on which
exit_mmap() is running wants to make sure there are no stale
references to that address space on any cpu active (this e.g. is
required when running as a Xen PV guest, where this problem has been
observed and analyzed).
In order to avoid that, drop off TLB lazy mode before switching to the
temporary address space.
Fixes: cefa929c034eb5d ("x86/mm: Introduce temporary mm structs")
Signed-off-by: Juergen Gross
Can anyone look at this, please? It is fixing a real problem which has
been seen several times.
As it happens I picked it up yesterday, just pushed it out for you.
Thank you very much!
Juergen
Re: [PATCH v2 4/8] evtchn: let evtchn_set_priority() acquire the per-channel lock
On Tue, Oct 20, 2020 at 04:09:41PM +0200, Jan Beulich wrote: > Some lock wants to be held to make sure the port doesn't change state, > but there's no point holding the per-domain event lock here. Switch to > using the finer grained per-channel lock instead. While true that's a fine grained lock, it also disables interrupts, which the global event_lock didn't. > FAOD this doesn't guarantee anything towards in particular > evtchn_fifo_set_pending(), as for interdomain channels that function > would be called with the remote side's per-channel lock held. > > Signed-off-by: Jan Beulich Reviewed-by: Roger Pau Monné Thanks, Roger.
Re: XSM and the idle domain
(also replying to others in this thread.)
On Wed, 2020-10-21 at 12:21 -0400, Jason Andryuk wrote:
> On Wed, Oct 21, 2020 at 10:35 AM Hongyan Xia wrote:
> >
> > Hi,
>
> ...
> >
> > The first question came up during ongoing work in LiveUpdate. After
> > an
> > LU, the next Xen needs to restore all domains. To do that, some
> > hypercalls need to be issued from the idle domain context and
> > apparently XSM does not like it. We need to introduce hacks in the
> > dummy module to leave the idle domain alone.
>
> Is this modifying xsm_default_action() to add an is_idle_domain()
> check which always succeeds?
Yes. We had to do exactly that to avoid LU actions being denied by XSM.
> > Our work is not compiled
> > with CONFIG_XSM at all, but with CONFIG_XSM, are we able to enforce
> > security policies against the idle domain?
>
> It's not clear to me if you want to use CONFIG_XSM, or just don't
> want
> to break it.
We don't (and won't) enable XSM in our build, but still we need a hack
to work around it, so I am just curious about what happens when people
use both LU and XSM at the same time.
> > Of course, without any LU
> > work this does not make any difference because the idle domain does
> > not
> > do any useful work to be restricted anyway.
>
> I think this last sentence is the main point. It's always been
> labeled xen_t, but since it doesn't go through any of the hook
> points,
> it hasn't needed any restrictions. Actually, reviewing the Flask
> policy there is:
> # Domain destruction can result in some access checks for actions
> performed by
> # the hypervisor. These should always be allowed.
> allow xen_t resource_type : resource { remove_irq remove_ioport
> remove_iomem };
>
> > Also, should idle domain be restricted? IMO the idle domain is Xen
> > itself which mostly bootstraps the system and performs limited work
> > when switched to, and is not something a user (either dom0 or domU)
> > directly interacts with. I doubt XSM was designed to include the
> > idle
> > domain (although there is an ID allocated for it in the code), so I
> > would say just exclude idle in all security policy checks.
>
> I think it makes sense to label xen_t, even if it doesn't do
> anything.
> As you say, it is a distinct entity from dom0 and domU. Yes, it can
> circumvent the policy, but it's not actively hurting anything. And
> it
> can be good to catch when it does start doing something, as you
> found.
>
> Might it make sense to create a LU domain instead of using the idle
> domain for Live Update? Another approach could be to run the
> idle_domain as "dom0" during Live Update, and then transition to the
> regular idle_domain when it completes? You are re-creating dom0, but
> you could flip is_privileged on during live update and then remove it
> once complete.
Actually I think your suggestion and what Daniel suggested make sense.
We could just have a domLU that does all the restore work which has its
own security policies. That sounds like a clean solution to me.
However, one top priority of LU is to minimise the down time so that
domains won't feel a thing and every millisecond counts. I don't know
how much overhead this adds (maybe negligible if we just let domLU sit
in idle domain's page tables so switching and passing the LU save
stream to it is painless), but is something we need to keep in mind.
But this still sidesteps the question of whether the idle domain should
be subject to security policies. From another reply it sounds like the
idle domain should not be exempt from XSM. Although, to me restrictions
on idle domain are more like a debugging feature than a security
policy, since it prevents, e.g., accidentally issuing hypercalls from
it, but if the idle domain really wants to do something then there is
nothing to stop it. This is different from enforcing policies on a real
domain which guarantees things won't happen and the domain simply has
no mechanism to circumvent it (hopefully).
My experience with XSM is only the idle domain hack for LU so what I
said about it here may not make sense.
Hongyan
Re: XSM and the idle domain
On 21/10/2020 15:34, Hongyan Xia wrote: > The first question came up during ongoing work in LiveUpdate. After an > LU, the next Xen needs to restore all domains. To do that, some > hypercalls need to be issued from the idle domain context and > apparently XSM does not like it. There is no such thing as issuing hypercalls from the idle domain (context or otherwise), because the idle domain does not have enough associated guest state for anything to make the requisite SYSCALL/INT80/VMCALL/VMMCALL invocation. I presume from this comment that what you mean is that you're calling the plain hypercall functions, context checks and everything, from the idle context? If so, this is buggy for more reasons than just XSM objecting to its calling context, and that XSM is merely the first thing to explode. Therefore, I don't think modifications to XSM are applicable to solving the problem. (Of course, this is all speculation because there's no concrete implementation to look at.) ~Andrew
Re: XSM and the idle domain
On Thu, 22 Oct 2020 04:13:53 -0400 Jan Beulich wrote > On 22.10.2020 03:23, Daniel P. Smith wrote: > > On 10/21/20 10:34 AM, Hongyan Xia wrote: > >> Also, should idle domain be restricted? IMO the idle domain is Xen > >> itself which mostly bootstraps the system and performs limited work > >> when switched to, and is not something a user (either dom0 or domU) > >> directly interacts with. I doubt XSM was designed to include the idle > >> domain (although there is an ID allocated for it in the code), so I > >> would say just exclude idle in all security policy checks. > > > > The idle domain is a limited, internal construct within the hypervisor > > and should be constrained as part of the hypervisor, which is why its > > domain id gets labeled with the same label as the hypervisor. For this > > reason I would wholeheartedly disagree with exempting the idle domain id > > from XSM hooks as that would effectively be saying the core hypervisor > > should not be constrained. The purpose of the XSM hooks is to control > > the flow of information in the system in a non-bypassable way. Codifying > > bypasses completely subverts the security model behind XSM for which the > > flask security server is dependent upon. > > While what you say may in general make sense, I have two questions: [Apologies for any poor formatting, responding from webmail interface ( ._.)] Hey Jan, these are very legitimate questions. > 1) When the idle domain is purely an internal construct of Xen, why > does it need limiting in any way? In fact, if restricting it in a > bad way, aren't you risking to prevent the system from functioning > correctly? Think in terms of least privilege, do you want the idle domain and by extension the hypervisor to have the additional privilege of imposing state on to the system as opposed to processing the state changes. I am not saying it is wrong technical approach (though I do believe at a minimum the implementation approach is flawed), I am just asking is it wise from a privilege delegation aspect of whether it could be done differently from a technical stand point. The underlying concern here is once you grant the privilege the hypervisor will forever have the privilege which can be used for good (LU) and bad (corruption). Take for instance what is being attempted with DomB, in this approach the privilege to impose state (configure domains) is delegated to the Boot Domain but it is not delegated the privilege to create state (domain creation). As I mentioned before, this is what Jason was suggesting in having another domain type that is allowed to impose the state that is transitioned to from the idle domain to conduct the action. Whether or not the idle domain is allowed to make hypercalls is not necessarily a concern of the XSM hooks. If it is decided that this is the desired path, then what is of concern is that the corrective action does not weaken/break the hooks. If this ends up being the desired approach, then IMHO the correct action is to update the dummy policy, flask policy, and SILO (if it applies) to allow the privilege/access to occur versus putting bypasses into the security hooks. > 2) LU is merely restoring the prior state of the system. This prior > state was reached with security auditing as per the system's > policy at the time. Why should there be anything denind in the > process of re-establishing this same state? IOW can't XSM checking > be globally disabled until the system is ready be run normally > again? There is an assumption you made there that is being overlooked and that is you are assuming it is the same state. It is important to understand what assumptions are being made and when possible impose those assumptions through policy than with code. Not everyone will want to make the same assumptions and may want a better controlled path for that state to flow. No you don't want to globally disable the XSM checking as that means you have lost all control over the system where any and all policy violations could occur without any auditing. This would open a huge hole for a malicious actor to take advantage of for an attack against the system. In the end to reiterate, if this is decided to be the desired approach then IMHO the correct implementation is to encode the access in policy not in bypasses to the XSM hooks. > Please forgive if this sounds like rubbish to you - I may not have a > good enough understanding of the abstract constraints involved here. No worries, it is always better to question when in doubt than making an assumption. Hopefully I helped in providing a better explanation. > Jan >
Re: [PATCH v2 3/5] xen/events: only register debug interrupt for 2-level events
On 22.10.20 12:35, Jan Beulich wrote:
On 22.10.2020 11:49, Juergen Gross wrote:
@@ -2080,10 +2080,12 @@ void __init xen_init_IRQ(void)
int ret = -EINVAL;
evtchn_port_t evtchn;
- if (fifo_events)
+ if (xen_fifo_events)
ret = xen_evtchn_fifo_init();
- if (ret < 0)
+ if (ret < 0) {
xen_evtchn_2l_init();
+ xen_fifo_events = false;
+ }
Another note: While it may not matter right here, maybe better
first set the variable and the call the function?
I don't think this is really important, TBH.
This code is executed before other cpus are up and we'd have major
other problems in case the sequence would really matter here.
Juergen
Re: [PATCH v2 3/5] xen/events: only register debug interrupt for 2-level events
On 22.10.2020 15:09, Jürgen Groß wrote:
> On 22.10.20 12:35, Jan Beulich wrote:
>> On 22.10.2020 11:49, Juergen Gross wrote:
>>> @@ -2080,10 +2080,12 @@ void __init xen_init_IRQ(void)
>>> int ret = -EINVAL;
>>> evtchn_port_t evtchn;
>>>
>>> - if (fifo_events)
>>> + if (xen_fifo_events)
>>> ret = xen_evtchn_fifo_init();
>>> - if (ret < 0)
>>> + if (ret < 0) {
>>> xen_evtchn_2l_init();
>>> + xen_fifo_events = false;
>>> + }
>>
>> Another note: While it may not matter right here, maybe better
>> first set the variable and the call the function?
>
> I don't think this is really important, TBH.
>
> This code is executed before other cpus are up and we'd have major
> other problems in case the sequence would really matter here.
Fair enough; I was thinking in particular that it ought to be
legitimate for xen_evtchn_2l_init() to BUG_ON(xen_fifo_events).
Jan
Re: [PATCH v2 4/8] evtchn: let evtchn_set_priority() acquire the per-channel lock
On 22.10.2020 13:17, Roger Pau Monné wrote: > On Tue, Oct 20, 2020 at 04:09:41PM +0200, Jan Beulich wrote: >> Some lock wants to be held to make sure the port doesn't change state, >> but there's no point holding the per-domain event lock here. Switch to >> using the finer grained per-channel lock instead. > > While true that's a fine grained lock, it also disables interrupts, > which the global event_lock didn't. True, yet we're aiming at dropping this aspect again. Hence I've added "(albeit as a downside for the time being this requires disabling interrupts for a short period of time)". >> FAOD this doesn't guarantee anything towards in particular >> evtchn_fifo_set_pending(), as for interdomain channels that function >> would be called with the remote side's per-channel lock held. >> >> Signed-off-by: Jan Beulich > > Reviewed-by: Roger Pau Monné Thanks. Jan
[PATCH] x86emul: fix PINSRW and adjust other {,V}PINSR*
The use of simd_packed_int together with no further update to op_bytes
has lead to wrong signaling of #GP(0) for PINSRW without a 16-byte
aligned memory operand. Use simd_none instead and override it after
general decoding with simd_other, like is done for the B/D/Q siblings.
While benign, for consistency also use DstImplicit instead of DstReg
in x86_decode_twobyte().
PINSR{B,D,Q} also had a stray (redundant) get_fpu() invocation, which
gets dropped.
For further consistency also
- use src.bytes instead of op_bytes in relevant memcpy() invocations,
- avoid the pointless updating of op_bytes (all we care about later is
that the value be less than 16).
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -362,7 +362,7 @@ static const struct twobyte_table {
[0xc1] = { DstMem|SrcReg|ModRM },
[0xc2] = { DstImplicit|SrcImmByte|ModRM, simd_any_fp, d8s_vl },
[0xc3] = { DstMem|SrcReg|ModRM|Mov },
-[0xc4] = { DstReg|SrcImmByte|ModRM, simd_packed_int, 1 },
+[0xc4] = { DstImplicit|SrcImmByte|ModRM, simd_none, 1 },
[0xc5] = { DstReg|SrcImmByte|ModRM|Mov },
[0xc6] = { DstImplicit|SrcImmByte|ModRM, simd_packed_fp, d8s_vl },
[0xc7] = { ImplicitOps|ModRM },
@@ -2786,7 +2786,7 @@ x86_decode_twobyte(
/* fall through */
case X86EMUL_OPC_VEX_66(0, 0xc4): /* vpinsrw */
case X86EMUL_OPC_EVEX_66(0, 0xc4): /* vpinsrw */
-state->desc = DstReg | SrcMem16;
+state->desc = DstImplicit | SrcMem16;
break;
case 0xf0:
@@ -8589,6 +8589,7 @@ x86_emulate(
generate_exception_if(vex.l, EXC_UD);
memcpy(mmvalp, &src.val, 2);
ea.type = OP_MEM;
+state->simd_size = simd_other;
goto simd_0f_int_imm8;
#ifndef X86EMUL_NO_SIMD
@@ -8603,9 +8604,8 @@ x86_emulate(
host_and_vcpu_must_have(avx512bw);
if ( !mode_64bit() )
evex.w = 0;
-memcpy(mmvalp, &src.val, op_bytes);
+memcpy(mmvalp, &src.val, src.bytes);
ea.type = OP_MEM;
-op_bytes = src.bytes;
d = SrcMem16; /* Fake for the common SIMD code below. */
state->simd_size = simd_other;
goto avx512f_imm8_no_sae;
@@ -10774,10 +10774,8 @@ x86_emulate(
case X86EMUL_OPC_66(0x0f3a, 0x20): /* pinsrb $imm8,r32/m8,xmm */
case X86EMUL_OPC_66(0x0f3a, 0x22): /* pinsr{d,q} $imm8,r/m,xmm */
host_and_vcpu_must_have(sse4_1);
-get_fpu(X86EMUL_FPU_xmm);
-memcpy(mmvalp, &src.val, op_bytes);
+memcpy(mmvalp, &src.val, src.bytes);
ea.type = OP_MEM;
-op_bytes = src.bytes;
d = SrcMem16; /* Fake for the common SIMD code below. */
state->simd_size = simd_other;
goto simd_0f3a_common;
@@ -10787,9 +10785,8 @@ x86_emulate(
generate_exception_if(vex.l, EXC_UD);
if ( !mode_64bit() )
vex.w = 0;
-memcpy(mmvalp, &src.val, op_bytes);
+memcpy(mmvalp, &src.val, src.bytes);
ea.type = OP_MEM;
-op_bytes = src.bytes;
d = SrcMem16; /* Fake for the common SIMD code below. */
state->simd_size = simd_other;
goto simd_0f_int_imm8;
[PATCH] x86emul: increase FPU save area in test harness/fuzzer
Running them on a system (or emulator) with AMX support requires this to be quite a bit larger than 8k, to avoid triggering the assert() in emul_test_init(). Bump all the way up to 16k right away. Signed-off-by: Jan Beulich --- Intel's Software Development Emulator properly reports XSAVE-related AMX CPUID data for Sapphire Rapids emulation as of 8.59.0. --- a/tools/tests/x86_emulator/x86-emulate.c +++ b/tools/tests/x86_emulator/x86-emulate.c @@ -33,7 +33,7 @@ uint32_t mxcsr_mask = 0xffbf; struct cpuid_policy cp; -static char fpu_save_area[4096] __attribute__((__aligned__((64; +static char fpu_save_area[0x4000] __attribute__((__aligned__((64; static bool use_xsave; /*
Re: [PATCH] x86emul: increase FPU save area in test harness/fuzzer
On 22/10/2020 14:39, Jan Beulich wrote: > Running them on a system (or emulator) with AMX support requires this > to be quite a bit larger than 8k, to avoid triggering the assert() in > emul_test_init(). Bump all the way up to 16k right away. > > Signed-off-by: Jan Beulich Acked-by: Andrew Cooper > --- > Intel's Software Development Emulator properly reports XSAVE-related AMX > CPUID data for Sapphire Rapids emulation as of 8.59.0. > > --- a/tools/tests/x86_emulator/x86-emulate.c > +++ b/tools/tests/x86_emulator/x86-emulate.c > @@ -33,7 +33,7 @@ > uint32_t mxcsr_mask = 0xffbf; > struct cpuid_policy cp; > > -static char fpu_save_area[4096] __attribute__((__aligned__((64; > +static char fpu_save_area[0x4000] __attribute__((__aligned__((64; > static bool use_xsave; > > /*
[libvirt test] 156082: regressions - FAIL
flight 156082 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/156082/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-i386-libvirt6 libvirt-buildfail REGR. vs. 151777 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 151777 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a version targeted for testing: libvirt 25cb07498e962e9e8452e6b17c397132e8364ec7 baseline version: libvirt 2c846fa6bcc11929c9fb857a22430fb9945654ad Last test of basis 151777 2020-07-10 04:19:19 Z 104 days Failing since151818 2020-07-11 04:18:52 Z 103 days 98 attempts Testing same since 156082 2020-10-22 04:19:11 Z0 days1 attempts People who touched revisions under test: Adolfo Jayme Barrientos Andika Triwidada Andrea Bolognani Balázs Meskó Bastien Orivel Bihong Yu Binfeng Wu Boris Fiuczynski Christian Ehrhardt Cole Robinson Collin Walling Cornelia Huck Côme Borsoi Daniel Henrique Barboza Daniel Letai Daniel P. Berrange Daniel P. Berrangé Erik Skultety Fabian Freyer Fangge Jin Fedora Weblate Translation Halil Pasic Han Han Hao Wang Ian Wienand Jamie Strandboge Jamie Strandboge Jean-Baptiste Holcroft Jianan Gao Jim Fehlig Jin Yan Jiri Denemark Jonathon Jongsma Ján Tomko Kashyap Chamarthy Kevin Locke Laine Stump Liao Pingfang Lin Ma Lin Ma Marc Hartmayer Marek Marczykowski-Górecki Markus Schade Martin Kletzander Masayoshi Mizuma Matt Coleman Matt Coleman Mauro Matteo Cascella Michal Privoznik Michał Smyk Milo Casagrande Neal Gompa Nico Pache Nikolay Shirokovskiy Olesya Gerasimenko Patrick Magauran Paulo de Rezende Pinatti Pavel Hrdina Peter Krempa Pino Toscano Pino Toscano Piotr Drąg Prathamesh Chavan Roman Bogorodskiy Roman Bolshakov Ryan Schmidt Sam Hartman Scott Shambarger Sebastian Mitterle Simon Gaiser Stefan Bader Stefan Berger Szymon Scholz Thomas Huth Tim Wiederhake Tomáš Golembiovský Wang Xin Weblate Yang Hang Yanqiu Zhang Yi Li Yi Wang Yuri Chornoivan Zheng Chuan zhenwei pi Zhenyu Zheng jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt fail build-arm64-libvirt fail build-armhf-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm blocked test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmblocked test-amd64-amd64-libvirt-xsm blocked test-arm64-arm64-libvirt-xsm blocked test-amd64-i386
[ovmf test] 156065: all pass - PUSHED
flight 156065 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/156065/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 26442d11e620a9e81c019a24a4ff38441c64ba10 baseline version: ovmf f82b827c92f77eac8debdce6ef9689d156771871 Last test of basis 156017 2020-10-20 06:40:54 Z2 days Testing same since 156065 2020-10-21 06:40:48 Z1 days1 attempts People who touched revisions under test: Jian J Wang jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git f82b827c92..26442d11e6 26442d11e620a9e81c019a24a4ff38441c64ba10 -> xen-tested-master
[PATCH] xen: add support for automatic debug key actions in case of crash
When the host crashes it would sometimes be nice to have additional
debug data available which could be produced via debug keys, but
halting the server for manual intervention might be impossible due to
the need to reboot/kexec rather sooner than later.
Add support for automatic debug key actions in case of crashes which
can be activated via boot- or runtime-parameter.
Depending on the type of crash the desired data might be different, so
support different settings for the possible types of crashes.
The parameter is "crash-debug" with the following syntax:
crash-debug-=
with being one of:
panic, hwdom, watchdog, kexeccmd, debugkey
and a sequence of debug key characters with '.' having the
special semantics of a 1 second pause.
So "crash-debug-watchdog=0.0qr" would result in special output in case
of watchdog triggered crash (dom0 state, 1 second pause, dom0 state,
domain info, run queues).
Signed-off-by: Juergen Gross
---
docs/misc/xen-command-line.pandoc | 23 +++
xen/common/kexec.c| 8 ---
xen/common/keyhandler.c | 37 +++
xen/common/shutdown.c | 4 ++--
xen/drivers/char/console.c| 2 +-
xen/include/xen/kexec.h | 10 +++--
xen/include/xen/keyhandler.h | 11 +
7 files changed, 87 insertions(+), 8 deletions(-)
diff --git a/docs/misc/xen-command-line.pandoc
b/docs/misc/xen-command-line.pandoc
index 4ae9391fcd..f328c99cf8 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -574,6 +574,29 @@ reduction of features at Xen's disposal to manage guests.
### cpuinfo (x86)
> `= `
+### crash-debug-debugkey
+### crash-debug-hwdom
+### crash-debug-kexeccmd
+### crash-debug-panic
+### crash-debug-watchdog
+> `= `
+
+> Can be modified at runtime
+
+Specify debug-key actions in cases of crashes. Each of the parameters applies
+to a different crash reason. The `` is a sequence of debug key
+characters, with `.` having the special meaning of a 1 second pause.
+
+So e.g. `crash-debug-watchdog=0.0r` would dump dom0 state twice with a
+second between the two state dumps, followed by the run queues of the
+hypervisor, if the system crashes due to a watchdog timeout.
+
+These parameters should be used carefully, as e.g. specifying
+`crash-debug-debugkey=C` would result in an endless loop. Depending on the
+reason of the system crash it might happen that triggering some debug key
+action will result in a hang instead of dumping data and then doing a
+reboot or crash dump.
+
### crashinfo_maxaddr
> `= `
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 52cdc4ebc3..ebeee6405a 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -373,10 +373,12 @@ static int kexec_common_shutdown(void)
return 0;
}
-void kexec_crash(void)
+void kexec_crash(enum crash_reason reason)
{
int pos;
+keyhandler_crash_action(reason);
+
pos = (test_bit(KEXEC_FLAG_CRASH_POS, &kexec_flags) != 0);
if ( !test_bit(KEXEC_IMAGE_CRASH_BASE + pos, &kexec_flags) )
return;
@@ -409,7 +411,7 @@ static long kexec_reboot(void *_image)
static void do_crashdump_trigger(unsigned char key)
{
printk("'%c' pressed -> triggering crashdump\n", key);
-kexec_crash();
+kexec_crash(CRASHREASON_DEBUGKEY);
printk(" * no crash kernel loaded!\n");
}
@@ -840,7 +842,7 @@ static int kexec_exec(XEN_GUEST_HANDLE_PARAM(void) uarg)
ret = continue_hypercall_on_cpu(0, kexec_reboot, image);
break;
case KEXEC_TYPE_CRASH:
-kexec_crash(); /* Does not return */
+kexec_crash(CRASHREASON_KEXECCMD); /* Does not return */
break;
}
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 68364e987d..ac8229a4d7 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -3,7 +3,9 @@
*/
#include
+#include
#include
+#include
#include
#include
#include
@@ -507,6 +509,41 @@ void __init initialize_keytable(void)
}
}
+#define CRASHACTION_SIZE 32
+static char crash_debug_panic[CRASHACTION_SIZE];
+static char crash_debug_hwdom[CRASHACTION_SIZE];
+static char crash_debug_watchdog[CRASHACTION_SIZE];
+static char crash_debug_kexeccmd[CRASHACTION_SIZE];
+static char crash_debug_debugkey[CRASHACTION_SIZE];
+
+static char *crash_action[CRASHREASON_N] = {
+[CRASHREASON_PANIC] = crash_debug_panic,
+[CRASHREASON_HWDOM] = crash_debug_hwdom,
+[CRASHREASON_WATCHDOG] = crash_debug_watchdog,
+[CRASHREASON_KEXECCMD] = crash_debug_kexeccmd,
+[CRASHREASON_DEBUGKEY] = crash_debug_debugkey,
+};
+
+string_runtime_param("crash-debug-panic", crash_debug_panic);
+string_runtime_param("crash-debug-hwdom", crash_debug_hwdom);
+string_runtime_param("crash-debug-watchdog", crash_debug_watchdog);
+string_runtime_param("crash-debug-kexeccmd", crash_debug_kexeccmd);
+string_runtime_param("crash-debug-debugkey", crash_debug_debugkey);
+
+void keyhandler_crash_action
[xen-4.14-testing test] 156063: tolerable FAIL - PUSHED
flight 156063 xen-4.14-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/156063/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-qemut-rhel6hvm-intel 14 guest-start/redhat.repeat fail in 156031 pass in 156063 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail in 156031 pass in 156063 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail pass in 156031 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 155417 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 155417 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 155417 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 155417 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 155417 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 155417 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 155417 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 155417 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 155417 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 155417 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 155417 test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: xen 7b1e587f25c2dda38236e48aae81729798f10663 baseline version: xen c93b520a41f2787dd76bfb2e454836d1d5787505 Last test of basis 155417 2020-10-04 02:29:19 Z 18 days Testing same since 156031 2020-10-20 13:06:19 Z2 days2 attempts -
[qemu-mainline test] 156081: regressions - FAIL
flight 156081 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/156081/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 152631 build-amd64 6 xen-buildfail REGR. vs. 152631 build-arm64 6 xen-buildfail REGR. vs. 152631 build-arm64-xsm 6 xen-buildfail REGR. vs. 152631 build-i3866 xen-buildfail REGR. vs. 152631 build-i386-xsm6 xen-buildfail REGR. vs. 152631 build-armhf 6 xen-buildfail REGR. vs. 152631 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit1 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-coresched-i386-xl 1 build-check(1) blocked n/a test-amd64-coresched-amd64-xl 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-shadow1 build-check(1) blocked n/a test-amd64-amd64-xl-rtds 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-amd 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvshim1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-intel 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-intel 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-amd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-freebsd11-amd64 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-freebsd12-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit1 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-amd 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 bu
[xen-unstable test] 156085: regressions - FAIL
flight 156085 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/156085/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 156013 build-arm64 6 xen-buildfail REGR. vs. 156013 build-arm64-xsm 6 xen-buildfail REGR. vs. 156013 build-amd64 6 xen-buildfail REGR. vs. 156013 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-rtds 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvshim1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-intel 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-amd 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit1 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-amd 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-intel 1 build-check(1) blocked n/a test-amd64-amd64-examine 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-amd 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-freebsd12-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-freebsd11-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-livepatch1 build-check(1) blocked n/a test-amd64-amd64-migrupgrade 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-shadow1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-coresched-amd64-xl 1 build-check(1) blocked n/a test-amd64-coresched-i386-xl 1 build-check(1) blocked n/a test-amd64-i386-examine 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-livepatch 1 build-check(1) blocked n/a test-amd6
Re: [PATCH v2 0/3] Add Xen CpusAccel
On Tue, Oct 13, 2020 at 1:16 PM Paolo Bonzini wrote:
>
> On 13/10/20 16:05, Jason Andryuk wrote:
> > Xen was left behind when CpusAccel became mandatory and fails the assert
> > in qemu_init_vcpu(). It relied on the same dummy cpu threads as qtest.
> > Move the qtest cpu functions to a common location and reuse them for
> > Xen.
> >
> > v2:
> > New patch "accel: Remove _WIN32 ifdef from qtest-cpus.c"
> > Use accel/dummy-cpus.c for filename
> > Put prototype in include/sysemu/cpus.h
> >
> > Jason Andryuk (3):
> > accel: Remove _WIN32 ifdef from qtest-cpus.c
> > accel: move qtest CpusAccel functions to a common location
> > accel: Add xen CpusAccel using dummy-cpus
> >
> > accel/{qtest/qtest-cpus.c => dummy-cpus.c} | 27 --
> > accel/meson.build | 8 +++
> > accel/qtest/meson.build| 1 -
> > accel/qtest/qtest-cpus.h | 17 --
> > accel/qtest/qtest.c| 5 +++-
> > accel/xen/xen-all.c| 8 +++
> > include/sysemu/cpus.h | 3 +++
> > 7 files changed, 27 insertions(+), 42 deletions(-)
> > rename accel/{qtest/qtest-cpus.c => dummy-cpus.c} (71%)
> > delete mode 100644 accel/qtest/qtest-cpus.h
> >
>
> Acked-by: Paolo Bonzini
Thank you, Paolo. Also Anthony Acked and Claudio Reviewed patch 3.
How can we get this into the tree?
Regards,
Jason
Re: [PATCH v2 0/3] Add Xen CpusAccel
On 22/10/20 17:17, Jason Andryuk wrote:
> On Tue, Oct 13, 2020 at 1:16 PM Paolo Bonzini wrote:
>>
>> On 13/10/20 16:05, Jason Andryuk wrote:
>>> Xen was left behind when CpusAccel became mandatory and fails the assert
>>> in qemu_init_vcpu(). It relied on the same dummy cpu threads as qtest.
>>> Move the qtest cpu functions to a common location and reuse them for
>>> Xen.
>>>
>>> v2:
>>> New patch "accel: Remove _WIN32 ifdef from qtest-cpus.c"
>>> Use accel/dummy-cpus.c for filename
>>> Put prototype in include/sysemu/cpus.h
>>>
>>> Jason Andryuk (3):
>>> accel: Remove _WIN32 ifdef from qtest-cpus.c
>>> accel: move qtest CpusAccel functions to a common location
>>> accel: Add xen CpusAccel using dummy-cpus
>>>
>>> accel/{qtest/qtest-cpus.c => dummy-cpus.c} | 27 --
>>> accel/meson.build | 8 +++
>>> accel/qtest/meson.build| 1 -
>>> accel/qtest/qtest-cpus.h | 17 --
>>> accel/qtest/qtest.c| 5 +++-
>>> accel/xen/xen-all.c| 8 +++
>>> include/sysemu/cpus.h | 3 +++
>>> 7 files changed, 27 insertions(+), 42 deletions(-)
>>> rename accel/{qtest/qtest-cpus.c => dummy-cpus.c} (71%)
>>> delete mode 100644 accel/qtest/qtest-cpus.h
>>>
>>
>> Acked-by: Paolo Bonzini
>
> Thank you, Paolo. Also Anthony Acked and Claudio Reviewed patch 3.
> How can we get this into the tree?
I think Anthony should send a pull request?
Paolo
Re: [PATCH v2 5/8] evtchn: drop acquiring of per-channel lock from send_guest_{global,vcpu}_virq()
On Tue, Oct 20, 2020 at 04:10:09PM +0200, Jan Beulich wrote: > The per-vCPU virq_lock, which is being held anyway, together with there > not being any call to evtchn_port_set_pending() when v->virq_to_evtchn[] > is zero, provide sufficient guarantees. This is also fine because closing the event channel will be done with the VIRQ hold held also AFAICT. > Undo the lock addition done for > XSA-343 (commit e045199c7c9c "evtchn: address races with > evtchn_reset()"). Update the description next to struct evtchn_port_ops > accordingly. > > Signed-off-by: Jan Beulich > --- > v2: New. > > --- a/xen/common/event_channel.c > +++ b/xen/common/event_channel.c > @@ -794,7 +794,6 @@ void send_guest_vcpu_virq(struct vcpu *v > unsigned long flags; > int port; > struct domain *d; > -struct evtchn *chn; > > ASSERT(!virq_is_global(virq)); > > @@ -805,10 +804,7 @@ void send_guest_vcpu_virq(struct vcpu *v > goto out; > > d = v->domain; > -chn = evtchn_from_port(d, port); > -spin_lock(&chn->lock); > -evtchn_port_set_pending(d, v->vcpu_id, chn); > -spin_unlock(&chn->lock); > +evtchn_port_set_pending(d, v->vcpu_id, evtchn_from_port(d, port)); > > out: > spin_unlock_irqrestore(&v->virq_lock, flags); > @@ -837,9 +833,7 @@ void send_guest_global_virq(struct domai > goto out; > > chn = evtchn_from_port(d, port); > -spin_lock(&chn->lock); > evtchn_port_set_pending(d, chn->notify_vcpu_id, chn); > -spin_unlock(&chn->lock); > > out: > spin_unlock_irqrestore(&v->virq_lock, flags); > --- a/xen/include/xen/event.h > +++ b/xen/include/xen/event.h > @@ -177,9 +177,16 @@ int evtchn_reset(struct domain *d, bool > * Low-level event channel port ops. > * > * All hooks have to be called with a lock held which prevents the channel > - * from changing state. This may be the domain event lock, the per-channel > - * lock, or in the case of sending interdomain events also the other side's > - * per-channel lock. Exceptions apply in certain cases for the PV shim. > + * from changing state. This may be > + * - the domain event lock, > + * - the per-channel lock, > + * - in the case of sending interdomain events the other side's per-channel > + * lock, > + * - in the case of sending non-global vIRQ-s the per-vCPU virq_lock (in > + * combination with the ordering enforced through how the vCPU's > + * virq_to_evtchn[] gets updated), > + * - in the case of sending global vIRQ-s vCPU 0's virq_lock. > + * Exceptions apply in certain cases for the PV shim. Having such a wide locking discipline looks dangerous to me, it's easy to get things wrong without notice IMO. Maybe we could add an assert to that effect in evtchn_port_set_pending in order to make sure callers follow the discipline? Roger.
Re: [PATCH v2 05/11] x86/vioapic: switch to use the EOI callback mechanism
On 30.09.2020 12:41, Roger Pau Monne wrote: > --- a/xen/arch/x86/hvm/vlapic.c > +++ b/xen/arch/x86/hvm/vlapic.c > @@ -189,7 +189,7 @@ void vlapic_set_irq_callback(struct vlapic *vlapic, > uint8_t vec, uint8_t trig, > > if ( hvm_funcs.update_eoi_exit_bitmap ) > alternative_vcall(hvm_funcs.update_eoi_exit_bitmap, target, vec, > - trig || callback); > + callback); There's a shortcoming in the alternative call framework which I see no way to eliminate but which makes it necessary to use !!callback here. Otherwise, if the callback happens to sit on a 256-byte boundary (low address byte zero), you'll pass false when you mean true. (The original use, i.e. prior to patch 3, of just "trig" was sufficiently okay, because the parameter - despite being u8 - is effectively used as a boolean by the callers iirc.) Or perhaps the best thing is to require wrappers for all hooks taking bool parameters, because then the necessary conversion will be done when calling the wrapper. Jan
Re: [PATCH v2 5/8] evtchn: drop acquiring of per-channel lock from send_guest_{global,vcpu}_virq()
On 22.10.2020 18:00, Roger Pau Monné wrote: > On Tue, Oct 20, 2020 at 04:10:09PM +0200, Jan Beulich wrote: >> The per-vCPU virq_lock, which is being held anyway, together with there >> not being any call to evtchn_port_set_pending() when v->virq_to_evtchn[] >> is zero, provide sufficient guarantees. > > This is also fine because closing the event channel will be done with > the VIRQ hold held also AFAICT. Right, I'm not going into these details (or else binding would also need mentioning) here, as the code comment update should sufficiently cover it. Hence just saying "sufficient guarantees". >> --- a/xen/include/xen/event.h >> +++ b/xen/include/xen/event.h >> @@ -177,9 +177,16 @@ int evtchn_reset(struct domain *d, bool >> * Low-level event channel port ops. >> * >> * All hooks have to be called with a lock held which prevents the channel >> - * from changing state. This may be the domain event lock, the per-channel >> - * lock, or in the case of sending interdomain events also the other side's >> - * per-channel lock. Exceptions apply in certain cases for the PV shim. >> + * from changing state. This may be >> + * - the domain event lock, >> + * - the per-channel lock, >> + * - in the case of sending interdomain events the other side's per-channel >> + * lock, >> + * - in the case of sending non-global vIRQ-s the per-vCPU virq_lock (in >> + * combination with the ordering enforced through how the vCPU's >> + * virq_to_evtchn[] gets updated), >> + * - in the case of sending global vIRQ-s vCPU 0's virq_lock. >> + * Exceptions apply in certain cases for the PV shim. > > Having such a wide locking discipline looks dangerous to me, it's easy > to get things wrong without notice IMO. It is effectively only describing how things are (or were before XSA-343, getting restored here). > Maybe we could add an assert to that effect in > evtchn_port_set_pending in order to make sure callers follow the > discipline? Would be nice, but (a) see the last sentence of the comment still in context above and (b) it shouldn't be just set_pending(). The comment starts with "All hooks ..." after all. Jan
[qemu-mainline test] 156094: regressions - FAIL
flight 156094 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/156094/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 152631 build-amd64 6 xen-buildfail REGR. vs. 152631 build-arm64 6 xen-buildfail REGR. vs. 152631 build-arm64-xsm 6 xen-buildfail REGR. vs. 152631 build-i3866 xen-buildfail REGR. vs. 152631 build-i386-xsm6 xen-buildfail REGR. vs. 152631 build-armhf 6 xen-buildfail REGR. vs. 152631 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit1 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-coresched-i386-xl 1 build-check(1) blocked n/a test-amd64-coresched-amd64-xl 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-shadow1 build-check(1) blocked n/a test-amd64-amd64-xl-rtds 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-amd 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvshim1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-intel 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-intel 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-amd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-freebsd11-amd64 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-freebsd12-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit1 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-amd 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-inte
[OSSTEST PATCH 08/16] PDU/MSW: Make show() return the value from get()
No-one uses this return value yet, so NFC.
Signed-off-by: Ian Jackson
---
pdu-msw | 1 +
1 file changed, 1 insertion(+)
diff --git a/pdu-msw b/pdu-msw
index 196b6c45..2d4ec967 100755
--- a/pdu-msw
+++ b/pdu-msw
@@ -119,6 +119,7 @@ sub get () {
sub show () {
my $mean = get();
printf "pdu-msw $dnsname: #%s \"%s\" = %s\n", $useport, $usename, $mean;
+return $mean;
}
sub action_value () {
--
2.20.1
[OSSTEST PATCH 02/16] share in jobdb: Move out-of-flight special case higher up
This avoids running the runvar computation loop outside flights.
This is good amongst other things because that loop prints warnings
about undef $flight and $job.
Signed-off-by: Ian Jackson
---
Osstest/JobDB/Executive.pm | 33 ++---
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/Osstest/JobDB/Executive.pm b/Osstest/JobDB/Executive.pm
index 071f31f1..4fa42e5d 100644
--- a/Osstest/JobDB/Executive.pm
+++ b/Osstest/JobDB/Executive.pm
@@ -587,6 +587,18 @@ END
$constraintsq->fetchrow_array() or confess "$hostname ?";
};
+
+if (!defined $flight) {
+ db_retry($dbh_tests,[], sub {
+ $insertq->execute($hostname, $ttaskid,
+ undef,undef,
+ undef,
+ undef,undef);
+ $checkconstraints->();
+ });
+ return;
+}
+
my $ojvn = "$ho->{Ident}_lifecycle";
if (length $r{$ojvn}) {
@@ -660,26 +672,17 @@ END
}
}
- if (defined $flight) {
- $insertq->execute($hostname, $ttaskid,
- $flight, $job,
- ($mode eq 'selectprep')+0,
+ $insertq->execute($hostname, $ttaskid,
+ $flight, $job,
+ ($mode eq 'selectprep')+0,
# ^ DBD::Pg doesn't accept perl canonical false for bool!
# https://rt.cpan.org/Public/Bug/Display.html?id=133229
- $tident, $tstepno);
- } else {
- $insertq->execute($hostname, $ttaskid,
- undef,undef,
- undef,
- undef,undef);
- }
+ $tident, $tstepno);
$checkconstraints->();
});
-if (defined $flight) {
- push @lifecycle, $newsigil if length $newsigil;
- store_runvar($ojvn, "@lifecycle");
-}
+push @lifecycle, $newsigil if length $newsigil;
+store_runvar($ojvn, "@lifecycle");
}
sub current_stepno ($) { #method
--
2.20.1
[OSSTEST PATCH 00/16] Bugfixes
Many of these are fixes to host sharing. I'm still doing a formal dev test but I expect to push these soon. Ian Jackson (16): share in jobdb: Break out $checkconstraints and move call share in jobdb: Move out-of-flight special case higher up PDU/IPMI: Retransmit, don't just wait PDU/MSW: Warn that SNMP status is often not immediately updated PDU/MSW: Break out get() PDU/MSW: Break out action_value() PDU/MSW: Actually implement delayed-* PDU/MSW: Make show() return the value from get() PDU/MSU: Retransmit on/off until PDU has changed host reuse fixes: Fix running of steps adhoc host reuse fixes: Fix runvar entry for adhoc tasks Introduce guest_mk_lv_name Prefix guest LV names with the job name reporting: Minor fix to reporting of tasks with no subtask host reuse fixes: Do not break host-reuse if no host allocated starvation: Do not count more than half a flight as starved Osstest/Executive.pm| 2 +- Osstest/JobDB/Executive.pm | 46 +++-- Osstest/PDU/ipmi.pm | 5 ++-- Osstest/TestSupport.pm | 9 ++-- pdu-msw | 37 + ts-debian-fixup | 22 ++ ts-debian-install | 2 +- ts-host-reuse | 2 +- ts-hosts-allocate-Executive | 2 +- 9 files changed, 97 insertions(+), 30 deletions(-) -- 2.20.1
[OSSTEST PATCH 01/16] share in jobdb: Break out $checkconstraints and move call
This must happen after we introduce our new row or it is not
effective!
Signed-off-by: Ian Jackson
---
Osstest/JobDB/Executive.pm | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Osstest/JobDB/Executive.pm b/Osstest/JobDB/Executive.pm
index f69ce277..071f31f1 100644
--- a/Osstest/JobDB/Executive.pm
+++ b/Osstest/JobDB/Executive.pm
@@ -582,6 +582,11 @@ END
VALUES (?,?, ?, ?, ?, ?, ? )
END
+my $checkconstraints = sub {
+ $constraintsq->execute($hostname, $ttaskid);
+ $constraintsq->fetchrow_array() or confess "$hostname ?";
+};
+
my $ojvn = "$ho->{Ident}_lifecycle";
if (length $r{$ojvn}) {
@@ -654,8 +659,6 @@ END
push @lifecycle, "$omarks$otj:$o->{stepno}$osuffix";
}
}
- $constraintsq->execute($hostname, $ttaskid);
- $constraintsq->fetchrow_array() or confess "$hostname ?";
if (defined $flight) {
$insertq->execute($hostname, $ttaskid,
@@ -670,6 +673,7 @@ END
undef,
undef,undef);
}
+ $checkconstraints->();
});
if (defined $flight) {
--
2.20.1
[OSSTEST PATCH 07/16] PDU/MSW: Actually implement delayed-*
Nothing in our tree uses this but having it here is useful docs for
the protocol so I shan't just delete it.
Signed-off-by: Ian Jackson
---
pdu-msw | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pdu-msw b/pdu-msw
index 03b0f342..196b6c45 100755
--- a/pdu-msw
+++ b/pdu-msw
@@ -127,7 +127,7 @@ sub action_value () {
$action =~ m/^(?:1|on)$/ ? 1 :
$action =~ m/^(?:reboot)$/ ? 3 :
die "unknown action $action\n$usagemsg");
-return $valset;
+return $valset + $delayadd;
}
sub set ($) {
--
2.20.1
[OSSTEST PATCH 04/16] PDU/MSW: Warn that SNMP status is often not immediately updated
If you don't know this, it's very confusing.
Signed-off-by: Ian Jackson
---
pdu-msw | 1 +
1 file changed, 1 insertion(+)
diff --git a/pdu-msw b/pdu-msw
index d2691567..04b03a22 100755
--- a/pdu-msw
+++ b/pdu-msw
@@ -133,4 +133,5 @@ if (!defined $action) {
print "was: "; show();
set();
print "now: "; show();
+print "^ note, PDUs often do not update returned info immediately\n";
}
--
2.20.1
[OSSTEST PATCH 09/16] PDU/MSU: Retransmit on/off until PDU has changed
The main effect of this is that the transcript will actually show the
new PDU state. Previously we would call show(), but APC PDUs would
normally not change immediately, so the transcript would show the old
state.
This also guards against an unresponsive PDU or a packet getting lost.
I don't think we have ever seen that.
Signed-off-by: Ian Jackson
---
pdu-msw | 21 ++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/pdu-msw b/pdu-msw
index 2d4ec967..c57f9f7c 100755
--- a/pdu-msw
+++ b/pdu-msw
@@ -41,6 +41,7 @@ while (@ARGV && $ARGV[0] =~ m/^-/) {
if (@ARGV<2 || @ARGV>3 || $ARGV[0] =~ m/^-/) { die "bad usage\n$usagemsg"; }
+our ($max_retries) = 16; # timeout = 0.05 * max_retries^2
our ($dnsname,$outlet,$action) = @ARGV;
my ($session,$error) = Net::SNMP->session(
@@ -142,7 +143,21 @@ if (!defined $action) {
} else {
my $valset = action_value();
print "was: "; show();
-set($valset);
-print "now: "; show();
-print "^ note, PDUs often do not update returned info immediately\n";
+
+my $retries = 0;
+for (;;) {
+ set($valset);
+ sleep $retries * 0.1;
+ print "now: "; my $got = show();
+ if ($got eq $map[$valset]) { last; }
+ if ($map[$valset] !~ m{^(?:off|on)$}) {
+ print
+ "^ note, PDUs often do not update returned info immediately\n";
+ last;
+ }
+ if ($retries >= $max_retries) {
+ die "PDU does not seem to be changing state!\n";
+ }
+ $retries++;
+}
}
--
2.20.1
[OSSTEST PATCH 03/16] PDU/IPMI: Retransmit, don't just wait
We have a system for which
ipmitool -H sabro0m -U root -P -I lanplus power on
seems to work but doesn't take effect the first time.
Retransit each retry.
Signed-off-by: Ian Jackson
---
Osstest/PDU/ipmi.pm | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Osstest/PDU/ipmi.pm b/Osstest/PDU/ipmi.pm
index 98e8957f..21c94d98 100644
--- a/Osstest/PDU/ipmi.pm
+++ b/Osstest/PDU/ipmi.pm
@@ -66,11 +66,12 @@ sub pdu_power_state {
return;
}
-system_checked((@cmd, qw(power), $onoff));
-
my $count = 60;
for (;;) {
last if $getstatus->() eq $onoff;
+
+ system_checked((@cmd, qw(power), $onoff));
+
die "did not power $onoff" unless --$count > 0;
sleep(1);
}
--
2.20.1
[OSSTEST PATCH 05/16] PDU/MSW: Break out get()
This is going to be useful in a moment.
Signed-off-by: Ian Jackson
---
pdu-msw | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/pdu-msw b/pdu-msw
index 04b03a22..58c33952 100755
--- a/pdu-msw
+++ b/pdu-msw
@@ -106,13 +106,18 @@ my @map= (undef, qw(
delayed-off
delayed-reboot));
-sub show () {
+sub get () {
my $got= $session->get_request($read_oid);
die "SNMP error reading $read_oid ".$session->error()." " unless $got;
my $val= $got->{$read_oid};
die unless $val;
my $mean= $map[$val];
die "$val ?" unless defined $mean;
+return $mean;
+}
+
+sub show () {
+my $mean = get();
printf "pdu-msw $dnsname: #%s \"%s\" = %s\n", $useport, $usename, $mean;
}
--
2.20.1
[OSSTEST PATCH 06/16] PDU/MSW: Break out action_value()
This is going to be useful in a moment.
Signed-off-by: Ian Jackson
---
pdu-msw | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/pdu-msw b/pdu-msw
index 58c33952..03b0f342 100755
--- a/pdu-msw
+++ b/pdu-msw
@@ -121,13 +121,17 @@ sub show () {
printf "pdu-msw $dnsname: #%s \"%s\" = %s\n", $useport, $usename, $mean;
}
-sub set () {
+sub action_value () {
my $delayadd= ($action =~ s/^delayed-// ? 3 : 0);
my $valset= ($action =~ m/^(?:0|off)$/ ? 2 :
$action =~ m/^(?:1|on)$/ ? 1 :
$action =~ m/^(?:reboot)$/ ? 3 :
die "unknown action $action\n$usagemsg");
-
+return $valset;
+}
+
+sub set ($) {
+my ($valset) = @_;
my $res= $session->set_request(-varbindlist => [ $write_oid, INTEGER,
$valset ]);
die "SNMP set ".$session->error()." " unless $res;
}
@@ -135,8 +139,9 @@ sub set () {
if (!defined $action) {
show();
} else {
+my $valset = action_value();
print "was: "; show();
-set();
+set($valset);
print "now: "; show();
print "^ note, PDUs often do not update returned info immediately\n";
}
--
2.20.1
[ovmf test] 156091: all pass - PUSHED
flight 156091 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/156091/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 24cf72726564eac7ba9abb24f3d05795164d0a70 baseline version: ovmf 26442d11e620a9e81c019a24a4ff38441c64ba10 Last test of basis 156065 2020-10-21 06:40:48 Z1 days Testing same since 156091 2020-10-22 14:14:55 Z0 days1 attempts People who touched revisions under test: Pete Batard Sami Mujawar jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 26442d11e6..24cf727265 24cf72726564eac7ba9abb24f3d05795164d0a70 -> xen-tested-master
Re: XSM and the idle domain
On Thu, 2020-10-22 at 13:51 +0100, Andrew Cooper wrote: > On 21/10/2020 15:34, Hongyan Xia wrote: > > The first question came up during ongoing work in LiveUpdate. After > > an > > LU, the next Xen needs to restore all domains. To do that, some > > hypercalls need to be issued from the idle domain context and > > apparently XSM does not like it. > > There is no such thing as issuing hypercalls from the idle domain > (context or otherwise), because the idle domain does not have enough > associated guest state for anything to make the requisite > SYSCALL/INT80/VMCALL/VMMCALL invocation. > > I presume from this comment that what you mean is that you're calling > the plain hypercall functions, context checks and everything, from > the > idle context? Yep, the restore code just calls the hypercall functions from idle context. > If so, this is buggy for more reasons than just XSM objecting to its > calling context, and that XSM is merely the first thing to explode. > Therefore, I don't think modifications to XSM are applicable to > solving > the problem. > > (Of course, this is all speculation because there's no concrete > implementation to look at.) Another explosion is the inability to create hypercall preemption, which for now is disabled when the calling context is the idle domain. Apart from XSM and preemption, the LU prototype works fine. We only reuse a limited number of hypercall functions and are not trying to be able to call all possible hypercalls from idle. Having a dedicated domLU just like domB (or reusing domB) sounds like a viable option. If the overhead can be made low enough then we won't need to work around XSM and hypercall preemption. Although the question was whether XSM should interact with the idle domain. With a good design LU should be able to sidestep this though. Hongyan
[OSSTEST PATCH 12/16] Introduce guest_mk_lv_name
This changes the way the disk name is constructed but not to any
overall effect.
Signed-off-by: Ian Jackson
---
Osstest/TestSupport.pm | 9 +++--
ts-debian-install | 2 +-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 5e6b15d9..12aaba79 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -76,7 +76,7 @@ BEGIN {
target_jobdir target_extract_jobdistpath_subdir
target_extract_jobdistpath target_extract_distpart
target_tftp_prefix
- lv_create lv_dev_mapper
+ lv_create lv_dev_mapper guest_mk_lv_name
poll_loop tcpconnect await_tcp
contents_make_cpio file_simple_write_contents
@@ -2177,6 +2177,11 @@ sub guest_var_commalist ($$) {
return split /\,/, guest_var($gho,$runvartail,'');
}
+sub guest_mk_lv_name ($$) {
+my ($gho, $suffix) = @_;
+return "$gho->{Name}".$suffix;
+}
+
sub prepareguest ($$) {
my ($ho, $gn, $hostname, $tcpcheckport, $mb,
$boot_timeout) = @_;
@@ -2205,7 +2210,7 @@ sub prepareguest ($$) {
# If we have defined guest specific disksize, use it
$mb = guest_var($gho,'disksize',$mb);
if (defined $mb) {
- store_runvar("${gn}_disk_lv", $r{"${gn}_hostname"}.'-disk');
+ store_runvar("${gn}_disk_lv", guest_mk_lv_name($gho, '-disk'));
}
if (defined $mb) {
diff --git a/ts-debian-install b/ts-debian-install
index f07dd676..8caa9d76 100755
--- a/ts-debian-install
+++ b/ts-debian-install
@@ -100,7 +100,7 @@ END
my $cfg= "/etc/xen/$gho->{Name}.cfg";
store_runvar("$gho->{Guest}_cfgpath", $cfg);
-store_runvar("$gho->{Guest}_swap_lv", "$gho->{Name}-swap");
+store_runvar("$gho->{Guest}_swap_lv", guest_mk_lv_name($gho, "-swap"));
}
prep();
--
2.20.1
[OSSTEST PATCH 10/16] host reuse fixes: Fix running of steps adhoc
When a ts script is run by hand, for adhoc testing, there is no
OSSTEST_TESTID variable in the environment and the script does not
know it's own step number. Such adhoc runs are not tracked as steps
in the steps table.
For host lifecycle purposes, treat these as ad-hoc out-of-flight uses,
based only on the taskid (which will usually be a person's personal
static task).
Without this, these adhoc runs fail with a constraint violating trying
to insert a flight/job/step row into the host lifecycle table: the
constraint requires the step to be specified but it is NULL.
Signed-off-by: Ian Jackson
---
Osstest/JobDB/Executive.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Osstest/JobDB/Executive.pm b/Osstest/JobDB/Executive.pm
index 4fa42e5d..04555113 100644
--- a/Osstest/JobDB/Executive.pm
+++ b/Osstest/JobDB/Executive.pm
@@ -588,7 +588,7 @@ END
};
-if (!defined $flight) {
+if (!defined $flight || !defined $tstepno) {
db_retry($dbh_tests,[], sub {
$insertq->execute($hostname, $ttaskid,
undef,undef,
--
2.20.1
[OSSTEST PATCH 16/16] starvation: Do not count more than half a flight as starved
This seems like a sensible rule.
This also prevents the following bizarre behaviour: when a flight has
a handful of jobs that cannot be run at all (eg because it's a
commissioning flight for only hosts of a particular arch), those jobs
can complete quite quickly. Even with a high X value because only a
smallish portion of the flight has finished, this can lead to a modest
threshhold value. This combines particularly badly with commissioning
flights, where the duraation estimates are often nonsense.
Signed-off-by: Ian Jackson
---
ts-hosts-allocate-Executive | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index b216186a..459b9215 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -863,7 +863,7 @@ sub starving ($$) {
"D=%d W=%d X=%.3f t_D=%s t_me=%s t_lim=%.3f X'=%.4f (fi.s=%s)",
$d, $w, $X, $total_d, $projected_me, $lim, $Xcmp,
$fi->{started} - $now;
-my $bad = $projected_me > $lim;
+my $bad = $projected_me > $lim && $d >= $w;
return ($bad, $m);
}
--
2.20.1
[OSSTEST PATCH 11/16] host reuse fixes: Fix runvar entry for adhoc tasks
When processing an item from the host lifecycle table into the runvar,
we don't want to do all the processing of flight and job. Instead, we
should simply put the ? into the runvar.
Previously this would produce ?: which the flight reporting
code would choke on.
Signed-off-by: Ian Jackson
---
Osstest/JobDB/Executive.pm | 5 +
1 file changed, 5 insertions(+)
diff --git a/Osstest/JobDB/Executive.pm b/Osstest/JobDB/Executive.pm
index 04555113..1dcf55ff 100644
--- a/Osstest/JobDB/Executive.pm
+++ b/Osstest/JobDB/Executive.pm
@@ -649,6 +649,11 @@ END
}
next if $tj_seen{$oisprepmark.$otj}++;
+ if (!defined $o->{flight}) {
+ push @lifecycle, "$omarks$otj";
+ next;
+ }
+
if (!$omarks && !$olive && defined($o->{flight}) &&
$ho->{Shared} &&
$ho->{Shared}{Type} =~ m/^build-/ &&
--
2.20.1
[OSSTEST PATCH 14/16] reporting: Minor fix to reporting of tasks with no subtask
subtask can be NULL. If so, do not include it.
This change fixes a warning and a minor cosmetic defect.
Signed-off-by: Ian Jackson
---
Osstest/Executive.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index e3ab1dc3..d95d848d 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -427,7 +427,7 @@ sub report_rogue_task_description ($) {
my $info= "rogue task ";
$info .= " $arow->{type} $arow->{refkey}";
$info .= " ($arow->{comment})" if defined $arow->{comment};
-$info .= " $arow->{subtask}";
+$info .= " $arow->{subtask}" if defined $arow->{subtask};
$info .= " (user $arow->{username})";
return $info;
}
--
2.20.1
[OSSTEST PATCH 13/16] Prefix guest LV names with the job name
This means that a subsequent test which reuses the same host will not
use the same LVs. This is a good idea because reusing the same LV
names in a subsequent job means relying on the "ad hoc run" cleanup
code. This is a bad idea because that code is rarely tested.
And because, depending on the situation, the old LVs may even still be
in use. For example, in a pair test, the guest's LVs will still be
set up for use with nbd.
It seems better to fix this by using a fresh LV rather than adding
more teardown code.
The "wear limit" on host reuse is what prevents the disk filling up
with LVs from old guests.
ts-debian-fixup needs special handling, because Debian's xen-tools'
xen-create-image utility hardcodes its notion of LV name construction.
We need to rename the actual LVs (perhaps overwriting old ones from a
previous ad-hoc run) and also update the config.
Signed-off-by: Ian Jackson
---
Osstest/TestSupport.pm | 2 +-
ts-debian-fixup| 22 ++
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 12aaba79..9362a865 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -2179,7 +2179,7 @@ sub guest_var_commalist ($$) {
sub guest_mk_lv_name ($$) {
my ($gho, $suffix) = @_;
-return "$gho->{Name}".$suffix;
+return $job."_$gho->{Name}".$suffix;
}
sub prepareguest ($$) {
diff --git a/ts-debian-fixup b/ts-debian-fixup
index a878fe50..810b3aba 100755
--- a/ts-debian-fixup
+++ b/ts-debian-fixup
@@ -37,6 +37,27 @@ sub savecfg () {
$cfg= get_filecontents("$cfgstash.orig");
}
+sub lvnames () {
+my $lvs = target_cmd_output_root($ho, "lvdisplay --colon", 30);
+foreach my $suffix (qw(disk swap)) {
+ my $old = "$gho->{Name}-$suffix";
+ my $new = "${job}_${old}";
+ my $full_old = "/dev/$gho->{Vg}/$old";
+ my $full_new = "/dev/$gho->{Vg}/$new";
+ $cfg =~ s{\Q$full_old\E(?![0-9a-zA-Z/_.-])}{
+logm "Replacing in domain config \`$&' with \`$full_new'";
+$full_new;
+}ge;
+ if ($lvs =~ m{^ *\Q$full_old\E}m) {
+ if ($lvs =~ m{^ *\Q$full_new\E}m) {
+ # In case we are re-running (eg, adhoc)
+ target_cmd_root($ho, "lvremove -f $full_new", 30);
+ }
+ target_cmd_root($ho, "lvrename $full_old $new", 30);
+ }
+}
+}
+
sub ether () {
#$cfg =~ s/^ [ \t]*
#( vif [ \t]* \= [ \t]* \[ [ \t]* [\'\"]
@@ -207,6 +228,7 @@ sub writecfg () {
}
savecfg();
+lvnames();
ether();
access();
$console = target_setup_rootdev_console_inittab($ho,$gho,"$mountpoint");
--
2.20.1
[OSSTEST PATCH 15/16] host reuse fixes: Do not break host-reuse if no host allocated
If host allocation failed, or our dependency jobs failed, then we
won't have allocated a host. The host runvar will not be set.
In this case, we want to do nothing.
But we forgot to pass $noneok to selecthost.
Signed-off-by: Ian Jackson
---
ts-host-reuse | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ts-host-reuse b/ts-host-reuse
index e2498bb6..b885a3e6 100755
--- a/ts-host-reuse
+++ b/ts-host-reuse
@@ -165,7 +165,7 @@ sub act_start_test () {
sub act_final () {
if (!@ARGV) {
- $ho = selecthost($whhost);
+ $ho = selecthost($whhost, 1);
return unless $ho;
host_update_lifecycle_info($ho, 'final');
} elsif ("@ARGV" eq "--post-test-ok") {
--
2.20.1
Re: [PATCH] xen/acpi: Don't fail if SPCR table is absent
On Thu, Oct 22, 2020 at 09:42:17AM +0200, Jan Beulich wrote:
> On 22.10.2020 00:12, Elliott Mitchell wrote:
> > --- a/xen/arch/arm/acpi/domain_build.c
> > +++ b/xen/arch/arm/acpi/domain_build.c
> > @@ -42,17 +42,18 @@ static int __init acpi_iomem_deny_access(struct domain
> > *d)
> > status = acpi_get_table(ACPI_SIG_SPCR, 0,
> > (struct acpi_table_header **)&spcr);
> >
> > -if ( ACPI_FAILURE(status) )
> > +if ( ACPI_SUCCESS(status) )
> > {
> > -printk("Failed to get SPCR table\n");
> > -return -EINVAL;
> > +mfn = spcr->serial_port.address >> PAGE_SHIFT;
> > +/* Deny MMIO access for UART */
> > +rc = iomem_deny_access(d, mfn, mfn + 1);
> > +if ( rc )
> > +return rc;
> > +}
> > +else
> > +{
> > +printk("Failed to get SPCR table, Xen console may be
> > unavailable\n");
> > }
>
> Nit: While I see you've got Stefano's R-b already, I Xen we typically
> omit the braces here.
Personally, I prefer that myself, but was unsure of the preference here.
I've seen multiple projects which *really* dislike using having brackets
for some clauses, but not others (ie they want either all clauses with or
all clauses without; instead of only if required).
I sent what I thought was the more often used format. (I also like tabs,
and dislike having so many spaces; alas my preferences are apparently
uncommon)
--
(\___(\___(\__ --=> 8-) EHM <=-- __/)___/)___/)
\BS (| ehem+sig...@m5p.com PGP 87145445 |) /
\_CS\ | _ -O #include O- _ | / _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
[linux-linus test] 156079: regressions - trouble: broken/fail/pass
flight 156079 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/156079/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-xl-credit1 broken test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-xsm7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-i386-libvirt-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-coresched-i386-xl 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-amd64-i386-libvirt 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl7 xen-install fail REGR. vs. 152332 test-amd64-i386-examine 6 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-raw7 xen-install fail REGR. vs. 152332 test-amd64-i386-freebsd10-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-pvshim 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-freebsd10-i386 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-arm64-arm64-xl 8 xen-boot fail REGR. vs. 152332 test-arm64-arm64-xl-xsm 8 xen-boot fail REGR. vs. 152332 test-arm64-arm64-examine 8 reboot fail REGR. vs. 152332 test-arm64-arm64-xl-credit2 12 debian-install fail REGR. vs. 152332 test-armhf-armhf-xl-credit2 8 xen-boot fail REGR. vs. 152332 test-armhf-armhf-libvirt 8 xen-boot fail REGR. vs. 152332 test-armhf-armhf-examine 8 reboot fail REGR. vs. 152332 test-armhf-armhf-xl-cubietruck 8 xen-boot fail REGR. vs. 152332 test-armhf-armhf-xl-multivcpu 8 xen-bootfail REGR. vs. 152332 test-arm64-arm64-xl-seattle 8 xen-boot fail REGR. vs. 152332 test-armhf-armhf-xl-credit1 8 xen-boot fail REGR. vs. 152332 test-armhf-armhf-xl 8 xen-boot fail REGR. vs. 152332 test-armhf-armhf-xl-vhd 8 xen-boot fail REGR. vs. 152332 test-armhf-armhf-libvirt-raw 8 xen-boot fail REGR. vs. 152332 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 8 xen-boot fail REGR. vs. 152332 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-credit1 5 host-install(5) broken blocked in 152332 test-arm64-arm64-libvirt-xsm 11 leak-check/basis(11)fail blocked in 152332 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 152332 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 152332 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 152332 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152332 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 152332 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16
Re: [PATCH] xen/acpi: Don't fail if SPCR table is absent
Hi Elliott,
On 22/10/2020 18:13, Elliott Mitchell wrote:
On Thu, Oct 22, 2020 at 09:42:17AM +0200, Jan Beulich wrote:
On 22.10.2020 00:12, Elliott Mitchell wrote:
--- a/xen/arch/arm/acpi/domain_build.c
+++ b/xen/arch/arm/acpi/domain_build.c
@@ -42,17 +42,18 @@ static int __init acpi_iomem_deny_access(struct domain *d)
status = acpi_get_table(ACPI_SIG_SPCR, 0,
(struct acpi_table_header **)&spcr);
-if ( ACPI_FAILURE(status) )
+if ( ACPI_SUCCESS(status) )
{
-printk("Failed to get SPCR table\n");
-return -EINVAL;
+mfn = spcr->serial_port.address >> PAGE_SHIFT;
+/* Deny MMIO access for UART */
+rc = iomem_deny_access(d, mfn, mfn + 1);
+if ( rc )
+return rc;
+}
+else
+{
+printk("Failed to get SPCR table, Xen console may be unavailable\n");
}
Nit: While I see you've got Stefano's R-b already, I Xen we typically
omit the braces here.
Personally, I prefer that myself, but was unsure of the preference here.
I don't think we are very consistent here... I would not add them
myself, but I don't particularly mind them (I know some editors will add
them automatically).
I will keep them while committing. For the patch:
Acked-by: Julien Grall
I've seen multiple projects which *really* dislike using having brackets
for some clauses, but not others (ie they want either all clauses with or
all clauses without; instead of only if required).
I sent what I thought was the more often used format. (I also like tabs,
and dislike having so many spaces; alas my preferences are apparently
uncommon)
We have a few files in Xen using tabs (yes, we like mixing coding style!).
Cheers,
--
Julien Grall
[qemu-mainline test] 156100: regressions - FAIL
flight 156100 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/156100/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 152631 build-amd64 6 xen-buildfail REGR. vs. 152631 build-arm64 6 xen-buildfail REGR. vs. 152631 build-arm64-xsm 6 xen-buildfail REGR. vs. 152631 build-i3866 xen-buildfail REGR. vs. 152631 build-i386-xsm6 xen-buildfail REGR. vs. 152631 build-armhf 6 xen-buildfail REGR. vs. 152631 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit1 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-coresched-i386-xl 1 build-check(1) blocked n/a test-amd64-coresched-amd64-xl 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-shadow1 build-check(1) blocked n/a test-amd64-amd64-xl-rtds 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-amd 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvshim1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-intel 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-intel 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-amd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-freebsd11-amd64 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-freebsd12-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit1 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-amd 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-inte
Re: [PATCH] xen/acpi: Don't fail if SPCR table is absent
Hi,
Thank you for the patch. FIY I tweak a bit the commit title before
committing.
The title is now: "xen/arm: acpi: Don't fail it SPCR table is absent".
Cheers,
On 21/10/2020 23:12, Elliott Mitchell wrote:
Absence of a SPCR table likely means the console is a framebuffer. In
such case acpi_iomem_deny_access() should NOT fail.
Signed-off-by: Elliott Mitchell
---
xen/arch/arm/acpi/domain_build.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/xen/arch/arm/acpi/domain_build.c b/xen/arch/arm/acpi/domain_build.c
index 1b1cfabb00..bbdc90f92c 100644
--- a/xen/arch/arm/acpi/domain_build.c
+++ b/xen/arch/arm/acpi/domain_build.c
@@ -42,17 +42,18 @@ static int __init acpi_iomem_deny_access(struct domain *d)
status = acpi_get_table(ACPI_SIG_SPCR, 0,
(struct acpi_table_header **)&spcr);
-if ( ACPI_FAILURE(status) )
+if ( ACPI_SUCCESS(status) )
{
-printk("Failed to get SPCR table\n");
-return -EINVAL;
+mfn = spcr->serial_port.address >> PAGE_SHIFT;
+/* Deny MMIO access for UART */
+rc = iomem_deny_access(d, mfn, mfn + 1);
+if ( rc )
+return rc;
+}
+else
+{
+printk("Failed to get SPCR table, Xen console may be unavailable\n");
}
-
-mfn = spcr->serial_port.address >> PAGE_SHIFT;
-/* Deny MMIO access for UART */
-rc = iomem_deny_access(d, mfn, mfn + 1);
-if ( rc )
-return rc;
/* Deny MMIO access for GIC regions */
return gic_iomem_deny_access(d);
--
Julien Grall
Re: [PATCH] xen/acpi: Don't fail if SPCR table is absent
On Thu, Oct 22, 2020 at 07:38:26PM +0100, Julien Grall wrote:
> I don't think we are very consistent here... I would not add them
> myself, but I don't particularly mind them (I know some editors will add
> them automatically).
>
> I will keep them while committing. For the patch:
I would tend to remove them on commit since I dislike them. Just as
stated, I was unsure.
On default settings, clang-format will object to:
if(thing)
{
foo
}
else
bar
Or
if(thing)
foo
else
{
bar
}
I *like* those formats, but was under the impression most people did not.
The indentation is the more visually obvious indicator, just the compiler
actually uses the brackets. As such I *like* the misleading indentation
warnings as those seemed to have a fairly high true-positive rate.
On Thu, Oct 22, 2020 at 07:44:26PM +0100, Julien Grall wrote:
> Thank you for the patch. FIY I tweak a bit the commit title before
> committing.
>
> The title is now: "xen/arm: acpi: Don't fail it SPCR table is absent".
Perhaps "xen/arm: acpi: Don't fail on absent SPCR table"?
What you're suggesting doesn't read well to me.
--
(\___(\___(\__ --=> 8-) EHM <=-- __/)___/)___/)
\BS (| ehem+sig...@m5p.com PGP 87145445 |) /
\_CS\ | _ -O #include O- _ | / _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
Re: [PATCH] xen/arm: Remove EXPERT dependancy
On Thu, 22 Oct 2020, Julien Grall wrote: > On 22/10/2020 02:43, Elliott Mitchell wrote: > > Linux requires UEFI support to be enabled on ARM64 devices. While many > > ARM64 devices lack ACPI, the writing seems to be on the wall of UEFI/ACPI > > potentially taking over. Some common devices may need ACPI table > > support. > > > > Presently I think it is worth removing the dependancy on CONFIG_EXPERT. > > The idea behind EXPERT is to gate any feature that is not considered to be > stable/complete enough to be used in production. Yes, and from that point of view I don't think we want to remove EXPERT from ACPI yet. However, the idea of hiding things behind EXPERT works very well for new esoteric features, something like memory introspection or memory overcommit. It does not work well for things that are actually required to boot on the platform. Typically ACPI systems don't come with device tree at all (RPi4 being an exception), so users don't really have much of a choice in the matter. >From that point of view, it would be better to remove EXPERT from ACPI, maybe even build ACPI by default, *but* to add a warning at boot saying something like: "ACPI support is experimental. Boot using Device Tree if you can." That would better convey the risks of using ACPI, while at the same time making it a bit easier for users to boot on their ACPI-only platforms. > I don't consider the ACPI complete because the parsing of the IORT (used to > discover SMMU and GICv3 ITS) is not there yet. > > I vaguely remember some issues on system using SMMU (e.g. Thunder-X) because > Dom0 will try to use the IOMMU and this would break PV drivers. I am not sure why Dom0 using the IOMMU would break PV drivers? Is it because the pagetable is not properly updated when mapping foreign pages? > Therefore I think we at least want to consider to hide SMMUs from dom0 before > removing EXPERT. Ideally, I would also like the feature to be tested in > Osstest. > > The good news is Xen Project already has systems (e.g. Thunder-X, Softiron) > that can supported ACPI. So it should hopefully be a matter to tell them to > boot with ACPI rather than DT. I agree that we want to keep ACPI "expert/experimental" given its current state but maybe we can find a better way to carry that message than to set EXPERT in Kconfig. And yes, if we wanted to make ACPI less "expert/experimental" we definitely need some testing in OSSTest and any critical bugs (e.g. PV drivers not working) addressed.
[xen-unstable-smoke test] 156108: tolerable all pass - PUSHED
flight 156108 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/156108/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 861f0c110976fa8879b7bf63d9478b6be83d4ab6 baseline version: xen 3b49791e4cc2f38dd84bf331b75217adaef636e3 Last test of basis 156047 2020-10-20 21:00:31 Z2 days Testing same since 156108 2020-10-22 19:02:27 Z0 days1 attempts People who touched revisions under test: Elliott Mitchell Julien Grall jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 3b49791e4c..861f0c1109 861f0c110976fa8879b7bf63d9478b6be83d4ab6 -> smoke
Re: [PATCH] xen/arm: Remove EXPERT dependancy
On Thu, Oct 22, 2020 at 02:17:13PM -0700, Stefano Stabellini wrote: > On Thu, 22 Oct 2020, Julien Grall wrote: > > On 22/10/2020 02:43, Elliott Mitchell wrote: > > > Linux requires UEFI support to be enabled on ARM64 devices. While many > > > ARM64 devices lack ACPI, the writing seems to be on the wall of UEFI/ACPI > > > potentially taking over. Some common devices may need ACPI table > > > support. > > > > > > Presently I think it is worth removing the dependancy on CONFIG_EXPERT. > > > > The idea behind EXPERT is to gate any feature that is not considered to be > > stable/complete enough to be used in production. > > Yes, and from that point of view I don't think we want to remove EXPERT > from ACPI yet. However, the idea of hiding things behind EXPERT works > very well for new esoteric features, something like memory introspection > or memory overcommit. It does not work well for things that are actually > required to boot on the platform. > > Typically ACPI systems don't come with device tree at all (RPi4 being an > exception), so users don't really have much of a choice in the matter. > > >From that point of view, it would be better to remove EXPERT from ACPI, > maybe even build ACPI by default, *but* to add a warning at boot saying > something like: > > "ACPI support is experimental. Boot using Device Tree if you can." > > > That would better convey the risks of using ACPI, while at the same time > making it a bit easier for users to boot on their ACPI-only platforms. This matches my view. I was thinking about including "default y", but I felt the chances of that getting through were lower. I concur with a warning on boot being a good approach. -- (\___(\___(\__ --=> 8-) EHM <=-- __/)___/)___/) \BS (| ehem+sig...@m5p.com PGP 87145445 |) / \_CS\ | _ -O #include O- _ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
BUG: credit=sched2 machine hang when using DRAKVUF
Hello, when using DRAKVUF against a Windows 7 x64 DomU, the whole machine hangs after a few minutes. The chance for a hang seems to be correlated with number of PCPUs, in this case we have 14 PCPUs and hang is very easily reproducible, while on other machines with 2-4 PCPUs it's very rare (but still occurring sometimes). The issue is observed with the default sched=credit2 and is no longer reproducible once sched=credit is set. Enclosed: panic log from my Dom0. Best regards, Michał Leszczyński CERT Polska paź 22 12:20:50 hostname kernel: rcu: INFO: rcu_sched self-detected stall on CPU paź 22 12:20:50 hostname kernel: rcu: 3-: (21002 ticks this GP) idle=7e2/1/0x4002 softirq=61729/61729 fqs=10490 paź 22 12:20:50 hostname kernel: rcu: (t=21003 jiffies g=36437 q=9406) paź 22 12:20:50 hostname kernel: NMI backtrace for cpu 3 paź 22 12:20:50 hostname kernel: CPU: 3 PID: 4153 Comm: drakvuf Tainted: P OEL4.19.0-6-amd64 #1 Debian 4.19.67-2+deb10u2 paź 22 12:20:50 hostname kernel: Hardware name: Dell Inc. PowerEdge R640/08HT8T, BIOS 2.1.8 04/30/2019 paź 22 12:20:50 hostname kernel: Call Trace: paź 22 12:20:50 hostname kernel: paź 22 12:20:50 hostname kernel: dump_stack+0x5c/0x80 paź 22 12:20:50 hostname kernel: nmi_cpu_backtrace.cold.4+0x13/0x50 paź 22 12:20:50 hostname kernel: ? lapic_can_unplug_cpu.cold.29+0x3b/0x3b paź 22 12:20:50 hostname kernel: nmi_trigger_cpumask_backtrace+0xf9/0xfb paź 22 12:20:50 hostname kernel: rcu_dump_cpu_stacks+0x9b/0xcb paź 22 12:20:50 hostname kernel: rcu_check_callbacks.cold.81+0x1db/0x335 paź 22 12:20:50 hostname kernel: ? tick_sched_do_timer+0x60/0x60 paź 22 12:20:50 hostname kernel: update_process_times+0x28/0x60 paź 22 12:20:50 hostname kernel: tick_sched_handle+0x22/0x60 paź 22 12:20:50 hostname kernel: tick_sched_timer+0x37/0x70 paź 22 12:20:50 hostname kernel: __hrtimer_run_queues+0x100/0x280 paź 22 12:20:50 hostname kernel: hrtimer_interrupt+0x100/0x220 paź 22 12:20:50 hostname kernel: xen_timer_interrupt+0x1e/0x30 paź 22 12:20:50 hostname kernel: __handle_irq_event_percpu+0x46/0x190 paź 22 12:20:50 hostname kernel: handle_irq_event_percpu+0x30/0x80 paź 22 12:20:50 hostname kernel: handle_percpu_irq+0x40/0x60 paź 22 12:20:50 hostname kernel: generic_handle_irq+0x27/0x30 paź 22 12:20:50 hostname kernel: __evtchn_fifo_handle_events+0x17d/0x190 paź 22 12:20:50 hostname kernel: __xen_evtchn_do_upcall+0x42/0x80 paź 22 12:20:50 hostname kernel: xen_evtchn_do_upcall+0x27/0x40 paź 22 12:20:50 hostname kernel: xen_do_hypervisor_callback+0x29/0x40 paź 22 12:20:50 hostname kernel: paź 22 12:20:50 hostname kernel: RIP: e030:smp_call_function_single+0xce/0xf0 paź 22 12:20:50 hostname kernel: Code: 8b 4c 24 38 65 48 33 0c 25 28 00 00 00 75 34 c9 c3 48 89 d1 48 89 f2 48 89 e6 e8 6d fe ff ff 8b 54 24 18 83 e2 01 74 0b f3 90 <8b> 54 24 18 8 3 e2 01 75 f5 eb ca 8b 05 b9 99 4d 01 85 c0 75 88 0f paź 22 12:20:50 hostname kernel: RSP: e02b:c9004713bd00 EFLAGS: 0202 paź 22 12:20:50 hostname kernel: RAX: RBX: 888b0b6eea40 RCX: 0200 paź 22 12:20:50 hostname kernel: RDX: 0001 RSI: 8212e4a0 RDI: 81c2dec0 paź 22 12:20:50 hostname kernel: RBP: c9004713bd50 R08: R09: 888c54052480 paź 22 12:20:50 hostname kernel: R10: 888c540524a8 R11: R12: c9004713bd60 paź 22 12:20:50 hostname kernel: R13: 8000 R14: 8000 R15: 888b0b6eeab0 paź 22 12:20:50 hostname kernel: ? xen_pgd_alloc+0x110/0x110 paź 22 12:20:50 hostname kernel: xen_exit_mmap+0xaa/0x100 paź 22 12:20:50 hostname kernel: exit_mmap+0x64/0x180 paź 22 12:20:50 hostname kernel: ? __raw_spin_unlock+0x5/0x10 paź 22 12:20:50 hostname kernel: ? __handle_mm_fault+0x1090/0x1270 paź 22 12:20:50 hostname kernel: ? _raw_spin_unlock_irqrestore+0x14/0x20 paź 22 12:20:50 hostname kernel: ? exit_robust_list+0x5b/0x130 paź 22 12:20:50 hostname kernel: mmput+0x54/0x130 paź 22 12:20:50 hostname kernel: do_exit+0x290/0xb90 paź 22 12:20:50 hostname kernel: ? handle_mm_fault+0xd6/0x200 paź 22 12:20:50 hostname kernel: do_group_exit+0x3a/0xa0 paź 22 12:20:50 hostname kernel: __x64_sys_exit_group+0x14/0x20 paź 22 12:20:50 hostname kernel: do_syscall_64+0x53/0x110 paź 22 12:20:50 hostname kernel: entry_SYSCALL_64_after_hwframe+0x44/0xa9 paź 22 12:20:50 hostname kernel: RIP: 0033:0x7f98d23ec9d6 paź 22 12:20:50 hostname kernel: Code: Bad RIP value. paź 22 12:20:50 hostname kernel: RSP: 002b:7ffc4a0327f8 EFLAGS: 0246 ORIG_RAX: 00e7 paź 22 12:20:50 hostname kernel: RAX: ffda RBX: 7f98d24dd760 RCX: 7f98d23ec9d6 paź 22 12:20:50 hostname kernel: RDX: RSI: 003c RDI: paź 22 12:20:50 hostname kernel: RBP: R08: 00e7 R09: ff60 paź 22 12:20:50 hostname kernel: R10: R11: 0246 R12:
Re: [XEN PATCH v1] xen/arm : Add support for SMMUv3 driver
On Thu, 22 Oct 2020, Julien Grall wrote: > > > On 20/10/2020 16:25, Rahul Singh wrote: > > > > Add support for ARM architected SMMUv3 implementations. It is based on > > > > the Linux SMMUv3 driver. > > > > Major differences between the Linux driver are as follows: > > > > 1. Only Stage-2 translation is supported as compared to the Linux driver > > > > that supports both Stage-1 and Stage-2 translations. > > > > 2. Use P2M page table instead of creating one as SMMUv3 has the > > > > capability to share the page tables with the CPU. > > > > 3. Tasklets is used in place of threaded IRQ's in Linux for event queue > > > > and priority queue IRQ handling. > > > > > > Tasklets are not a replacement for threaded IRQ. In particular, they will > > > have priority over anything else (IOW nothing will run on the pCPU until > > > they are done). > > > > > > Do you know why Linux is using thread. Is it because of long running > > > operations? > > > > Yes you are right because of long running operations Linux is using the > > threaded IRQs. > > > > SMMUv3 reports fault/events bases on memory-based circular buffer queues not > > based on the register. As per my understanding, it is time-consuming to > > process the memory based queues in interrupt context because of that Linux > > is using threaded IRQ to process the faults/events from SMMU. > > > > I didn’t find any other solution in XEN in place of tasklet to defer the > > work, that’s why I used tasklet in XEN in replacement of threaded IRQs. If > > we do all work in interrupt context we will make XEN less responsive. > > So we need to make sure that Xen continue to receives interrupts, but we also > need to make sure that a vCPU bound to the pCPU is also responsive. > > > > > If you know another solution in XEN that will be used to defer the work in > > the interrupt please let me know I will try to use that. > > One of my work colleague encountered a similar problem recently. He had a long > running tasklet and wanted to be broken down in smaller chunk. > > We decided to use a timer to reschedule the taslket in the future. This allows > the scheduler to run other loads (e.g. vCPU) for some time. > > This is pretty hackish but I couldn't find a better solution as tasklet have > high priority. > > Maybe the other will have a better idea. Julien's suggestion is a good one. But I think tasklets can be configured to be called from the idle_loop, in which case they are not run in interrupt context? Still, tasklets run until completion in Xen, which could take too long. The code has to voluntarily release control of the execution flow once it realizes it has been running for too long. The rescheduling via a timer works. Now, to brainstorm other possible alternatives, for hypercalls we have been using hypercall continuations. Continuations is a way to break a hypercall implementation that takes too long into multiple execution chunks. It works by calling into itself again: making the same hypercall again with updated arguments, so that the scheduler has a chance to do other operations in between, including running other tasklets and softirqs. That works well because the source of the work is a guest request, specifically a hypercall. However, in the case of the SMMU driver, there is no hypercall. The Xen driver has to do work in response to an interrupt and the work is not tied to one particular domain. So I don't think the hypercall continuation model could work here. The timer seems to be the best option. > > > > 4. Latest version of the Linux SMMUv3 code implements the commands queue > > > > access functions based on atomic operations implemented in Linux. > > > > > > Can you provide more details? > > > > I tried to port the latest version of the SMMUv3 code than I observed that > > in order to port that code I have to also port atomic operation implemented > > in Linux to XEN. As latest Linux code uses atomic operation to process the > > command queues (atomic_cond_read_relaxed(),atomic_long_cond_read_relaxed() , > > atomic_fetch_andnot_relaxed()) . > > Thank you for the explanation. I think it would be best to import the atomic > helpers and use the latest code. > > This will ensure that we don't re-introduce bugs and also buy us some time > before the Linux and Xen driver diverge again too much. > > Stefano, what do you think? I think you are right.
[qemu-mainline test] 156109: regressions - FAIL
flight 156109 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/156109/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 152631 build-amd64 6 xen-buildfail REGR. vs. 152631 build-arm64 6 xen-buildfail REGR. vs. 152631 build-arm64-xsm 6 xen-buildfail REGR. vs. 152631 build-i3866 xen-buildfail REGR. vs. 152631 build-i386-xsm6 xen-buildfail REGR. vs. 152631 build-armhf 6 xen-buildfail REGR. vs. 152631 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit1 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-coresched-i386-xl 1 build-check(1) blocked n/a test-amd64-coresched-amd64-xl 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-shadow1 build-check(1) blocked n/a test-amd64-amd64-xl-rtds 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-amd 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvshim1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-intel 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-intel 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-amd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-freebsd11-amd64 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-freebsd12-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit1 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-amd 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-inte
[ovmf test] 156102: all pass - PUSHED
flight 156102 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/156102/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 264eccb5dfc345c2e004883f00e62959f818fafd baseline version: ovmf 24cf72726564eac7ba9abb24f3d05795164d0a70 Last test of basis 156091 2020-10-22 14:14:55 Z0 days Testing same since 156102 2020-10-22 17:10:41 Z0 days1 attempts People who touched revisions under test: Michael Kubacki jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 24cf727265..264eccb5df 264eccb5dfc345c2e004883f00e62959f818fafd -> xen-tested-master
[qemu-mainline test] 156110: regressions - FAIL
flight 156110 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/156110/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 152631 build-amd64 6 xen-buildfail REGR. vs. 152631 build-arm64 6 xen-buildfail REGR. vs. 152631 build-arm64-xsm 6 xen-buildfail REGR. vs. 152631 build-i3866 xen-buildfail REGR. vs. 152631 build-i386-xsm6 xen-buildfail REGR. vs. 152631 build-armhf 6 xen-buildfail REGR. vs. 152631 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit1 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-coresched-i386-xl 1 build-check(1) blocked n/a test-amd64-coresched-amd64-xl 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-shadow1 build-check(1) blocked n/a test-amd64-amd64-xl-rtds 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-amd 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvshim1 build-check(1) blocked n/a test-amd64-amd64-dom0pvh-xl-intel 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-intel 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-amd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-freebsd11-amd64 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-freebsd12-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit1 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-amd 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-inte
[PATCH] xen/arm: ACPI: Remove EXPERT dependancy, default on for ARM64
Linux requires UEFI support to be enabled on ARM64 devices. While many
ARM64 devices lack ACPI, the writing seems to be on the wall of UEFI/ACPI
potentially taking over. Some common devices may require ACPI table
support to boot.
For devices which can boot in either mode, continue defaulting to
device-tree. Add warnings about using ACPI advising users of present
situation.
Signed-off-by: Elliott Mitchell
---
Okay, hopefully this is okay. Warning in Kconfig, warning on boot.
Perhaps "default y if ARM_64" is redundant, yet if someone tries to make
it possible to boot aarch32 on a ACPI machine...
I also want a date in the message. Theory is this won't be there
forever, so a date is essential.
---
xen/arch/arm/Kconfig | 7 ++-
xen/arch/arm/acpi/boot.c | 9 +
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 2777388265..29624d03fa 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -32,13 +32,18 @@ menu "Architecture Features"
source "arch/Kconfig"
config ACPI
- bool "ACPI (Advanced Configuration and Power Interface) Support" if
EXPERT
+ bool "ACPI (Advanced Configuration and Power Interface) Support"
depends on ARM_64
+ default y if ARM_64
---help---
Advanced Configuration and Power Interface (ACPI) support for Xen is
an alternative to device tree on ARM64.
+ Note this is presently EXPERIMENTAL. If a given device has both
+ device-tree and ACPI support, it is presently (October 2020)
+ recommended to boot using the device-tree.
+
config GICV3
bool "GICv3 driver"
depends on ARM_64 && !NEW_VGIC
diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index 30e4bd1bc5..c0e8f85325 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -254,6 +254,15 @@ int __init acpi_boot_table_init(void)
dt_scan_depth1_nodes, NULL) )
goto disable;
+printk("\n"
+"*\n"
+"*WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING*\n"
+"* *\n"
+"* Xen-ARM ACPI support is EXPERIMENTAL. It is presently (October 2020) *\n"
+"* recommended you boot your system in device-tree mode if you can. *\n"
+"*\n"
+"\n");
+
/*
* ACPI is disabled at this point. Enable it in order to parse
* the ACPI tables.
--
2.20.1
--
(\___(\___(\__ --=> 8-) EHM <=-- __/)___/)___/)
\BS (| ehem+sig...@m5p.com PGP 87145445 |) /
\_CS\ | _ -O #include O- _ | / _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
