Re: [Xen-devel] [v7][PATCH 05/16] hvmloader: get guest memory map into memory_map[]

[Chen, Tiejun]

On 2015/7/10 21:49, George Dunlap wrote:

On Thu, Jul 9, 2015 at 6:33 AM, Tiejun Chen  wrote:

Now we get this map layout by call XENMEM_memory_map then
save them into one global variable memory_map[]. It should
include lowmem range, rdm range and highmem range. Note
rdm range and highmem range may not exist in some cases.

And here we need to check if any reserved memory conflicts with
[RESERVED_MEMORY_DYNAMIC_START - 1, RESERVED_MEMORY_DYNAMIC_END].
This range is used to allocate memory in hvmloder level, and
we would lead hvmloader failed in case of conflict since its
another rare possibility in real world.

CC: Keir Fraser 
CC: Jan Beulich 
CC: Andrew Cooper 
CC: Ian Jackson 
CC: Stefano Stabellini 
CC: Ian Campbell 
CC: Wei Liu 
Signed-off-by: Tiejun Chen 
Reviewed-by: Kevin Tian 
---
v5 ~ v7:

* Nothing is changed.

v4:

* Move some codes related to e820 to that specific file, e820.c.

* Consolidate "printf()+BUG()" and "BUG_ON()"

* Avoid another fixed width type for the parameter of get_mem_mapping_layout()

  tools/firmware/hvmloader/e820.c  | 35 +++
  tools/firmware/hvmloader/e820.h  |  7 +++
  tools/firmware/hvmloader/hvmloader.c |  2 ++
  tools/firmware/hvmloader/util.c  | 26 ++
  tools/firmware/hvmloader/util.h  | 12 
  5 files changed, 82 insertions(+)

diff --git a/tools/firmware/hvmloader/e820.c b/tools/firmware/hvmloader/e820.c
index 2e05e93..3e53c47 100644
--- a/tools/firmware/hvmloader/e820.c
+++ b/tools/firmware/hvmloader/e820.c
@@ -23,6 +23,41 @@
  #include "config.h"
  #include "util.h"

+struct e820map memory_map;
+
+void memory_map_setup(void)
+{
+unsigned int nr_entries = E820MAX, i;
+int rc;
+uint64_t alloc_addr = RESERVED_MEMORY_DYNAMIC_START - 1;
+uint64_t alloc_size = RESERVED_MEMORY_DYNAMIC_END - alloc_addr;


Why START-1 rather than just START?


I also think this is wrong after I double check this point. This two 
lines seems be copied simply from another place where we're allocating 
space based on RESERVED_MEMORY_DYNAMIC_{START, END}. But here I think 
you're right.


So let me correct this and update the patch description.

Thanks
Tiejun



It looks like RESERVED_MEMORY_DYNAMIC_START is set to 0xFC001000.  In
the code the way it is, if there is an RMRR from 0xFC00 of size
0x1000, it looks like check_overlap() below will fail and hvmloader
will BUG().

Is that really what we want?  Why can we not have an RMRR range that
goes right up to the edge of the reserved range?

Other than that this patch looks good.

  -George


+
+rc = get_mem_mapping_layout(memory_map.map, &nr_entries);
+
+if ( rc || !nr_entries )
+{
+printf("Get guest memory maps[%d] failed. (%d)\n", nr_entries, rc);
+BUG();
+}
+
+memory_map.nr_map = nr_entries;
+
+for ( i = 0; i < nr_entries; i++ )
+{
+if ( memory_map.map[i].type == E820_RESERVED )
+{
+if ( check_overlap(alloc_addr, alloc_size,
+   memory_map.map[i].addr,
+   memory_map.map[i].size) )
+{
+printf("Fail to setup memory map due to conflict");
+printf(" on dynamic reserved memory range.\n");
+BUG();
+}
+}
+}
+}
+
  void dump_e820_table(struct e820entry *e820, unsigned int nr)
  {
  uint64_t last_end = 0, start, end;
diff --git a/tools/firmware/hvmloader/e820.h b/tools/firmware/hvmloader/e820.h
index b2ead7f..8b5a9e0 100644
--- a/tools/firmware/hvmloader/e820.h
+++ b/tools/firmware/hvmloader/e820.h
@@ -15,6 +15,13 @@ struct e820entry {
  uint32_t type;
  } __attribute__((packed));

+#define E820MAX128
+
+struct e820map {
+unsigned int nr_map;
+struct e820entry map[E820MAX];
+};
+
  #endif /* __HVMLOADER_E820_H__ */

  /*
diff --git a/tools/firmware/hvmloader/hvmloader.c 
b/tools/firmware/hvmloader/hvmloader.c
index 25b7f08..84c588c 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -262,6 +262,8 @@ int main(void)

  init_hypercalls();

+memory_map_setup();
+
  xenbus_setup();

  bios = detect_bios();
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 80d822f..122e3fa 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -27,6 +27,17 @@
  #include 
  #include 

+/*
+ * Check whether there exists overlap in the specified memory range.
+ * Returns true if exists, else returns false.
+ */
+bool check_overlap(uint64_t start, uint64_t size,
+   uint64_t reserved_start, uint64_t reserved_size)
+{
+return (start + size > reserved_start) &&
+(start < reserved_start + reserved_size);
+}
+
  void wrmsr(uint32_t idx, uint64_t v)
  {
  asm volatile (
@@ -368,6 +379,21 @@ uuid_to_string(char *dest, uint8_t *uuid)
  *p = '\0';
  }

+int get_mem_mapping_layout(st

Re: [Xen-devel] [PATCH v4 07/15] VMX: add VMFUNC leaf 0 (EPTP switching) to emulator.

[Jan Beulich]

>>> On 11.07.15 at 22:01,  wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>>Sent: Friday, July 10, 2015 2:31 AM
>>
> On 10.07.15 at 02:52,  wrote:
>>> @@ -3234,6 +3256,13 @@ void vmx_vmexit_handler(struct cpu_user_regs
>>*regs)
>>>  update_guest_eip();
>>>  break;
>>>
>>> +case EXIT_REASON_VMFUNC:
>>> +if ( vmx_vmfunc_intercept(regs) == X86EMUL_EXCEPTION )
>>> +hvm_inject_hw_exception(TRAP_invalid_op,
>>HVM_DELIVER_NO_ERROR_CODE);
>>> +else
>>> +update_guest_eip();
>>> +break;
>>
>>How about X86EMUL_UNHANDLEABLE and X86EMUL_RETRY? As said before,
>>either get this right, or simply fold the relatively pointless helper into 
> here.
> 
> Sure I can add the other error conditions but note that they will be handled 
> as EXCEPTION.

The reason for this would need to go into ...

> Let me explain the point of the relatively pointless :-) helper 
> was to have the interface complete so that if someone in the future wanted to 
> handle VMFUNC exits (perhaps for lazily managing EPTP list for nesting 
> scenarios) they could do that by extending the vmx_vmfunc_intercept. I can 
> also add a comment there - Will that be sufficient? (I'm trying to avoid 
> another revision after I revise it to add the other exception conditions as 
> stated)

... such a comment. And yes, I'd be as fine with just a comment as
with the wrapper being folded in.

>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -3816,8 +3816,9 @@ x86_emulate(
>>>  struct segment_register reg;
>>>  unsigned long base, limit, cr0, cr0w;
>>>
>>> -if ( modrm == 0xdf ) /* invlpga */
>>> +switch( modrm )
>>>  {
>>> +case 0xdf: /* invlpga AMD */
>>>  generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, -1);
>>>  generate_exception_if(!mode_ring0(), EXC_GP, 0);
>>>  fail_if(ops->invlpg == NULL);
>>
>>The diff now looks much better. Yet I don't see why you added "AMD"
>>to the comment - we don't elsewhere note that certain instructions are
>>vendor specific (and really which ones are also changes over time, see RDTSCP
>>for a prominent example).
>>
> 
> I thought it would be better to specify instructions that are unique to a 
> specific CPU.
> But I can remove it.

Yes please.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 07/15] VMX: add VMFUNC leaf 0 (EPTP switching) to emulator.

[Jan Beulich]

>>> On 11.07.15 at 23:25,  wrote:
>> From: Sahita, Ravi
>>Sent: Saturday, July 11, 2015 1:01 PM
>>>From: Jan Beulich [mailto:jbeul...@suse.com]
>>>Sent: Friday, July 10, 2015 2:31 AM
>> On 10.07.15 at 02:52,  wrote:
 @@ -3825,10 +3826,7 @@ x86_emulate(
 ctxt)) )
  goto done;
  break;
 -}
 -
 -if ( modrm == 0xf9 ) /* rdtscp */
 -{
 +case 0xf9: /* rdtscp */ {
  uint64_t tsc_aux;
  fail_if(ops->read_msr == NULL);
  if ( (rc = ops->read_msr(MSR_TSC_AUX, &tsc_aux, ctxt))
 !=
 0 ) @@ -3836,7 +3834,19 @@ x86_emulate(
  _regs.ecx = (uint32_t)tsc_aux;
  goto rdtsc;
  }
 +case 0xd4: /* vmfunc */
 +generate_exception_if(lock_prefix | rep_prefix() |
 + (vex.pfx ==
>>>vex_66),
 +  EXC_UD, -1);
 +fail_if(ops->vmfunc == NULL);
 +if ( (rc = ops->vmfunc(ctxt) != X86EMUL_OKAY) )
 +goto done;
 +break;
 +default:
 +goto continue_grp7;
 +}
 +break;

 + continue_grp7:
>>>
>>>Already when first looking at this I disliked this label. Looking at it
>>>again, I'd really like to see it gone: RDTSCP handling already ends in
>>>a goto. Since the only VMFUNC currently implemented doesn't modify any
>>>register state either, its handling could end in an unconditional "goto done"
>>too for now.
>>>And INVLPG, not modifying any register state, could follow suit.
>>>
>>
>>Sure - no issues with that.
>>
> 
> On second thoughts, I cannot really use a goto done for these 2 cases since 
> that will skip the single-step tracing check that's performed in the existing 
> flow.

Good point.

> So I can add a new label entrypoint before the tracing check, or goto 
> writeback (with the dst.type switch there being a wasted check), or I can 
> keep the flow as is - which would you prefer?

I think "goto writeback" for an insn not having any register state to
write back may end up being confusing to future readers. I.e. such
use would need to at least be annotated with a brief comment.
Whether to go that route or add a new label no_writeback or
insn_done or some such (again accompanied by a brief comment)
I'd leave up to you.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 11/15] x86/altp2m: define and implement alternate p2m HVMOP types.

[Jan Beulich]

>>> On 11.07.15 at 00:03,  wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>>Sent: Friday, July 10, 2015 3:01 AM
> On 10.07.15 at 02:52,  wrote:
>>> +default:
>>> +return -ENOSYS;
>>> +
>>> +break;
>>
>>Bogus (unreachable) break.
> 
> Wanted to keep this so that if someone removes the error code then they 
> don't cause an invalid fall through.
> But ok with removing it if you think so.

We don't (intentionally) do this anywhere else, so it should be
removed.

>>> +if ( !(d ? d : current->domain)->arch.altp2m_active )
>>
>>This is bogus: d is NULL if and only if altp2m_vcpu_enable_notify, i.e. I 
> don't
>>see why you can't just use current->domain inside that case (and you really
>>do). That would then also eliminate the need for this redundant and
>>obfuscating switch() nesting you use.
>>
> 
> We need to check if the target domain is in altp2m mode for all the 
> following sub-ops.
> If we removed this check, we would need to check for target domain being in 
> altp2m for all the following cases.
> Andrew wanted to refactor and pull common code up, and that's what this is 
> one case of for hvm_op.

I'd be fine with such refactoring if it didn't result in nested switch()-es
using the same control expression.

>>> +
>>> +struct xen_hvm_altp2m_set_mem_access {
>>> +/* view */
>>> +uint16_t view;
>>> +/* Memory type */
>>> +uint16_t hvmmem_access; /* xenmem_access_t */
>>> +uint8_t pad[4];
>>> +/* gfn */
>>> +uint64_t gfn;
>>> +};
>>> +typedef struct xen_hvm_altp2m_set_mem_access
>>> xen_hvm_altp2m_set_mem_access_t;
>>> +DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t);
>>> +
>>> +struct xen_hvm_altp2m_change_gfn {
>>> +/* view */
>>> +uint16_t view;
>>> +uint8_t pad[6];
>>> +/* old gfn */
>>> +uint64_t old_gfn;
>>> +/* new gfn, INVALID_GFN (~0UL) means revert */
>>> +uint64_t new_gfn;
>>> +};
>>> +typedef struct xen_hvm_altp2m_change_gfn
>>xen_hvm_altp2m_change_gfn_t;
>>> +DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_change_gfn_t);
>>> +
>>> +struct xen_hvm_altp2m_op {
>>> +uint32_t cmd;
>>> +/* Get/set the altp2m state for a domain */
>>> +#define HVMOP_altp2m_get_domain_state 1
>>> +#define HVMOP_altp2m_set_domain_state 2
>>> +/* Set the current VCPU to receive altp2m event notifications */
>>> +#define HVMOP_altp2m_vcpu_enable_notify   3
>>> +/* Create a new view */
>>> +#define HVMOP_altp2m_create_p2m   4
>>> +/* Destroy a view */
>>> +#define HVMOP_altp2m_destroy_p2m  5
>>> +/* Switch view for an entire domain */
>>> +#define HVMOP_altp2m_switch_p2m   6
>>> +/* Notify that a page of memory is to have specific access types */
>>> +#define HVMOP_altp2m_set_mem_access   7
>>> +/* Change a p2m entry to have a different gfn->mfn mapping */
>>> +#define HVMOP_altp2m_change_gfn   8
>>> +domid_t domain;
>>> +uint8_t pad[2];
>>
>>While you added padding fields as asked for, you still don't verify them to 
> be
>>zero on input.
> 
> Specifically what were you thinking we need to do here - also would be good 
> if you can explain what was the underlying concern? (thanks)

I'm pretty sure I said so before - future extensibility. I.e. a means to
make use of the now unused (padding) fields, which can only be done
if the fields are being checked to be zero while unused.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] Requesting a freeze exception for vm_event memory introspection helpers

[Razvan Cojocaru]

Hello,

I'd like to ask for a freeze exception for the "vm_event memory
introspection helpers" series.

[PATCH 1/3] xen/mem_access: Support for memory-content hiding
[PATCH 2/3] xen/vm_event: Support for guest-requested events
[PATCH 3/3] xen/vm_event: Deny register writes if refused by
vm_event reply

All patches have been acked by at least one person (though patch 1 is
still under some discussion).

1. Benefits of the series making it in this release:

* Probably the most important benefit is that 4.6 development has been
very open to refactoring vm_events, and patch 3/3 makes vm_events behave
in a consistent manner (all register-write vm_events are pre-write events).

* There are 3rd parties interested in these features (Tamas, for
example, has already expressed interest in uses of patch 1/1).

2. Risks of including the series:

* Since two of the three patches have already received acks from 3+
people, I would assume that the risks for those are minimal. As for the
first patch, unless a vm_event consumer uses the new
VM_EVENT_FLAG_SET_EMUL_READ_DATA vm_event response flag in conjunction
with VM_EVENT_FLAG_EMULATE, the impact should be close to 0 (only a few
"if ( unlikely(set_context_enabled) )" extra statements).

A new series will follow as soon as possible, addressing Jan's comments
on the first patch and cleaning up patch 3 a little more.


Thank you in advance for considering this,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [RFC v3] xSplice design

[Martin Pohlack]

Hi,

I have a couple of high-level points here:

* Do you think it would be reasonable to have a minimal design without
  in-place patching, just using trampolines?  The examples discussed
  below suggest so.

* Regarding tboot integration: Do you plan to report hotpatching state
  to guests or do you want to keep that unchanged?

* If reporting:  Should we hash / sign the hotpatches in load order or
  should we work on an otherwise ordered set?

  In other words, do the sequences
  1: load + activate A, load + activate B
  2: load + activate B, load + activate A
  Result in the same system state and hash or not?

* Should we consider pairs of activation / deactivation events as
  no-ops?

  In other words, should hypervisor X and hypervisor X after activating
  and deactivating module A report the same system state and hash?

* What about auditing?  Currently the design discusses a method to
  query about the current state of affairs with regard to hotpatch
  modules.  Do we need something like a audit log for hotpatch
  operations?  We should at least report high-level operations that
  could impact the integrity on the console with a low threshold.

* There is a general (and mostly obscure) limitation on unloading
  hotpatches:

  In contrast to normal kernel modules where the module code adheres
  to specific conventions around resource allocation and locking,
  hotpatches typically contain code from any context.  That code is
  usually not aware that it can be unloaded.

  That code could leave behind in Xen references to itself, e.g., by
  installing a function pointer in a global data structure, without
  incrementing something like a usage count.  While most hotpatch code
  will probably be very simple and small, a similar effect could even
  be achieved by code called from the hotpatch in Xen, e.g., some code
  patch could dynamically generate a backtrace and later decide to
  inspect individual elements from the collected trace, later being a
  time, where the hotpatch has been unloaded again.

  One could approach that proplem from multiple angles: code
  inspection of generated hotpatches, testing, and by making unloading
  a very special and exceptional operation.


... and more inline comments below.

Regards,
Martin Pohlack

On 06.07.2015 22:26, Konrad Rzeszutek Wilk wrote:
> Since RFC v2 
> [http://lists.xen.org/archives/html/xen-devel/2015-05/msg02142.html]
>  - Ingested every review comment in.
> 
> For those who prefer an diff of what changed between v2 and this
> I am attaching an diff to help easy reviewing.
> 
> Please see inline the RFC v3 which in general:
>  - Ditches the attempt at defining an ELF payload using semi-Elf language
>and just concentrates on structures.
>  - Expands on the preemption of the hypercalls
>  - Expands the implementation details with various topics that emerged
>during v2 review
>  - Adds ASCII art (if you can call it that), and an example.
>  - state diagram the command hypercall.
> 
> # xSplice Design v1 (EXTERNAL RFC v3)
> 
> ## Rationale
> 
> A mechanism is required to binarily patch the running hypervisor with new
> opcodes that have come about due to primarily security updates.
> 
> This document describes the design of the API that would allow us to
> upload to the hypervisor binary patches.
> 
> The document is split in four sections:
>  - Detailed descriptions of the problem statement.
>  - Design of the data structures.
>  - Design of the hypercalls.
>  - Implementation notes that should be taken into consideration.
> 
> 
> ## Glossary
> 
>  * splice - patch in the binary code with new opcodes
>  * trampoline - a jump to a new instruction.
>  * payload - telemetries of the old code along with binary blob of the new
>function (if needed).
>  * reloc - telemetries contained in the payload to construct proper 
> trampoline.
> 
> ## Multiple ways to patch
> 
> The mechanism needs to be flexible to patch the hypervisor in multiple ways
> and be as simple as possible. The compiled code is contiguous in memory with
> no gaps - so we have no luxury of 'moving' existing code and must either
> insert a trampoline to the new code to be executed - or only modify in-place
> the code if there is sufficient space. The placement of new code has to be 
> done
> by hypervisor and the virtual address for the new code is allocated 
> dynamically.
> 
> This implies that the hypervisor must compute the new offsets when splicing
> in the new trampoline code. Where the trampoline is added (inside
> the function we are patching or just the callers?) is also important.
> 
> To lessen the amount of code in hypervisor, the consumer of the API
> is responsible for identifying which mechanism to employ

The hypervisor at least needs to make sure that in-place patches fit in
the old place.

> and how many locations
> to patch. Combinations of modifying in-place code, adding trampoline, etc
> has to be supported. The API should allow read/write any memory within
> t

Re: [Xen-devel] [PATCH v3 03/13] VMX: implement suppress #VE.

[Jan Beulich]

>>> On 10.07.15 at 21:30,  wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>>Sent: Thursday, July 09, 2015 6:01 AM
> On 01.07.15 at 20:09,  wrote:
>>> @@ -232,6 +235,15 @@ static int ept_set_middle_entry(struct p2m_domain
>>> @@ -1134,6 +1151,13 @@ int ept_p2m_init(struct p2m_domain *p2m)
>>>  p2m->flush_hardware_cached_dirty = ept_flush_pml_buffers;
>>>  }
>>>
>>> +table =
>>> + map_domain_page(pagetable_get_pfn(p2m_get_pagetable(p2m)));
>>> +
>>> +for ( i = 0; i < EPT_PAGETABLE_ENTRIES; i++ )
>>> +table[i].suppress_ve = 1;
>>> +
>>> +unmap_domain_page(table);
>>
>>... why is this needed? Bit 63 is documented to be ignored in PML4Es (just 
> like
>>in all other intermediate page tables).
> 
> Valid point - this has no negative side-effects per se so we didn't change 
> this.

Taking "we didn't change this" to refer to v3 -> v4, I still think this
should be dropped if it isn't needed. There can only be confusion
arising from code having no purpose.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Razvan Cojocaru]

Hello,

I'm battling the following hypervisor crash with current staging:

(d2) Invoking ROMBIOS ...
(XEN) stdvga.c:147:d2v0 entering stdvga and caching modes
(d2) VGABios $Id: vgabios.c,v 1.67 2008/01/27 09:44:12 vruppert Exp $
(XEN) Watchdog timer detects that CPU7 is stuck!
(XEN) [ Xen-4.6-unstable  x86_64  debug=y  Not tainted ]
(XEN) CPU:7
(XEN) RIP:e008:[] _spin_lock+0x31/0x54
(XEN) RFLAGS: 0202   CONTEXT: hypervisor (d2v0)
(XEN) rax: c11d   rbx: 83041e687970   rcx: c11e
(XEN) rdx: 83041e687970   rsi: c11e   rdi: 83041e687978
(XEN) rbp: 83040eb37208   rsp: 83040eb37200   r8:  
(XEN) r9:     r10: 82d08028c3c0   r11: 
(XEN) r12: 83041e687000   r13: 83041e687970   r14: 83040eb37278
(XEN) r15: 000c253f   cr0: 8005003b   cr4: 001526e0
(XEN) cr3: 0004054a   cr2: 
(XEN) ds:    es:    fs:    gs:    ss:    cs: e008
(XEN) Xen stack trace from rsp=83040eb37200:
(XEN)83040eb37278 83040eb37238 82d0801d09b6 0282
(XEN)0008 830403791bf0 83041e687000 83040eb37268
(XEN)82d0801cb23a 000c253f 8300d85fc000 0001
(XEN)00c2 83040eb37298 82d0801cb410 000c253f
(XEN) 00010001 0100 83040eb37328
(XEN)82d0801c2403 83040eb37394 83040eb3 
(XEN)83040eb37360 00c2 8304054cb000 053f
(XEN)0002  83040eb373f4 00c2
(XEN)83040eb373d8   82d08028c620
(XEN) 83040eb37338 82d0801c3e5d 83040eb37398
(XEN)82d0801cb107 00010eb37394 830403791bf0 830403791bf0
(XEN)83041e687000 83040eb37398 830403791bf0 0001
(XEN)83040eb373d8 0001 000c253f 83040eb373c8
(XEN)82d0801cb291 83040eb37b30 8300d85fc000 0001
(XEN) 83040eb37428 82d0801bb440 000a0001
(XEN)000c253f 00010001 0111 83040eb37478
(XEN)0001   0001
(XEN)0001 83040eb374a8 82d0801bc0b9 0001
(XEN)000c253f 8300d85fc000 000a0001 0100
(XEN)83040eb37728 82e00819dc60  83040eb374c8
(XEN) Xen call trace:
(XEN)[] _spin_lock+0x31/0x54
(XEN)[] stdvga_mem_accept+0x3b/0x125
(XEN)[] hvm_find_io_handler+0x68/0x8a
(XEN)[] hvm_mmio_internal+0x37/0x67
(XEN)[] __hvm_copy+0xe9/0x37d
(XEN)[] hvm_copy_from_guest_phys+0x14/0x16
(XEN)[] hvm_process_io_intercept+0x10b/0x1d6
(XEN)[] hvm_io_intercept+0x35/0x5b
(XEN)[] hvmemul_do_io+0x1ff/0x2c1
(XEN)[] hvmemul_do_io_addr+0x117/0x163
(XEN)[] hvmemul_do_mmio_addr+0x24/0x26
(XEN)[] hvmemul_rep_movs+0x1ef/0x335
(XEN)[] x86_emulate+0x56c9/0x13088
(XEN)[] _hvm_emulate_one+0x186/0x281
(XEN)[] hvm_emulate_one+0x10/0x12
(XEN)[] handle_mmio+0x54/0xd2
(XEN)[] handle_mmio_with_translation+0x44/0x46
(XEN)[] hvm_hap_nested_page_fault+0x15f/0x589
(XEN)[] vmx_vmexit_handler+0x150e/0x188d
(XEN)[] vmx_asm_vmexit_handler+0x41/0xc0
(XEN)
(XEN)
(XEN) 
(XEN) Panic on CPU 7:
(XEN) FATAL TRAP: vector = 2 (nmi)
(XEN) [error_code=]
(XEN) 

At first I thought it was caused by V5 of the vm_event-based
introspection series, but I've rolled it back enough to apply V4 on top
of it (which has been thoroughly tested on Thursday), and it still
happens, so this would at least appear to be unrelated at this point
(other than the fact that our use case is maybe somewhat unusual with
heavy emulation).

I'll keep digging, but since this is a busy time for Xen I thought I'd
issue a heads-up here as soon as possible, in case the problem is
obvious for somebody and it helps getting it fixed sooner.


Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 05/13] x86/altp2m: basic data structures and support routines.

[Jan Beulich]

>>> On 10.07.15 at 23:48,  wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>>Sent: Thursday, July 09, 2015 6:30 AM
>>
> On 01.07.15 at 20:09,  wrote:
>>> ---
>>>  xen/arch/x86/hvm/Makefile|  1 +
>>>  xen/arch/x86/hvm/altp2m.c| 92
>>+
>>
>>Wouldn't this better go into xen/arch/x86/mm/?
> 
> In this case we followed the pattern of nestedhvm - hope that's ok.

Not really imo: Nested HVM obviously belongs in hvm/; alt-P2m
is more of a mm extension than a HVM one afaict, and hence
would rather belong in mm/.

>>> +int
>>> +altp2m_vcpu_initialise(struct vcpu *v) {
>>> +int rc = -EOPNOTSUPP;
>>> +
>>> +if ( v != current )
>>> +vcpu_pause(v);
>>> +
>>> +if ( !hvm_funcs.ap2m_vcpu_initialise ||
>>> + (hvm_funcs.ap2m_vcpu_initialise(v) == 0) )
>>> +{
>>> +rc = 0;
>>
>>I think you would better honor the error code returned by
>>hvm_funcs.ap2m_vcpu_initialise() and enter this block only if it was zero.
> 
> The code is checking that condition - did I misinterpret?

It is checking the condition, yes, but not propagating the error
code.

>>> +altp2m_vcpu_reset(v);
>>> +vcpu_altp2m(v).p2midx = 0;
>>> +atomic_inc(&p2m_get_altp2m(v)->active_vcpus);
>>> +
>>> +ap2m_vcpu_update_eptp(v);
>>
>>We're in vendor independent code here - either the function is misnamed, or
>>it shouldn't be called directly from here.
> 
> Would it be reasonable to add if hap_enabled and cpu_has_vmx checks like 
> other code in this file that invokes ept specific ops?
> Otherwise it seems ok that the function would be called from here for 
> p2m_altp2m interactions such as switching altp2m by id etc.
> Open to any other suggestions from you, or we would like to leave it as it 
> is.

Imo such should be abstracted out properly (if it's indeed EPT-specific),
or the function be renamed.

>>> +void
>>> +altp2m_vcpu_destroy(struct vcpu *v)
>>> +{
>>> +struct p2m_domain *p2m;
>>> +
>>> +if ( v != current )
>>> +vcpu_pause(v);
>>> +
>>> +if ( hvm_funcs.ap2m_vcpu_destroy )
>>> +hvm_funcs.ap2m_vcpu_destroy(v);
>>> +
>>> +if ( (p2m = p2m_get_altp2m(v)) )
>>> +atomic_dec(&p2m->active_vcpus);
>>
>>The ordering looks odd - from an abstract perspective I'd expect
>>p2m_get_altp2m() to not return the p2m anymore that was just destroyed via
>>hvm_funcs.ap2m_vcpu_destroy().
>>
> 
> ap2m_vcpu_destroy is for destroying vcpu context related to altp2m - note 
> this is not implemented since its not needed for Intel implementation.  The 
> idea is that if something needs to be done specifically for for AMD then that 
> could be done here. 

First of all this doesn't invalidate or address the concern raised.
And then - if you don't need the hook, why don't you leave it out
altogether, eliminating the need to decide about its caller's proper
placement?

>>> +void ap2m_vcpu_update_eptp(struct vcpu *v) {
>>
>>As I think I said before, I consider these ap2m_ prefixes ambiguous - the 'a'
>>could also stand for accelerated, advanced, ... Consistently staying with
>>altp2m_ would seem better.
>>
> 
> We have a comment above the list of these ap2m_ functions in hvm.h stating 
> these are for Alternate p2m - do you feel strongly about us changing this 
> naming? Also this is the interface naming, and if we renamed it altp2m_xxx it 
> would cause confusion with the actual altp2m_xx functionality - so we would 
> like to leave it as proposed.

I don't think there would be much confusion - structure member
names and function names live in different name spaces anyway.
So yes, I continue to think ap2m is a bad prefix...

>>> --- a/xen/arch/x86/mm/hap/hap.c
>>> +++ b/xen/arch/x86/mm/hap/hap.c
>>> @@ -459,7 +459,7 @@ void hap_domain_init(struct domain *d)  int
>>> hap_enable(struct domain *d, u32 mode)  {
>>>  unsigned int old_pages;
>>> -uint8_t i;
>>> +uint16_t i;
>>
>>unsigned int (also elsewhere, including uint8_t-s)
> 
> We used existing iterator types that were being used (uint8_t was being used 
> in hap_final_teardown).
> If you feel strongly we could change it but we would change code that we 
> didn't need to touch for this patch.

I didn't say you should change code you otherwise don't need to
touch. But both new code as well as code being changed anyway
shouldn't repeat/continue pre-existing mistakes (or however you'd
want to call such).

>>> @@ -294,6 +298,12 @@ struct arch_domain
>>>  struct p2m_domain *nested_p2m[MAX_NESTEDP2M];
>>>  mm_lock_t nested_p2m_lock;
>>>
>>> +/* altp2m: allow multiple copies of host p2m */
>>> +bool_t altp2m_active;
>>> +struct p2m_domain *altp2m_p2m[MAX_ALTP2M];
>>> +mm_lock_t altp2m_lock;
>>> +uint64_t *altp2m_eptp;
>>
>>This is a non-insignificant increase of the structure size - perhaps all
>>of these should hang off of struct arch_domain via a single,
>>separately allocated pointer?
> 
> Is this a nice-to-have - again we modelled 

[Xen-devel] [PATCH OSSTEST v2] No longer export $OSSTEST_CONFIG

[Ian Campbell]

>From cri-args-hostlists or invoke-daemon.

All sites now have a suitable $HOME/.xen-osstest/settings in place
which does this.

Signed-off-by: Ian Campbell 
---
This was waiting to be applied once " allow instance specific
settings" passed the Cambridge push gate, which happened ages ago.
---
 cri-args-hostlists | 1 -
 invoke-daemon  | 2 --
 2 files changed, 3 deletions(-)

diff --git a/cri-args-hostlists b/cri-args-hostlists
index a4e57b3..0dd2ef3 100644
--- a/cri-args-hostlists
+++ b/cri-args-hostlists
@@ -19,7 +19,6 @@
 if [ -e $HOME/.xen-osstest/settings ]; then
  source $HOME/.xen-osstest/settings
 fi
-export OSSTEST_CONFIG=${OSSTEST_CONFIG:-production-config}
 
 check_stop_core () {
if [ "x$OSSTEST_IGNORE_STOP" = xy ]; then return; fi
diff --git a/invoke-daemon b/invoke-daemon
index 6006798..5fab1da 100755
--- a/invoke-daemon
+++ b/invoke-daemon
@@ -19,8 +19,6 @@
 if [ -e $HOME/.xen-osstest/settings ]; then
  source $HOME/.xen-osstest/settings
 fi
-export OSSTEST_CONFIG=${OSSTEST_CONFIG:-production-config}
-
 
 cd "${0%/*}"
 if [ "x$2" != x ]; then sleep $2; fi
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Requesting for freeze exception for RMRR

[Jan Beulich]

>>> On 13.07.15 at 08:31,  wrote:
> 3. explain why it doesn't break things (risks).
> 
> Our policy makes sure that system will work in the original way by 
> default as without the RMRR patches. And especially, this series just 
> impacts those platforms which have RMRR.

I think this should read "Our policy intends to make sure ...", making
more clear that there is a risk here (supported by the history of the
series).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Andrew Cooper]

On 13/07/2015 08:48, Razvan Cojocaru wrote:
> Hello,
>
> I'm battling the following hypervisor crash with current staging:
>
> (d2) Invoking ROMBIOS ...
> (XEN) stdvga.c:147:d2v0 entering stdvga and caching modes
> (d2) VGABios $Id: vgabios.c,v 1.67 2008/01/27 09:44:12 vruppert Exp $
> (XEN) Watchdog timer detects that CPU7 is stuck!
> (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Not tainted ]
> (XEN) CPU:7
> (XEN) RIP:e008:[] _spin_lock+0x31/0x54
> (XEN) RFLAGS: 0202   CONTEXT: hypervisor (d2v0)
> (XEN) rax: c11d   rbx: 83041e687970   rcx: c11e
> (XEN) rdx: 83041e687970   rsi: c11e   rdi: 83041e687978
> (XEN) rbp: 83040eb37208   rsp: 83040eb37200   r8:  
> (XEN) r9:     r10: 82d08028c3c0   r11: 
> (XEN) r12: 83041e687000   r13: 83041e687970   r14: 83040eb37278
> (XEN) r15: 000c253f   cr0: 8005003b   cr4: 001526e0
> (XEN) cr3: 0004054a   cr2: 
> (XEN) ds:    es:    fs:    gs:    ss:    cs: e008
> (XEN) Xen stack trace from rsp=83040eb37200:
> (XEN)83040eb37278 83040eb37238 82d0801d09b6 0282
> (XEN)0008 830403791bf0 83041e687000 83040eb37268
> (XEN)82d0801cb23a 000c253f 8300d85fc000 0001
> (XEN)00c2 83040eb37298 82d0801cb410 000c253f
> (XEN) 00010001 0100 83040eb37328
> (XEN)82d0801c2403 83040eb37394 83040eb3 
> (XEN)83040eb37360 00c2 8304054cb000 053f
> (XEN)0002  83040eb373f4 00c2
> (XEN)83040eb373d8   82d08028c620
> (XEN) 83040eb37338 82d0801c3e5d 83040eb37398
> (XEN)82d0801cb107 00010eb37394 830403791bf0 830403791bf0
> (XEN)83041e687000 83040eb37398 830403791bf0 0001
> (XEN)83040eb373d8 0001 000c253f 83040eb373c8
> (XEN)82d0801cb291 83040eb37b30 8300d85fc000 0001
> (XEN) 83040eb37428 82d0801bb440 000a0001
> (XEN)000c253f 00010001 0111 83040eb37478
> (XEN)0001   0001
> (XEN)0001 83040eb374a8 82d0801bc0b9 0001
> (XEN)000c253f 8300d85fc000 000a0001 0100
> (XEN)83040eb37728 82e00819dc60  83040eb374c8
> (XEN) Xen call trace:
> (XEN)[] _spin_lock+0x31/0x54
> (XEN)[] stdvga_mem_accept+0x3b/0x125
> (XEN)[] hvm_find_io_handler+0x68/0x8a
> (XEN)[] hvm_mmio_internal+0x37/0x67
> (XEN)[] __hvm_copy+0xe9/0x37d
> (XEN)[] hvm_copy_from_guest_phys+0x14/0x16
> (XEN)[] hvm_process_io_intercept+0x10b/0x1d6
> (XEN)[] hvm_io_intercept+0x35/0x5b
> (XEN)[] hvmemul_do_io+0x1ff/0x2c1
> (XEN)[] hvmemul_do_io_addr+0x117/0x163
> (XEN)[] hvmemul_do_mmio_addr+0x24/0x26
> (XEN)[] hvmemul_rep_movs+0x1ef/0x335
> (XEN)[] x86_emulate+0x56c9/0x13088
> (XEN)[] _hvm_emulate_one+0x186/0x281
> (XEN)[] hvm_emulate_one+0x10/0x12
> (XEN)[] handle_mmio+0x54/0xd2
> (XEN)[] handle_mmio_with_translation+0x44/0x46
> (XEN)[] hvm_hap_nested_page_fault+0x15f/0x589
> (XEN)[] vmx_vmexit_handler+0x150e/0x188d
> (XEN)[] vmx_asm_vmexit_handler+0x41/0xc0
> (XEN)
> (XEN)
> (XEN) 
> (XEN) Panic on CPU 7:
> (XEN) FATAL TRAP: vector = 2 (nmi)
> (XEN) [error_code=]
> (XEN) 
>
> At first I thought it was caused by V5 of the vm_event-based
> introspection series, but I've rolled it back enough to apply V4 on top
> of it (which has been thoroughly tested on Thursday), and it still
> happens, so this would at least appear to be unrelated at this point
> (other than the fact that our use case is maybe somewhat unusual with
> heavy emulation).
>
> I'll keep digging, but since this is a busy time for Xen I thought I'd
> issue a heads-up here as soon as possible, in case the problem is
> obvious for somebody and it helps getting it fixed sooner.

In c/s 3bbaaec09b1b942f5624dee176da6e416d31f982 there is now a
deliberate split between stdvga_mem_accept() and stdvga_mem_complete()
about locking and unlocking the stdvga lock.

At a guess, the previous chain of execution accidentally omitted the
stdvga_mem_complete() call.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH OSSTEST] INSTALL.production: Start writing some stuff down

[Ian Campbell]

I wrote this ages ago while migrating the Cambridge instance to a new
VM and then forgot about it. It's incomplete and I'm not sure where I
was planning to go next, but better than nothing maybe?

Signed-off-by: Ian Campbell 
---
 INSTALL.production | 78 ++
 1 file changed, 78 insertions(+)
 create mode 100644 INSTALL.production

diff --git a/INSTALL.production b/INSTALL.production
new file mode 100644
index 000..2db8bd3
--- /dev/null
+++ b/INSTALL.production
@@ -0,0 +1,78 @@
+BIG FAT WARNING
+===
+
+These instructions are based on my understanding of the system and the
+things which needed to be changed when we moved the Citrix Cambridge
+instance of osstest from one subdomain to another. They have not been
+tried in a real standalone deployment
+
+STANDALONE MODE
+===
+
+These instructions deal with setting up a full production instance of
+osstest, i.e. one which is cron job driven. You might call this
+"infrastructure mode" or "executive mode".
+
+Most individual developers are probably better served with "standalone
+mode" which is described in the main README.
+
+OSSTEST USER
+
+
+osstest is designed to run as its own user on test control host. In
+this document it is assumed this user is called "osstest".
+
+INFRASTRUCTURE
+==
+
+DATABASE SERVER
+---
+
+osstest requires a Postgres database server and a database configured
+with the schema described in executive-postgresql-schema which should
+be accessible to an osstest role account.
+
+The hostname and dbname are configured via the "ExecutiveDbnamePat"
+config option.
+
+OWNER AND QUEUE DAEMONS
+-
+
+These two daemons (ms-ownerdaemon and ms-queuedaemon) are part of
+osstest and should be run out of inittab from a dedicated clone of the
+osstest git repo. e.g.
+
+otdo:2345:respawn:su osstest -c 
'/home/osstest/daemons-testing.git/invoke-daemon ms-ownerdaemon'
+otdq:2345:respawn:su osstest -c 
'/home/osstest/daemons-testing.git/invoke-daemon ms-queuedaemon 2'
+
+The two daemons may share the same git repo
+(/home/osstest/daemons-testing.git in this example) but it should not
+be shared with other osstest activities (i.e. the production cronjobs)
+
+The two daemons need not run on the same host as either each other or
+even on the osstest control VM. (For fate-sharing reasons it may be
+preferred to run the owner daemon on the same host as the postgres
+database).
+
+The hosts running the two daemons should be referenced by the
+OwnerDaemonHost and QueueDaemonHost configuration options. If they
+happen to run on the same host then setting ControlDaemonHost will
+configure both in one go.
+
+DHCP AND PXE
+
+
+osstest does not require a dedicated DHCP server but one must be
+present on the network and osstest requires access to its leases
+table (either as a local file or as a host + TCP port to be connected
+to which will dump the lease file. This is configured via the
+DhcpWatchMethod host prop (default configured via
+HostProp_DhcpWatchMethod)
+
+Likewise osstest does not require a dedicated PXE server but one must
+exist on the network and be accessible via a local path on the osstest
+control VM (e.g. over NFS)
+
+CONFIGURATION
+=
+
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH OSSTEST] mg-list-all-branches: Sort branches according to any embedded version

[Ian Campbell]

Many of our branches include a version number, this change results in
e.g. linux-3.0 < linux-3.4 < linux-3.10 rather than linux-3.0 <
linux-3.10 < linux-3.4, which is more natural for uses such as
./mg-all-branch-statuses.

Requires Sort::Versions (Debian package libsort-versions-perl).

Signed-off-by: Ian Campbell 
---
 mg-list-all-branches | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mg-list-all-branches b/mg-list-all-branches
index c968a99..1549f81 100755
--- a/mg-list-all-branches
+++ b/mg-list-all-branches
@@ -3,6 +3,7 @@
 # mentioned in cr-daily-branch or crontab
 
 use strict;
+use Sort::Versions;
 
 our %branches;
 
@@ -15,4 +16,4 @@ foreach my $f (qw(cr-for-branches crontab)) {
 close C or die $!;
 }
 
-print $_,"\n" or die $! foreach sort keys %branches;
+print $_,"\n" or die $! foreach sort { versioncmp($a, $b) } keys %branches;
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 4/5] sched: credit2: add soft affinity awareness to function get_fallback_cpu

[Justin T. Weaver]

by adding a two step loop. The function now finds a fallback cpu for a given
vcpu using the following precedence...
1) the vcpu's current pcpu
soft affinity step...
2) another pcpu from the vcpu's current runq in the vcpu's soft affinity
3) an online pcpu in the vcpu's domain's cpupool, and in the vcpu's soft
   affinity
hard affinity step...
4) another pcpu from the vcpu's current runq in the vcpu's hard affinity
3) an online pcpu in the vcpu's domain's cpupool, and in the vcpu's hard
   affinity

Signed-off-by: Justin T. Weaver 
---
Changes in v4:
 * renamed all uses of csched2_cpumask to scratch_mask
 * updated the comment before the function describing the added soft affinity
   aware functionality
 * updated the function to match the flow of the rewrite in the hard affinity
   patch based on the v3 hard affinity review
 * moved the VCPU2ONLINE section outside of the else block; removed the else
   block
Changes in v3:
 * added balance loop to try to find a soft affinity cpu
Changes in v2:
 * Not submitted in version 2; focus was on the hard affinity patch
---
 xen/common/sched_credit2.c |   31 ++-
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 42a1097..66f0a20 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -284,25 +284,38 @@ struct csched2_dom {
  *
  * Function returns a valid pcpu for svc, in order of preference:
  * - svc's current pcpu;
- * - another pcpu from svc's current runq;
+ * - another pcpu from svc's current runq in svc's soft affinity;
+ * - an online pcpu in svc's domain's cpupool, and in svc's soft affinity;
+ * - another pcpu from svc's current runq in svc's hard affinity;
  * - an online pcpu in svc's domain's cpupool, and in svc's hard affinity;
  */
 static int get_fallback_cpu(struct csched2_vcpu *svc)
 {
-int cpu;
+int cpu, balance_step;
 
 if ( likely(cpumask_test_cpu(svc->vcpu->processor,
  svc->vcpu->cpu_hard_affinity)) )
 return svc->vcpu->processor;
 
-cpumask_and(scratch_mask, svc->vcpu->cpu_hard_affinity,
-&svc->rqd->active);
-cpu = cpumask_first(scratch_mask);
-if ( likely(cpu < nr_cpu_ids) )
-return cpu;
+for_each_sched_balance_step( balance_step )
+{
+if ( balance_step == SCHED_BALANCE_SOFT_AFFINITY
+&& !__vcpu_has_soft_affinity(svc->vcpu,
+svc->vcpu->cpu_hard_affinity) )
+continue;
+
+sched_balance_cpumask(svc->vcpu, balance_step, scratch_mask);
+cpumask_and(scratch_mask, scratch_mask, &svc->rqd->active);
+cpu = cpumask_first(scratch_mask);
+if ( likely(cpu < nr_cpu_ids) )
+return cpu;
+
+sched_balance_cpumask(svc->vcpu, balance_step, scratch_mask);
+cpumask_and(scratch_mask, scratch_mask, VCPU2ONLINE(svc->vcpu));
+if ( !cpumask_empty(scratch_mask) )
+break;
+}
 
-cpumask_and(scratch_mask, svc->vcpu->cpu_hard_affinity,
-VCPU2ONLINE(svc->vcpu));
 ASSERT( !cpumask_empty(scratch_mask) );
 return cpumask_first(scratch_mask);
 }
-- 
1.7.10.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 1/5] sched: factor out VCPU2ONLINE to common header file

[Justin T. Weaver]

Move macro VCPU2ONLINE from schedule.c to sched.h so it can be used by other
source files.

Signed-off-by: Justin T. Weaver 
---
 xen/common/schedule.c   |1 -
 xen/include/xen/sched.h |2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index ecf1545..c43b733 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -80,7 +80,6 @@ static struct scheduler __read_mostly ops;
 
 #define DOM2OP(_d)(((_d)->cpupool == NULL) ? &ops : ((_d)->cpupool->sched))
 #define VCPU2OP(_v)   (DOM2OP((_v)->domain))
-#define VCPU2ONLINE(_v) cpupool_online_cpumask((_v)->domain->cpupool)
 
 static inline void trace_runstate_change(struct vcpu *v, int new_state)
 {
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index b29d9e7..e5dd040 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -891,6 +891,8 @@ extern void dump_runq(unsigned char key);
 
 void arch_do_physinfo(xen_sysctl_physinfo_t *pi);
 
+#define VCPU2ONLINE(_v) cpupool_online_cpumask((_v)->domain->cpupool)
+
 #endif /* __SCHED_H__ */
 
 /*
-- 
1.7.10.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 2/5] sched: credit2: respect per-vcpu hard affinity

[Justin T. Weaver]

by making sure that vcpus only run on the pcpu(s) they are allowed to
run on based on their hard affinity cpu masks.

Signed-off-by: Justin T. Weaver 
---
Changes in v4:
 * Renamed scratch_mask to _scratch_mask
 * Renamed csched2_cpumask to scratch_mask
 * Removed "else continue" in function choose_cpu's for_each_cpu loop to make
   the code less confusing
 * Added an ASSERT that triggers if _scratch_mask[cpu] is NULL after
   allocation in function csched2_alloc_pdata
 * Added assignment to NULL for _scratch_mask[cpu] after call to
   free_cpumask_var in function csched2_alloc_pdata
 * Changed allocation of _scratch_mask from using xmalloc_array back to using
   xzalloc_array
 * Moved allocation of _scratch_mask from function csched2_init to function
   csched2_global_init
 * Added comment to function csched2_vcpu_migrate explaining the need for the
   vc->processor assignment after the else
 * Modified comment before function get_fallback_cpu; reworded into bulleted
   list
 * Changed cpumask_any to cpumask_first at the end of function get_fallback_cpu
 * Fixed indentation in function get_fallback_cpu to align with opening parens
 * Changed function get_fallback_cpu to variant suggested in the v3 review
 * Changed comment before function vcpu_is_migrateable; vcpu svc to just svc
 * Changed "run queue" in several comments to "runqueue"
 * Renamed function valid_vcpu_migration to vcpu_is_migrateable
 * Made condition check in function vcpu_is_migrateable "positive"
Changes in v3:
(all changes are based on v2 review comments unless noted)
 * Renamed cpumask to scratch_mask
 * Renamed function get_safe_pcpu to get_fallback_cpu
 * Improved comment for function get_fallback_cpu
 * Replaced cpupool_online_cpumask with VCPU2ONLINE in function
   get_fallback_cpu to shorten the line
 * Added #define for VCPU2ONLINE (probably should be factored out of
   schedule.c and here, and put into a common header)
 * Modified code in function get_fallback_cpu: moved check for current
   processor to the top; added an ASSERT because the mask should not be empty
 * Modified code and comment in function choose_cpu in migrate request section
 * Added comment to function choose_cpu explaining why the vcpu passed to the
   function might not have hard affinity with any of the pcpus in its assigned
   run queue
 * Modified code in function choose_cpu to make it more readable
 * Moved/changed "We didn't find ..." comment in function choose_cpu
 * Combined migration flag check and hard affinity check into valid migration
   check helper function; replaced code in three places in function
   balance_load with call to the helper function
 * Changed a BUG_ON to an ASSERT in function csched2_vcpu_migrate
 * Moved vc->processor assignment in function csched2_vcpu_migrate to an else
   block to execute only if current and destination run queues are the same;
   Note: without the processor assignment here the vcpu might be assigned to a
   processor it no longer is allowed to run on. In that case, function
   runq_candidate may only get called for the vcpu's old processor, and
   runq_candidate will no longer let a vcpu run on a processor that it's not
   allowed to run on (because of the hard affinity check first introduced in
   v1 of this patch).
 * csched2_init: changed xzalloc_bytes to xmalloc_array for allocation of
   scratch_mask
 * csched2_deinit: removed scratch_mask freeing loop; it wasn't needed
Changes in v2:
 * Added dynamically allocated cpu masks to avoid putting them on the stack;
   replaced temp masks from v1 throughout
 * Added helper function for code suggested in v1 review and called it in two
   locations in function choose_cpu
 * Removed v1 change to comment in the beginning of choose_cpu
 * Replaced two instances of cpumask_and/cpumask_empty with cpumask_intersects
 * Removed v1 re-work of code in function migrate; only change in migrate in
   v2 is the assignment of a valid pcpu from the destination run queue to
   vc->processor
 * In function csched2_vcpu_migrate: removed change from v1 that called
   function migrate even if cur and dest run queues were the same in order
   to get a runq_tickle call; added processor assignment to new_cpu to fix
   the real underlying issue which was the vcpu not getting a call to
   sched_move_irqs
 * Removed the looping added in v1 in function balance_load; may be added back
   later because it would help to have balance_load be more aware of hard
   affinity, but adding it does not affect credit2's current inability to
   respect hard affinity.
 * Removed coding style fix in function balance_load
 * Improved comment in function runq_candidate
---
 xen/common/sched_credit2.c |  153 
 1 file changed, 125 insertions(+), 28 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 75e0321..42a1097 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -194,6 +194,12 @@ int opt_overload_

[Xen-devel] [PATCH v4 0/5] sched: credit2: introduce per-vcpu hard and soft affinity

[Justin T. Weaver]

Hello,

The credit2 vcpu scheduler currently ignores per-vcpu hard and soft affinity
masks.

In the v3 review I was asked by George Dunlap to split up the soft affinity
patch into multiple patches by function and only resend the changes to two of
them, get_fallback_cpu and runq_tickle, so this series does not include any
soft affinity changes to credit2 functions balance_load or choose_cpu.  

The first patch is new to the series. It just moves macro VCPU2ONLINE from 
schedule.c to sched.h so other schedulers can use it.

The second patch updates the scheduler to ensure that vcpus only run
on pcpus on which they are allowed to run (hard affinity). I tested it using
xl vcpu-pin and xl vcpu-list. I changed the affinity in different ways using
scripted calls to vcpu-pin and observed the results using vcpu-list. Each VCPU
ran where it was supposed to.

Patch three factors out code from the credit scheduler (sched_credit.c) related
to soft affinity load balancing and places it in a common header (sched-if.h).
This allows credit2 to reuse the functions and defines in the soft affinity
patches. The only change here from v3 is an update to the commit message
adding that no functional changes are intended with the patch. I carried over
the reviewed-by line from Dario Faggioli.

In the v3 series there was a patch that only included indents in credit2
function runq_tickle in order to make the soft affinity patch easier to review.
Based on the review that patch has been dropped and the indents are included in
the separate credit2 soft affinity runq_tickle patch.

The fourth and fifth patches add per-vcpu soft affinity awareness to functions
get_fallback_cpu and runq_tickle, respectively. 

Look forward to the review comments, thanks!

Justin Weaver

---
[1/5] sched: factor out VCPU2ONLINE to common header file
[2/5] sched: credit2: respect per-vcpu hard affinity
[3/5] sched: factor out per-vcpu affinity related code to common header file
[4/5] sched: credit2: add soft affinity awareness to function get_fallback_cpu
[5/5] sched: credit2: add soft affinity awareness to function runq_tickle

xen/common/sched_credit.c  |   87 ++
xen/common/sched_credit2.c |  268 +---
xen/common/schedule.c  |1 -
xen/include/xen/sched-if.h |   65 +++
xen/include/xen/sched.h|2 +
5 files changed, 282 insertions(+), 141 deletions(-)

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 3/5] sched: factor out per-vcpu affinity related code to common header file

[Justin T. Weaver]

Move affinity balancing related functions and defines from sched_credit.c to
sched-if.h so other schedulers can use them. Change name prefixes from csched
to sched since they are no longer specific to the credit scheduler. No
functional changes intended.

Signed-off-by: Justin T. Weaver 
Reviewed-by: Dario Faggioli 
---
Changes in v4:
 * only the commit message was modified to indicate no functional changes
---
 xen/common/sched_credit.c  |   87 ++--
 xen/include/xen/sched-if.h |   65 +
 2 files changed, 76 insertions(+), 76 deletions(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 953ecb0..22252aa 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -129,26 +129,6 @@
 
 
 /*
- * Hard and soft affinity load balancing.
- *
- * Idea is each vcpu has some pcpus that it prefers, some that it does not
- * prefer but is OK with, and some that it cannot run on at all. The first
- * set of pcpus are the ones that are both in the soft affinity *and* in the
- * hard affinity; the second set of pcpus are the ones that are in the hard
- * affinity but *not* in the soft affinity; the third set of pcpus are the
- * ones that are not in the hard affinity.
- *
- * We implement a two step balancing logic. Basically, every time there is
- * the need to decide where to run a vcpu, we first check the soft affinity
- * (well, actually, the && between soft and hard affinity), to see if we can
- * send it where it prefers to (and can) run on. However, if the first step
- * does not find any suitable and free pcpu, we fall back checking the hard
- * affinity.
- */
-#define CSCHED_BALANCE_SOFT_AFFINITY0
-#define CSCHED_BALANCE_HARD_AFFINITY1
-
-/*
  * Boot parameters
  */
 static int __read_mostly sched_credit_tslice_ms = CSCHED_DEFAULT_TSLICE_MS;
@@ -290,51 +270,6 @@ __runq_remove(struct csched_vcpu *svc)
 }
 
 
-#define for_each_csched_balance_step(step) \
-for ( (step) = 0; (step) <= CSCHED_BALANCE_HARD_AFFINITY; (step)++ )
-
-
-/*
- * Hard affinity balancing is always necessary and must never be skipped.
- * But soft affinity need only be considered when it has a functionally
- * different effect than other constraints (such as hard affinity, cpus
- * online, or cpupools).
- *
- * Soft affinity only needs to be considered if:
- * * The cpus in the cpupool are not a subset of soft affinity
- * * The hard affinity is not a subset of soft affinity
- * * There is an overlap between the soft affinity and the mask which is
- *   currently being considered.
- */
-static inline int __vcpu_has_soft_affinity(const struct vcpu *vc,
-   const cpumask_t *mask)
-{
-return !cpumask_subset(cpupool_online_cpumask(vc->domain->cpupool),
-   vc->cpu_soft_affinity) &&
-   !cpumask_subset(vc->cpu_hard_affinity, vc->cpu_soft_affinity) &&
-   cpumask_intersects(vc->cpu_soft_affinity, mask);
-}
-
-/*
- * Each csched-balance step uses its own cpumask. This function determines
- * which one (given the step) and copies it in mask. For the soft affinity
- * balancing step, the pcpus that are not part of vc's hard affinity are
- * filtered out from the result, to avoid running a vcpu where it would
- * like, but is not allowed to!
- */
-static void
-csched_balance_cpumask(const struct vcpu *vc, int step, cpumask_t *mask)
-{
-if ( step == CSCHED_BALANCE_SOFT_AFFINITY )
-{
-cpumask_and(mask, vc->cpu_soft_affinity, vc->cpu_hard_affinity);
-
-if ( unlikely(cpumask_empty(mask)) )
-cpumask_copy(mask, vc->cpu_hard_affinity);
-}
-else /* step == CSCHED_BALANCE_HARD_AFFINITY */
-cpumask_copy(mask, vc->cpu_hard_affinity);
-}
 
 static void burn_credits(struct csched_vcpu *svc, s_time_t now)
 {
@@ -396,18 +331,18 @@ __runq_tickle(unsigned int cpu, struct csched_vcpu *new)
  * Soft and hard affinity balancing loop. For vcpus without
  * a useful soft affinity, consider hard affinity only.
  */
-for_each_csched_balance_step( balance_step )
+for_each_sched_balance_step( balance_step )
 {
 int new_idlers_empty;
 
-if ( balance_step == CSCHED_BALANCE_SOFT_AFFINITY
+if ( balance_step == SCHED_BALANCE_SOFT_AFFINITY
  && !__vcpu_has_soft_affinity(new->vcpu,
   new->vcpu->cpu_hard_affinity) )
 continue;
 
 /* Are there idlers suitable for new (for this balance step)? */
-csched_balance_cpumask(new->vcpu, balance_step,
-   csched_balance_mask);
+sched_balance_cpumask(new->vcpu, balance_step,
+  csched_balance_mask);
 cpumask_and(&idle_mask, prv->idlers, csched_balance_mask);
 new_idlers_empty = cpumask_empty(&idle_mask);
 
@@ -417,7 

[Xen-devel] [PATCH v4 5/5] sched: credit2: add soft affinity awareness to function runq_tickle

[Justin T. Weaver]

by adding two two-step affinity loops.

The first looks for an idle, non-tickled cpu in the given vcpu's soft
affinity, and then in it's hard affinity.

If no cpu was found, the second two-step loop first looks for the non-idle,
non-tickled cpu with the lowest credit in the vcpu's soft affinity. If the
vcpu on the found cpu has less credit than the given vcpu, then that cpu is
chosen. Finally, if no cpu was picked yet, the second step looks for the
non-idle, non-tickled cpu with the lowest credit in the vcpu's hard affinity.

Signed-off-by: Justin T. Weaver 
---
Changes in v4:
 * removed "indent only" patch and integrated its changes into this patch
 * renamed all uses of csched2_cpumask to scratch_mask
 * moved comment outside of for_each_sched_balance_step loop and updated the
   comment for soft affinity in the "idle, not tickled" section
 * updated the functionality of the "not idle, not tickled" section; it now
   breaks out of the for_each_sched_balance_step loop if the vcpu on the cpu
   found during the soft affinity step has less credit than vcpu new
 * updated the comment above the "not idle, not tickled" section explaining
   the new functionality
Changes in v3:
 * replaced use of the on-stack cpumask_t with the per-vcpu scratch_mask
 * added two balance loops, one for finding idle, but not tickled, and other
   for finding non-idle with lowest credit
Changes in v2:
 * Not submitted in version 2; focus was on the hard affinity patch
---
 xen/common/sched_credit2.c |  112 
 1 file changed, 71 insertions(+), 41 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 66f0a20..cd44ac3 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -534,8 +534,8 @@ runq_tickle(const struct scheduler *ops, unsigned int cpu, 
struct csched2_vcpu *
 int i, ipid=-1;
 s_time_t lowest=(1<<30);
 struct csched2_runqueue_data *rqd = RQD(ops, cpu);
-cpumask_t mask;
 struct csched2_vcpu * cur;
+int balance_step;
 
 d2printk("rqt %pv curr %pv\n", new->vcpu, current);
 
@@ -552,57 +552,87 @@ runq_tickle(const struct scheduler *ops, unsigned int 
cpu, struct csched2_vcpu *
 goto tickle;
 }
 
-/* Get a mask of idle, but not tickled, that new is allowed to run on. */
-cpumask_andnot(&mask, &rqd->idle, &rqd->tickled);
-cpumask_and(&mask, &mask, new->vcpu->cpu_hard_affinity);
-
-/* If it's not empty, choose one */
-i = cpumask_cycle(cpu, &mask);
-if ( i < nr_cpu_ids )
+/*
+ * Look for an idle, untickled cpu in the vcpu's soft affinity, then in
+ * its hard affinity.
+ */
+for_each_sched_balance_step ( balance_step )
 {
-ipid = i;
-goto tickle;
-}
+if ( balance_step == SCHED_BALANCE_SOFT_AFFINITY
+&& !__vcpu_has_soft_affinity(new->vcpu,
+new->vcpu->cpu_hard_affinity) )
+continue;
+
+sched_balance_cpumask(new->vcpu, balance_step, scratch_mask);
+cpumask_and(scratch_mask, scratch_mask, &rqd->idle);
+cpumask_andnot(scratch_mask, scratch_mask, &rqd->tickled);
 
-/* Otherwise, look for the non-idle cpu with the lowest credit,
- * skipping cpus which have been tickled but not scheduled yet,
- * that new is allowed to run on. */
-cpumask_andnot(&mask, &rqd->active, &rqd->idle);
-cpumask_andnot(&mask, &mask, &rqd->tickled);
-cpumask_and(&mask, &mask, new->vcpu->cpu_hard_affinity);
+/* If it's not empty, choose one */
+i = cpumask_cycle(cpu, scratch_mask);
+if ( i < nr_cpu_ids )
+{
+ipid = i;
+goto tickle;
+}
+}
 
-for_each_cpu(i, &mask)
+/*
+ * Otherwise, look for the non-idle cpu whose vcpu has the lowest credit,
+ * skipping cpus which have been tickled but not scheduled yet.
+ * First look in new's soft affinity, and choose the cpu if its currently
+ * running vcpu's credit is lower than new's credit.
+ * If a cpu was not found using new's soft affinity, choose the cpu in
+ * new's hard affinity with the lowest credit.
+ */
+for_each_sched_balance_step ( balance_step )
 {
-struct csched2_vcpu * cur;
+if ( balance_step == SCHED_BALANCE_HARD_AFFINITY
+&& lowest < new->credit )
+goto tickle;
 
-/* Already looked at this one above */
-if ( i == cpu )
+if ( balance_step == SCHED_BALANCE_SOFT_AFFINITY
+&& !__vcpu_has_soft_affinity(new->vcpu,
+ new->vcpu->cpu_hard_affinity) )
 continue;
 
-cur = CSCHED2_VCPU(curr_on_cpu(i));
+sched_balance_cpumask(new->vcpu, balance_step, scratch_mask);
+cpumask_and(scratch_mask, scratch_mask, &rqd->active);
+cpumask_andnot(scratch_mask, scratch_mask, &rqd->idle);
+cpumask_andnot(scratch_mask, scratch_mask, &rqd->tickled);
+
+ 

[Xen-devel] [PATCH OSSTEST 1/2] cr-daily-branch: Begin to support other reasons for forcing a baseline.

[Ian Campbell]

By converting the current boolean $force_baseline into a keyword
indicating the reason.

Signed-off-by: Ian Campbell 
---
 cr-daily-branch | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/cr-daily-branch b/cr-daily-branch
index 34b6d2b..7e3e69e 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -47,7 +47,7 @@ determine_version () {
local tversionvar=$1
local tbranch=$2
local treevarwhich=$3
-   if [ "x$tbranch" = "x$branch" ] && ! $force_baseline; then
+   if [ "x$tbranch" = "x$branch" ] && [ "x$force_baseline" = x ]; then
 if [ "x$FORCE_REVISION" != x ]; then
 tversion="$FORCE_REVISION"
 else
@@ -70,7 +70,7 @@ fetch_version () {
 
 treeurl=`./ap-print-url $branch`
 
-force_baseline=false
+force_baseline='' # Non-empty = indication why we are forcing baseline.
 skipidentical=true
 wantpush=$OSSTEST_PUSH
 
@@ -91,7 +91,7 @@ if [ "x$OSSTEST_NO_BASELINE" != xy ] ; then
if [ "x$testedflight" = x ]; then
wantpush=false
skipidentical=false
-   force_baseline=true
+   force_baseline='untested'
if [ "x$treeurl" != xnone: ]; then
treearg=--tree-$tree=$treeurl
fi
@@ -248,7 +248,8 @@ heading=tmp/$flight.heading-info
 : >$heading
 sgr_args+=" --info-headers --include-begin=$heading"
 
-if $force_baseline; then
+case "$force_baseline" in
+untested)
subject_prefix="[$branch baseline test] "
cat >>$heading 

[Xen-devel] [PATCH OSSTEST 2/2] cambridge: arrange to test each new baseline

[Ian Campbell]

Provide a new cr-daily-branch setting OSSTEST_BASELINES_ONLY which
causes it to only attempt to test the current baseline (if it is
untested) and never the tip version. Such tests will not result in any
push.

Add a cronjob to Cambridge which runs in this manner, ensuring that
there will usually be some sort of reasonably up to date baseline for
any given branch which can be used for comparisons in adhoc testing or
bisections.

This will also give us some data on the success of various branches on
the set of machines in Cambridge, which can be useful/interesting.

Signed-off-by: Ian Campbell 
---
 cr-daily-branch   | 13 -
 crontab-cambridge |  1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/cr-daily-branch b/cr-daily-branch
index 7e3e69e..dac28ea 100755
--- a/cr-daily-branch
+++ b/cr-daily-branch
@@ -85,7 +85,11 @@ check_tested () {
  "$@"
 }
 
-if [ "x$OSSTEST_NO_BASELINE" != xy ] ; then
+if [ "x$OSSTEST_BASELINES_ONLY" = xy ] ; then
+force_baseline=baselines-only
+wantpush=false
+skipidentical=true
+elif [ "x$OSSTEST_NO_BASELINE" != xy ] ; then
testedflight=`check_tested --revision-$tree="$OLD_REVISION"`
 
if [ "x$testedflight" = x ]; then
@@ -258,6 +262,13 @@ any, is the most recent actually tested revision.
 
 END
 ;;
+baselines-only)
+#subject-prefix="[... ] "
+cat >> $heading 

[Xen-devel] [GIT-PULL OSSTEST] ap-fetch-version: Arrange for osstest merges from upstream to be stable

[Ian Campbell]

"ap-fetch-version: Arrange for osstest merges from upstream to be
stable" has now passed the Cambridge push gate and done a couple of
merges from upstream as well as some failed attempts, which behaved as
expected. I've dropped a stop file in Cambridge while this pull request
is pending, so we don't end up racing.

The following changes since commit 11e788f7b180ff7e693f7342617318ff01c961c5:

  JobDB/Executive: Improve an internal `die' error (2015-07-09 12:47:38 +0100)

are available in the git repository at:

  git://xenbits.xen.org/people/ianc/osstest.git from-cambridge/2015-07-13

for you to fetch changes up to 43b59cba103da97fd15ab63f570706a1b149970f:

  Automerge of git://xenbits.xen.org/osstest.git master into production 
(2015-07-11 08:35:43 +0100)


Ian Campbell (1):
  ap-fetch-version: Arrange for osstest merges from upstream to be stable

xen.org (2):
  Automerge of git://xenbits.xen.org/osstest.git master into production
  Automerge of git://xenbits.xen.org/osstest.git master into production

 ap-fetch-version | 36 +---
 1 file changed, 33 insertions(+), 3 deletions(-)



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler

[Jan Beulich]

>>> On 11.07.15 at 06:52,  wrote:
> @@ -1162,8 +1176,82 @@ rt_dom_cntl(
>  }
>  spin_unlock_irqrestore(&prv->lock, flags);
>  break;
> +case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
> +spin_lock_irqsave(&prv->lock, flags);
> +for ( index = 0; index < op->u.v.nr_vcpus; index++ )
> +{
> +if ( copy_from_guest_offset(&local_sched,
> +  op->u.v.vcpus, index, 1) )
> +{
> +rc = -EFAULT;
> +break;
> +}
> +if ( local_sched.vcpuid >= d->max_vcpus ||
> +  d->vcpu[local_sched.vcpuid] == NULL )
> +{
> +rc = -EINVAL;
> +break;
> +}
> +svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
> +
> +local_sched.s.rtds.budget = svc->budget / MICROSECS(1);
> +local_sched.s.rtds.period = svc->period / MICROSECS(1);
> +
> +if ( __copy_to_guest_offset(op->u.v.vcpus, index,
> +&local_sched, 1) )
> +{
> +rc = -EFAULT;
> +break;
> +}
> +if( hypercall_preempt_check() )
> +{
> +rc = -ERESTART;
> +break;
> +}

I still don't see how this is supposed to work.

> +}
> +spin_unlock_irqrestore(&prv->lock, flags);
> +break;
> +case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
> +spin_lock_irqsave(&prv->lock, flags);
> +for( index = 0; index < op->u.v.nr_vcpus; index++ )
> +{
> +if ( copy_from_guest_offset(&local_sched,
> +  op->u.v.vcpus, index, 1) )
> +{
> +rc = -EFAULT;
> +break;
> +}
> +if ( local_sched.vcpuid >= d->max_vcpus ||
> +  d->vcpu[local_sched.vcpuid] == NULL )
> +{
> +rc = -EINVAL;
> +break;
> +}
> +svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
> +period = MICROSECS(local_sched.s.rtds.period);
> +budget = MICROSECS(local_sched.s.rtds.budget);
> +if ( period < MICROSECS(10) || period > RTDS_MAX_PERIOD ||
> +  budget < MICROSECS(10) || budget > period )

Apart from numerous coding style issues I think the first of the
checks in this if() is redundant (covered by the combination of
the last two ones) and hence would better be dropped.

> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -1052,10 +1052,22 @@ long sched_adjust(struct domain *d, struct 
> xen_domctl_scheduler_op *op)
>  if ( ret )
>  return ret;
>  
> -if ( (op->sched_id != DOM2OP(d)->sched_id) ||
> - ((op->cmd != XEN_DOMCTL_SCHEDOP_putinfo) &&
> -  (op->cmd != XEN_DOMCTL_SCHEDOP_getinfo)) )
> +if ( op->sched_id != DOM2OP(d)->sched_id )
>  return -EINVAL;
> +else
> +switch ( op->cmd )
> +{
> +case XEN_DOMCTL_SCHEDOP_putinfo:
> +break;
> +case XEN_DOMCTL_SCHEDOP_getinfo:
> +break;
> +case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
> +break;
> +case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
> +break;

Only this break should stay, the three earlier ones should be dropped
as redundant.

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -330,31 +330,56 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
>  #define XEN_SCHEDULER_ARINC653 7
>  #define XEN_SCHEDULER_RTDS 8
>  
> +typedef struct xen_domctl_sched_sedf {
> +uint64_aligned_t period;
> +uint64_aligned_t slice;
> +uint64_aligned_t latency;
> +uint32_t extratime;
> +uint32_t weight;
> +} xen_domctl_sched_sedf_t;
> +
> +typedef struct xen_domctl_sched_credit {
> +uint16_t weight;
> +uint16_t cap;
> +} xen_domctl_sched_credit_t;
> +
> +typedef struct xen_domctl_sched_credit2 {
> +uint16_t weight;
> +} xen_domctl_sched_credit2_t;
> +
> +typedef struct xen_domctl_sched_rtds {
> +uint32_t period;
> +uint32_t budget;
> +} xen_domctl_sched_rtds_t;
> +
> +typedef struct xen_domctl_schedparam_vcpu {
> +union {
> +xen_domctl_sched_credit_t credit;
> +xen_domctl_sched_credit2_t credit2;
> +xen_domctl_sched_rtds_t rtds;
> +} s;
> +uint16_t vcpuid;
> +uint16_t padding;

This pads to a 32-bit boundary, leaving another 32-bit hole.

> +} xen_domctl_schedparam_vcpu_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_schedparam_vcpu_t);
> +
>  /* Set or get info? */
>  #define XEN_DOMCTL_SCHEDOP_putinfo 0
>  #define XEN_DOMCTL_SCHEDOP_getinfo 1
> +#define XEN_DOMCTL_SCHEDOP_putvcpuinfo 2
> +#define XEN_DOMCTL_SCHEDOP_getvcpuinfo 3
>  struct xen_domctl_scheduler_op {
>  uint32_t sched_id;  /* XEN_SCHEDULER_* */
>  uint32_t cmd;   /* XEN_DOMCTL_SCHEDOP_* */
>  union {
> -struct xen_domctl_s

[Xen-devel] [rumpuserxen test] 59489: regressions - FAIL

[osstest service owner]

flight 59489 rumpuserxen real [real]
http://logs.test-lab.xenproject.org/osstest/logs/59489/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-rumpuserxen   5 rumpuserxen-build fail REGR. vs. 33866
 build-i386-rumpuserxen5 rumpuserxen-build fail REGR. vs. 33866
 build-i386-pvops  5 kernel-build  fail REGR. vs. 33866
 build-amd64-pvops 5 kernel-build  fail REGR. vs. 33866

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a

version targeted for testing:
 rumpuserxen  3b91e44996ea6ae1276bce1cc44f38701c53ee6f
baseline version:
 rumpuserxen  30d72f3fc5e35cd53afd82c8179cc0e0b11146ad

Last test of basis33866  2015-01-28 04:19:26 Z  166 days
Failing since 34129  2015-02-03 04:21:40 Z  160 days  115 attempts
Testing same since50441  2015-04-15 20:51:55 Z   88 days   70 attempts


People who touched revisions under test:
  Antti Kantee 
  Ian Jackson 
  Martin Lucina 
  Wei Liu 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-pvopsfail
 build-i386-pvops fail
 build-amd64-rumpuserxen  fail
 build-i386-rumpuserxen   fail
 test-amd64-amd64-rumpuserxen-amd64   blocked 
 test-amd64-i386-rumpuserxen-i386 blocked 



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


Not pushing.

(No revision log; it would be 535 lines long.)

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [libvirt test] 59488: regressions - FAIL

[osstest service owner]

flight 59488 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/59488/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-libvirt5 libvirt-build fail REGR. vs. 58842
 build-i386-pvops  5 kernel-build  fail REGR. vs. 58842
 build-amd64-pvops 5 kernel-build  fail REGR. vs. 58842
 build-armhf-pvops 5 kernel-build  fail REGR. vs. 58842

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-xsm  1 build-check(1)   blocked  n/a

version targeted for testing:
 libvirt  4ffb21c89a6b9ae2b4bfd2999c24b01433e360a9
baseline version:
 libvirt  d10a5f58c75e7eb5943b44cc36a1e768adb2cdb0

Last test of basis58842  2015-06-23 04:23:54 Z   20 days
Failing since 58870  2015-06-24 04:20:11 Z   19 days   17 attempts
Testing same since59428  2015-07-11 04:20:59 Z2 days3 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Boris Fiuczynski 
  Cédric Bosdonnat 
  Daniel Veillard 
  Dmitry Guryanov 
  Eric Blake 
  Erik Skultety 
  Guido Günther 
  Jim Fehlig 
  Jiri Denemark 
  John Ferlan 
  Ján Tomko 
  Laine Stump 
  Luyao Huang 
  Martin Kletzander 
  Maxim Nestratov 
  Michal Dubiel 
  Michal Privoznik 
  Mikhail Feoktistov 
  Nikolay Shirokovskiy 
  Nikolay Shirokovskiy 
  Pavel Fedin 
  Pavel Hrdina 
  Peter Krempa 
  Prerna Saxena 
  Serge Hallyn 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   fail
 build-amd64-pvopsfail
 build-armhf-pvopsfail
 build-i386-pvops fail
 test-amd64-amd64-libvirt-xsm blocked
 test-armhf-armhf-libvirt-xsm blocked
 test-amd64-i386-libvirt-xsm  blocked
 test-amd64-amd64-libvirt blocked
 test-armhf-armhf-libvirt blocked
 test-amd64-i386-libvirt  blocked



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


Not pushing.

(No revision log; it would be 2619 lines long.)

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2] x86/hvm: add support for broadcast of buffered ioreqs...

[Jan Beulich]

>>> On 10.07.15 at 18:07,  wrote:
> @@ -2710,17 +2711,21 @@ int hvm_send_assist_req(struct hvm_ioreq_server *s, 
> ioreq_t *proto_p)
>  return X86EMUL_UNHANDLEABLE;
>  }
>  
> -void hvm_broadcast_assist_req(ioreq_t *p)
> +int hvm_broadcast_ioreq(ioreq_t *p, bool_t buffered)
>  {
>  struct domain *d = current->domain;
>  struct hvm_ioreq_server *s;
> +unsigned int failed = 0;
>  
>  ASSERT(p->type == IOREQ_TYPE_INVALIDATE);
>  
>  list_for_each_entry ( s,
>&d->arch.hvm_domain.ioreq_server.list,
>list_entry )
> -(void) hvm_send_assist_req(s, p);
> +if ( hvm_send_ioreq(s, p, buffered) == X86EMUL_UNHANDLEABLE )
> +failed++;
> +
> +return failed;

I'll try to remember fixing up the mismatch between function return
type and return expression upon commit. Looks good beyond that.

Thanks, Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v8 05/11] xen: grant_table: implement grant_table_warn_active_grants()

[Jan Beulich]

>>> On 10.07.15 at 18:24,  wrote:
> On Tue, Jun 23, 2015 at 06:11:47PM +0200, Vitaly Kuznetsov wrote:
>> Log first 10 active grants of a domain. This function is going to be used
>> for soft reset, active grants on this path usually mean misbehaving backends
>> refusing to release their mappings on shutdown.
> 
> Is there an particular reason 10 was choosen instead of 42 for example :-)
> 
> Also the 10 should probably have an #define for it.

Or even be command line controllable.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Razvan Cojocaru]

On 07/13/2015 11:10 AM, Andrew Cooper wrote:
> On 13/07/2015 08:48, Razvan Cojocaru wrote:
>> Hello,
>>
>> I'm battling the following hypervisor crash with current staging:
>>
>> (d2) Invoking ROMBIOS ...
>> (XEN) stdvga.c:147:d2v0 entering stdvga and caching modes
>> (d2) VGABios $Id: vgabios.c,v 1.67 2008/01/27 09:44:12 vruppert Exp $
>> (XEN) Watchdog timer detects that CPU7 is stuck!
>> (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Not tainted ]
>> (XEN) CPU:7
>> (XEN) RIP:e008:[] _spin_lock+0x31/0x54
>> (XEN) RFLAGS: 0202   CONTEXT: hypervisor (d2v0)
>> (XEN) rax: c11d   rbx: 83041e687970   rcx: c11e
>> (XEN) rdx: 83041e687970   rsi: c11e   rdi: 83041e687978
>> (XEN) rbp: 83040eb37208   rsp: 83040eb37200   r8:  
>> (XEN) r9:     r10: 82d08028c3c0   r11: 
>> (XEN) r12: 83041e687000   r13: 83041e687970   r14: 83040eb37278
>> (XEN) r15: 000c253f   cr0: 8005003b   cr4: 001526e0
>> (XEN) cr3: 0004054a   cr2: 
>> (XEN) ds:    es:    fs:    gs:    ss:    cs: e008
>> (XEN) Xen stack trace from rsp=83040eb37200:
>> (XEN)83040eb37278 83040eb37238 82d0801d09b6 0282
>> (XEN)0008 830403791bf0 83041e687000 83040eb37268
>> (XEN)82d0801cb23a 000c253f 8300d85fc000 0001
>> (XEN)00c2 83040eb37298 82d0801cb410 000c253f
>> (XEN) 00010001 0100 83040eb37328
>> (XEN)82d0801c2403 83040eb37394 83040eb3 
>> (XEN)83040eb37360 00c2 8304054cb000 053f
>> (XEN)0002  83040eb373f4 00c2
>> (XEN)83040eb373d8   82d08028c620
>> (XEN) 83040eb37338 82d0801c3e5d 83040eb37398
>> (XEN)82d0801cb107 00010eb37394 830403791bf0 830403791bf0
>> (XEN)83041e687000 83040eb37398 830403791bf0 0001
>> (XEN)83040eb373d8 0001 000c253f 83040eb373c8
>> (XEN)82d0801cb291 83040eb37b30 8300d85fc000 0001
>> (XEN) 83040eb37428 82d0801bb440 000a0001
>> (XEN)000c253f 00010001 0111 83040eb37478
>> (XEN)0001   0001
>> (XEN)0001 83040eb374a8 82d0801bc0b9 0001
>> (XEN)000c253f 8300d85fc000 000a0001 0100
>> (XEN)83040eb37728 82e00819dc60  83040eb374c8
>> (XEN) Xen call trace:
>> (XEN)[] _spin_lock+0x31/0x54
>> (XEN)[] stdvga_mem_accept+0x3b/0x125
>> (XEN)[] hvm_find_io_handler+0x68/0x8a
>> (XEN)[] hvm_mmio_internal+0x37/0x67
>> (XEN)[] __hvm_copy+0xe9/0x37d
>> (XEN)[] hvm_copy_from_guest_phys+0x14/0x16
>> (XEN)[] hvm_process_io_intercept+0x10b/0x1d6
>> (XEN)[] hvm_io_intercept+0x35/0x5b
>> (XEN)[] hvmemul_do_io+0x1ff/0x2c1
>> (XEN)[] hvmemul_do_io_addr+0x117/0x163
>> (XEN)[] hvmemul_do_mmio_addr+0x24/0x26
>> (XEN)[] hvmemul_rep_movs+0x1ef/0x335
>> (XEN)[] x86_emulate+0x56c9/0x13088
>> (XEN)[] _hvm_emulate_one+0x186/0x281
>> (XEN)[] hvm_emulate_one+0x10/0x12
>> (XEN)[] handle_mmio+0x54/0xd2
>> (XEN)[] handle_mmio_with_translation+0x44/0x46
>> (XEN)[] hvm_hap_nested_page_fault+0x15f/0x589
>> (XEN)[] vmx_vmexit_handler+0x150e/0x188d
>> (XEN)[] vmx_asm_vmexit_handler+0x41/0xc0
>> (XEN)
>> (XEN)
>> (XEN) 
>> (XEN) Panic on CPU 7:
>> (XEN) FATAL TRAP: vector = 2 (nmi)
>> (XEN) [error_code=]
>> (XEN) 
>>
>> At first I thought it was caused by V5 of the vm_event-based
>> introspection series, but I've rolled it back enough to apply V4 on top
>> of it (which has been thoroughly tested on Thursday), and it still
>> happens, so this would at least appear to be unrelated at this point
>> (other than the fact that our use case is maybe somewhat unusual with
>> heavy emulation).
>>
>> I'll keep digging, but since this is a busy time for Xen I thought I'd
>> issue a heads-up here as soon as possible, in case the problem is
>> obvious for somebody and it helps getting it fixed sooner.
> 
> In c/s 3bbaaec09b1b942f5624dee176da6e416d31f982 there is now a
> deliberate split between stdvga_mem_accept() and stdvga_mem_complete()
> about locking and unlocking the stdvga lock.
> 
> At a guess, the previous chain of execution accidentally omitted the
> stdvga_mem_complete() call.

Thanks, I've reverted that patch and the crash is gone. I'll be happy to
test a fix if one is provided, but I don't know enough about that code
to go mes

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Paul Durrant]

> -Original Message-
> From: Andrew Cooper [mailto:am...@hermes.cam.ac.uk] On Behalf Of
> Andrew Cooper
> Sent: 13 July 2015 09:11
> To: Razvan Cojocaru; xen-devel@lists.xen.org
> Cc: Keir (Xen.org); Jan Beulich; Paul Durrant
> Subject: Re: Deadlock in stdvga_mem_accept() with emulation
> 
> On 13/07/2015 08:48, Razvan Cojocaru wrote:
> > Hello,
> >
> > I'm battling the following hypervisor crash with current staging:
> >
> > (d2) Invoking ROMBIOS ...
> > (XEN) stdvga.c:147:d2v0 entering stdvga and caching modes
> > (d2) VGABios $Id: vgabios.c,v 1.67 2008/01/27 09:44:12 vruppert Exp $
> > (XEN) Watchdog timer detects that CPU7 is stuck!
> > (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Not tainted ]
> > (XEN) CPU:7
> > (XEN) RIP:e008:[] _spin_lock+0x31/0x54
> > (XEN) RFLAGS: 0202   CONTEXT: hypervisor (d2v0)
> > (XEN) rax: c11d   rbx: 83041e687970   rcx:
> c11e
> > (XEN) rdx: 83041e687970   rsi: c11e   rdi: 83041e687978
> > (XEN) rbp: 83040eb37208   rsp: 83040eb37200   r8:  
> > (XEN) r9:     r10: 82d08028c3c0   r11: 
> > (XEN) r12: 83041e687000   r13: 83041e687970   r14: 83040eb37278
> > (XEN) r15: 000c253f   cr0: 8005003b   cr4:
> 001526e0
> > (XEN) cr3: 0004054a   cr2: 
> > (XEN) ds:    es:    fs:    gs:    ss:    cs: e008
> > (XEN) Xen stack trace from rsp=83040eb37200:
> > (XEN)83040eb37278 83040eb37238 82d0801d09b6
> 0282
> > (XEN)0008 830403791bf0 83041e687000
> 83040eb37268
> > (XEN)82d0801cb23a 000c253f 8300d85fc000
> 0001
> > (XEN)00c2 83040eb37298 82d0801cb410
> 000c253f
> > (XEN) 00010001 0100
> 83040eb37328
> > (XEN)82d0801c2403 83040eb37394 83040eb3
> 
> > (XEN)83040eb37360 00c2 8304054cb000
> 053f
> > (XEN)0002  83040eb373f4
> 00c2
> > (XEN)83040eb373d8  
> 82d08028c620
> > (XEN) 83040eb37338 82d0801c3e5d
> 83040eb37398
> > (XEN)82d0801cb107 00010eb37394 830403791bf0
> 830403791bf0
> > (XEN)83041e687000 83040eb37398 830403791bf0
> 0001
> > (XEN)83040eb373d8 0001 000c253f
> 83040eb373c8
> > (XEN)82d0801cb291 83040eb37b30 8300d85fc000
> 0001
> > (XEN) 83040eb37428 82d0801bb440
> 000a0001
> > (XEN)000c253f 00010001 0111
> 83040eb37478
> > (XEN)0001  
> 0001
> > (XEN)0001 83040eb374a8 82d0801bc0b9
> 0001
> > (XEN)000c253f 8300d85fc000 000a0001
> 0100
> > (XEN)83040eb37728 82e00819dc60 
> 83040eb374c8
> > (XEN) Xen call trace:
> > (XEN)[] _spin_lock+0x31/0x54
> > (XEN)[] stdvga_mem_accept+0x3b/0x125
> > (XEN)[] hvm_find_io_handler+0x68/0x8a
> > (XEN)[] hvm_mmio_internal+0x37/0x67
> > (XEN)[] __hvm_copy+0xe9/0x37d
> > (XEN)[] hvm_copy_from_guest_phys+0x14/0x16
> > (XEN)[] hvm_process_io_intercept+0x10b/0x1d6
> > (XEN)[] hvm_io_intercept+0x35/0x5b
> > (XEN)[] hvmemul_do_io+0x1ff/0x2c1
> > (XEN)[] hvmemul_do_io_addr+0x117/0x163
> > (XEN)[] hvmemul_do_mmio_addr+0x24/0x26
> > (XEN)[] hvmemul_rep_movs+0x1ef/0x335
> > (XEN)[] x86_emulate+0x56c9/0x13088
> > (XEN)[] _hvm_emulate_one+0x186/0x281
> > (XEN)[] hvm_emulate_one+0x10/0x12
> > (XEN)[] handle_mmio+0x54/0xd2
> > (XEN)[] handle_mmio_with_translation+0x44/0x46
> > (XEN)[] hvm_hap_nested_page_fault+0x15f/0x589
> > (XEN)[] vmx_vmexit_handler+0x150e/0x188d
> > (XEN)[] vmx_asm_vmexit_handler+0x41/0xc0
> > (XEN)
> > (XEN)
> > (XEN) 
> > (XEN) Panic on CPU 7:
> > (XEN) FATAL TRAP: vector = 2 (nmi)
> > (XEN) [error_code=]
> > (XEN) 
> >
> > At first I thought it was caused by V5 of the vm_event-based
> > introspection series, but I've rolled it back enough to apply V4 on top
> > of it (which has been thoroughly tested on Thursday), and it still
> > happens, so this would at least appear to be unrelated at this point
> > (other than the fact that our use case is maybe somewhat unusual with
> > heavy emulation).
> >
> > I'll keep digging, but since this is a busy time for Xen I thought I'd
> > issue a heads-up here as soon as possible, in case the problem is
> > obvious for somebody and it helps getting it fixed sooner.
> 
> In c/s 3bbaaec09b1b942f5624dee176da6e416d31f982 there is now a

Re: [Xen-devel] [PATCH v2] x86/hvm: add support for broadcast of buffered ioreqs...

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 13 July 2015 09:44
> To: Paul Durrant
> Cc: Andrew Cooper; xen-devel@lists.xen.org; Keir (Xen.org)
> Subject: Re: [PATCH v2] x86/hvm: add support for broadcast of buffered
> ioreqs...
> 
> >>> On 10.07.15 at 18:07,  wrote:
> > @@ -2710,17 +2711,21 @@ int hvm_send_assist_req(struct
> hvm_ioreq_server *s, ioreq_t *proto_p)
> >  return X86EMUL_UNHANDLEABLE;
> >  }
> >
> > -void hvm_broadcast_assist_req(ioreq_t *p)
> > +int hvm_broadcast_ioreq(ioreq_t *p, bool_t buffered)
> >  {
> >  struct domain *d = current->domain;
> >  struct hvm_ioreq_server *s;
> > +unsigned int failed = 0;
> >
> >  ASSERT(p->type == IOREQ_TYPE_INVALIDATE);
> >
> >  list_for_each_entry ( s,
> >&d->arch.hvm_domain.ioreq_server.list,
> >list_entry )
> > -(void) hvm_send_assist_req(s, p);
> > +if ( hvm_send_ioreq(s, p, buffered) == X86EMUL_UNHANDLEABLE )
> > +failed++;
> > +
> > +return failed;
> 
> I'll try to remember fixing up the mismatch between function return
> type and return expression upon commit. Looks good beyond that.
> 

Ok. Thanks,

  Paul

> Thanks, Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 0/3] arm64: Add multiboot support (via fdt) for Xen boot

[fu . wei]

From: Fu Wei 

  - This adds support for the Xen boot on ARM specification for arm64.

  - The implementation for Xen is following  :
http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/Multiboot
and xen/docs/misc/arm/device-tree/booting.txt in Xen source code.

  - The multiboot/module commands have existed, so we use 
xen_hypervisor/xen_module instead.

  - This Xen boot support is built into linux module for aarch64,
and can not be used alone.

  - Adding this functionality to the existing "linux" module is for
reusing the existing code of devicetree.

  - Add the support of xen_hypervisor/xen_module commands in 
util/grub.d/20_linux_xen.in

  - Add the introduction of xen_hypervisor/xen_module commands in docs/grub.texi

  - The example of this support is 

https://wiki.linaro.org/LEG/Engineering/Grub2/Xen_booting_on_Foundation_FVP_model_by_GRUB

Changelog:
v2: remove the patches which have been accepted.
according to Vladimir's suggestion, change the command manes
and relevant code:
multiboot-->xen_hypervisor
module-->xen_module
improve the option parsing support for xen_hypervisor/xen_module commands.
add a patch for adding xen_hypervisor/xen_module support
in util/grub.d/20_linux_xen.in.
update docs/grub.texi patch for the new command names.

v1: The first version upstream patchset to grub-devel mailing list


Fu Wei (3):
  arm64: Add Xen boot support file
  * util/grub.d/20_linux_xen.in: Add support of the XEN boot on aarch64
  arm64: Add the introduction of xen_hypervisor/xen_module command in
docs/grub.texi

 docs/grub.texi|  27 ++
 grub-core/Makefile.core.def   |   1 +
 grub-core/loader/arm64/linux.c|   6 +
 grub-core/loader/arm64/xen_boot.c | 615 ++
 include/grub/arm64/xen_boot.h | 115 +++
 util/grub.d/20_linux_xen.in   |  14 +-
 6 files changed, 775 insertions(+), 3 deletions(-)
 create mode 100644 grub-core/loader/arm64/xen_boot.c
 create mode 100644 include/grub/arm64/xen_boot.h

-- 
1.8.3.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 2/3] util/grub.d/20_linux_xen.in: Add arm64 support

[fu . wei]

From: Fu Wei 

This patch adds the support of boot command on arm64 for XEN:
xen_hypervisor
xen_module

Signed-off-by: Fu Wei 
---
 util/grub.d/20_linux_xen.in | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index f532fb9..b52c50d 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -120,16 +120,16 @@ linux_entry ()
 else
 xen_rm_opts="no-real-mode edd=off"
 fi
-   multiboot   ${rel_xen_dirname}/${xen_basename} placeholder 
${xen_args} \${xen_rm_opts}
+   ${multiboot_cmd}${rel_xen_dirname}/${xen_basename} placeholder 
${xen_args} \${xen_rm_opts}
echo'$(echo "$lmessage" | grub_quote)'
-   module  ${rel_dirname}/${basename} placeholder 
root=${linux_root_device_thisversion} ro ${args}
+   ${module_cmd}   ${rel_dirname}/${basename} placeholder 
root=${linux_root_device_thisversion} ro ${args}
 EOF
   if test -n "${initrd}" ; then
 # TRANSLATORS: ramdisk isn't identifier. Should be translated.
 message="$(gettext_printf "Loading initial ramdisk ...")"
 sed "s/^/$submenu_indentation/" << EOF
echo'$(echo "$message" | grub_quote)'
-   module  --nounzip   ${rel_dirname}/${initrd}
+   ${module_cmd}   --nounzip   ${rel_dirname}/${initrd}
 EOF
   fi
   sed "s/^/$submenu_indentation/" << EOF
@@ -185,6 +185,14 @@ case "$machine" in
 *) GENKERNEL_ARCH="$machine" ;;
 esac
 
+if [ "x$machine" != xaarch64 ]; then
+   multiboot_cmd="multiboot"
+   module_cmd="module"
+else
+   multiboot_cmd="xen_hypervisor"
+   module_cmd="xen_module"
+fi
+
 # Extra indentation to add to menu entries in a submenu. We're not in a submenu
 # yet, so it's empty. In a submenu it will be equal to '\t' (one tab).
 submenu_indentation=""
-- 
1.8.3.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86: avoid invalid phys_proc_id reference

[Jan Beulich]

>>> On 13.07.15 at 05:36,  wrote:
> phys_proc_id is invalidated in remove_siblinginfo() which gets called
> before cpu_smpboot_free(). This means calling cpu_to_socket(cpu) in
> cpu_smpboot_free() is not possible to be correct.
> 
> This patch invokes remove_siblinginfo() in cpu_smpboot_free(),
> immediately after the use for cpu_to_socket(cpu).

You having picked that variant of the two I proposed, did you verify
that (as I said when talking about the alternative) there are no
hidden dependencies? If you didn't, or if for whatever else reason
there is any doubt, the less intrusive variant should be chosen at
least for now.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 1/3] arm64: Add Xen boot support file

[fu . wei]

From: Fu Wei 

This patch adds Xen boot support file:
grub-core/loader/arm64/xen_boot.c
include/grub/arm64/xen_boot.h

This patch also adds commands register code and hearder file into
grub-core/loader/arm64/linux.c

  - This adds support for the Xen boot on ARM specification for arm64.
  - The implementation for Xen is following  :
  http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/Multiboot
and xen/docs/misc/arm/device-tree/booting.txt in Xen source code.
  - The multiboot/module commands have existed,
so we use xen_hypervisor/xen_module instead.
  - This Xen boot support is built into linux module for aarch64.
  - Adding this functionality to the existing "linux" module is for
reusing the existing code of devicetree.

Signed-off-by: Fu Wei 
---
 grub-core/Makefile.core.def   |   1 +
 grub-core/loader/arm64/linux.c|   6 +
 grub-core/loader/arm64/xen_boot.c | 615 ++
 include/grub/arm64/xen_boot.h | 115 +++
 4 files changed, 737 insertions(+)
 create mode 100644 grub-core/loader/arm64/xen_boot.c
 create mode 100644 include/grub/arm64/xen_boot.h

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index a6101de..01f8261 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1659,6 +1659,7 @@ module = {
   ia64_efi = loader/ia64/efi/linux.c;
   arm = loader/arm/linux.c;
   arm64 = loader/arm64/linux.c;
+  arm64 = loader/arm64/xen_boot.c;
   fdt = lib/fdt.c;
   common = loader/linux.c;
   common = lib/cmdline.c;
diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index 987f5b9..7ae9bde 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -477,6 +478,9 @@ GRUB_MOD_INIT (linux)
   cmd_devicetree =
 grub_register_command ("devicetree", grub_cmd_devicetree, 0,
   N_("Load DTB file."));
+
+  grub_arm64_linux_register_xen_boot_command (mod, &loaded);
+
   my_mod = mod;
 }
 
@@ -485,4 +489,6 @@ GRUB_MOD_FINI (linux)
   grub_unregister_command (cmd_linux);
   grub_unregister_command (cmd_initrd);
   grub_unregister_command (cmd_devicetree);
+
+  grub_arm64_linux_unregister_xen_boot_command ();
 }
diff --git a/grub-core/loader/arm64/xen_boot.c 
b/grub-core/loader/arm64/xen_boot.c
new file mode 100644
index 000..23bd00e
--- /dev/null
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -0,0 +1,615 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2014  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static grub_dl_t linux_mod;
+static int *loaded;
+
+static struct xen_boot_binary *xen_hypervisor;
+static struct xen_boot_binary *module_head;
+static const grub_size_t module_default_align[] = {
+  MODULE_IMAGE_MIN_ALIGN,
+  MODULE_INITRD_MIN_ALIGN,
+  MODULE_OTHER_MIN_ALIGN,
+  MODULE_CUSTOM_MIN_ALIGN
+};
+
+static void *xen_boot_fdt;
+static const compat_string_struct_t default_compat_string[] = {
+  FDT_COMPATIBLE (MODULE_IMAGE_COMPATIBLE),
+  FDT_COMPATIBLE (MODULE_INITRD_COMPATIBLE),
+  FDT_COMPATIBLE (MODULE_OTHER_COMPATIBLE)
+};
+
+
+/* Parse all the options of xen_module command. For now, we support
+   (1) --type 
+   (2) --nounzip
+   We also set up the type of module in this function.
+   If there are some "--type" options in the command line,
+   we make a custom compatible stream in this function. */
+static grub_err_t
+set_module_type (struct xen_boot_binary *module, int argc, char *argv[],
+int *file_name_index)
+{
+  char **compat_string_temp_array =
+(char **) grub_zalloc (sizeof (char *) * argc);
+  static module_type_t default_type = MODULE_IMAGE;
+  grub_size_t total_size = 0;
+  int num_types = 0, i;
+  char *temp = NULL;
+
+  *file_name_index = 0;
+
+  /* if there are some options we need to process. */
+  while (argc > 1 && !grub_strncmp (argv[0], "--", 2))
+{
+  if (!grub_strcmp (argv[0], "--type"))
+   {
+ module->node_info.type = MODULE_CUSTOM;
+ ARG_SHIFT (argc, argv);
+ total_size += grub_strlen (argv[0]) + 

[Xen-devel] [PATCH v2 3/3] arm64: Add the introduction of Xen boot command

[fu . wei]

From: Fu Wei 

This patch adds the introduction of xen_hypervisor/xen_module commands
in docs/grub.texi

Signed-off-by: Fu Wei 
---
 docs/grub.texi | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/docs/grub.texi b/docs/grub.texi
index b9f41a7..3bd2fc3 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -3839,6 +3839,9 @@ you forget a command, you can run the command 
@command{help}
 @comment * vbeinfo:: List available video modes
 * verify_detached:: Verify detached digital signature
 * videoinfo::   List available video modes
+@comment * xen_hypervisor/xen_module::   Xen boot command, for arm64 only
+* xen_hypervisor::  Load xen hypervisor binary on arm64
+* xen_module::  Load module for xen hypervisor on arm64
 @end menu
 
 
@@ -5102,6 +5105,30 @@ successfully.  If validation fails, it is set to a 
non-zero value.
 List available video modes. If resolution is given, show only matching modes.
 @end deffn
 
+@node xen_hypervisor
+@subsection xen_hypervisor
+
+@deffn Command xen_hypervisor file  [arguments] @dots{}
+Load a Xen hypervisor binary from @var{file}. The rest of the
+line is passed verbatim as the @dfn{kernel command-line}. Any Xen module must
+be reloaded after using this command (@pxref{xen_module}).
+This command is only available on ARM64 systems.
+@end deffn
+
+@node xen_module
+@subsection xen_module
+
+@deffn Command xen_module [--type ] file [arguments]
+Load a module for xen hypervisor binary. The rest of the
+line is passed verbatim as the module command line.
+This command is only available on ARM64 systems.
+
+--type is an option which allow the module command to take "compatible" string.
+This would override default compatible string for this module.
+See 
@uref{http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/Multiboot},
+to obtain more information.
+@end deffn
+
 @node Networking commands
 @section The list of networking commands
 
-- 
1.8.3.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

[Jan Beulich]

>>> On 13.07.15 at 08:47,  wrote:
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -480,6 +480,7 @@ struct xen_domctl_assign_device {
>   } u;
>   /* IN */
>   #define XEN_DOMCTL_DEV_RDM_RELAXED  1
> +#define XEN_DOMCTL_DEV_RDM_MASK 0x1

As said before - I dislike this mask being made part of the public
interface, albeit it being a domctl thing makes it a minor issue.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Andrew Cooper]

On 13/07/15 09:50, Razvan Cojocaru wrote:
> On 07/13/2015 11:10 AM, Andrew Cooper wrote:
>> On 13/07/2015 08:48, Razvan Cojocaru wrote:
>>> Hello,
>>>
>>> I'm battling the following hypervisor crash with current staging:
>>>
>>> (d2) Invoking ROMBIOS ...
>>> (XEN) stdvga.c:147:d2v0 entering stdvga and caching modes
>>> (d2) VGABios $Id: vgabios.c,v 1.67 2008/01/27 09:44:12 vruppert Exp $
>>> (XEN) Watchdog timer detects that CPU7 is stuck!
>>> (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Not tainted ]
>>> (XEN) CPU:7
>>> (XEN) RIP:e008:[] _spin_lock+0x31/0x54
>>> (XEN) RFLAGS: 0202   CONTEXT: hypervisor (d2v0)
>>> (XEN) rax: c11d   rbx: 83041e687970   rcx: c11e
>>> (XEN) rdx: 83041e687970   rsi: c11e   rdi: 83041e687978
>>> (XEN) rbp: 83040eb37208   rsp: 83040eb37200   r8:  
>>> (XEN) r9:     r10: 82d08028c3c0   r11: 
>>> (XEN) r12: 83041e687000   r13: 83041e687970   r14: 83040eb37278
>>> (XEN) r15: 000c253f   cr0: 8005003b   cr4: 001526e0
>>> (XEN) cr3: 0004054a   cr2: 
>>> (XEN) ds:    es:    fs:    gs:    ss:    cs: e008
>>> (XEN) Xen stack trace from rsp=83040eb37200:
>>> (XEN)83040eb37278 83040eb37238 82d0801d09b6 0282
>>> (XEN)0008 830403791bf0 83041e687000 83040eb37268
>>> (XEN)82d0801cb23a 000c253f 8300d85fc000 0001
>>> (XEN)00c2 83040eb37298 82d0801cb410 000c253f
>>> (XEN) 00010001 0100 83040eb37328
>>> (XEN)82d0801c2403 83040eb37394 83040eb3 
>>> (XEN)83040eb37360 00c2 8304054cb000 053f
>>> (XEN)0002  83040eb373f4 00c2
>>> (XEN)83040eb373d8   82d08028c620
>>> (XEN) 83040eb37338 82d0801c3e5d 83040eb37398
>>> (XEN)82d0801cb107 00010eb37394 830403791bf0 830403791bf0
>>> (XEN)83041e687000 83040eb37398 830403791bf0 0001
>>> (XEN)83040eb373d8 0001 000c253f 83040eb373c8
>>> (XEN)82d0801cb291 83040eb37b30 8300d85fc000 0001
>>> (XEN) 83040eb37428 82d0801bb440 000a0001
>>> (XEN)000c253f 00010001 0111 83040eb37478
>>> (XEN)0001   0001
>>> (XEN)0001 83040eb374a8 82d0801bc0b9 0001
>>> (XEN)000c253f 8300d85fc000 000a0001 0100
>>> (XEN)83040eb37728 82e00819dc60  83040eb374c8
>>> (XEN) Xen call trace:
>>> (XEN)[] _spin_lock+0x31/0x54
>>> (XEN)[] stdvga_mem_accept+0x3b/0x125
>>> (XEN)[] hvm_find_io_handler+0x68/0x8a
>>> (XEN)[] hvm_mmio_internal+0x37/0x67
>>> (XEN)[] __hvm_copy+0xe9/0x37d
>>> (XEN)[] hvm_copy_from_guest_phys+0x14/0x16
>>> (XEN)[] hvm_process_io_intercept+0x10b/0x1d6
>>> (XEN)[] hvm_io_intercept+0x35/0x5b
>>> (XEN)[] hvmemul_do_io+0x1ff/0x2c1
>>> (XEN)[] hvmemul_do_io_addr+0x117/0x163
>>> (XEN)[] hvmemul_do_mmio_addr+0x24/0x26
>>> (XEN)[] hvmemul_rep_movs+0x1ef/0x335
>>> (XEN)[] x86_emulate+0x56c9/0x13088
>>> (XEN)[] _hvm_emulate_one+0x186/0x281
>>> (XEN)[] hvm_emulate_one+0x10/0x12
>>> (XEN)[] handle_mmio+0x54/0xd2
>>> (XEN)[] handle_mmio_with_translation+0x44/0x46
>>> (XEN)[] hvm_hap_nested_page_fault+0x15f/0x589
>>> (XEN)[] vmx_vmexit_handler+0x150e/0x188d
>>> (XEN)[] vmx_asm_vmexit_handler+0x41/0xc0
>>> (XEN)
>>> (XEN)
>>> (XEN) 
>>> (XEN) Panic on CPU 7:
>>> (XEN) FATAL TRAP: vector = 2 (nmi)
>>> (XEN) [error_code=]
>>> (XEN) 
>>>
>>> At first I thought it was caused by V5 of the vm_event-based
>>> introspection series, but I've rolled it back enough to apply V4 on top
>>> of it (which has been thoroughly tested on Thursday), and it still
>>> happens, so this would at least appear to be unrelated at this point
>>> (other than the fact that our use case is maybe somewhat unusual with
>>> heavy emulation).
>>>
>>> I'll keep digging, but since this is a busy time for Xen I thought I'd
>>> issue a heads-up here as soon as possible, in case the problem is
>>> obvious for somebody and it helps getting it fixed sooner.
>> In c/s 3bbaaec09b1b942f5624dee176da6e416d31f982 there is now a
>> deliberate split between stdvga_mem_accept() and stdvga_mem_complete()
>> about locking and unlocking the stdvga lock.
>>
>> At a guess, the previous chain of execution accidentally omitted the
>> stdvga_mem_complete() call.
> Thanks, I've reverted tha

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Razvan Cojocaru]

On 07/13/2015 12:00 PM, Andrew Cooper wrote:
> On 13/07/15 09:50, Razvan Cojocaru wrote:
>> On 07/13/2015 11:10 AM, Andrew Cooper wrote:
>>> On 13/07/2015 08:48, Razvan Cojocaru wrote:
 Hello,

 I'm battling the following hypervisor crash with current staging:

 (d2) Invoking ROMBIOS ...
 (XEN) stdvga.c:147:d2v0 entering stdvga and caching modes
 (d2) VGABios $Id: vgabios.c,v 1.67 2008/01/27 09:44:12 vruppert Exp $
 (XEN) Watchdog timer detects that CPU7 is stuck!
 (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Not tainted ]
 (XEN) CPU:7
 (XEN) RIP:e008:[] _spin_lock+0x31/0x54
 (XEN) RFLAGS: 0202   CONTEXT: hypervisor (d2v0)
 (XEN) rax: c11d   rbx: 83041e687970   rcx: c11e
 (XEN) rdx: 83041e687970   rsi: c11e   rdi: 83041e687978
 (XEN) rbp: 83040eb37208   rsp: 83040eb37200   r8:  
 (XEN) r9:     r10: 82d08028c3c0   r11: 
 (XEN) r12: 83041e687000   r13: 83041e687970   r14: 83040eb37278
 (XEN) r15: 000c253f   cr0: 8005003b   cr4: 001526e0
 (XEN) cr3: 0004054a   cr2: 
 (XEN) ds:    es:    fs:    gs:    ss:    cs: e008
 (XEN) Xen stack trace from rsp=83040eb37200:
 (XEN)83040eb37278 83040eb37238 82d0801d09b6 
 0282
 (XEN)0008 830403791bf0 83041e687000 
 83040eb37268
 (XEN)82d0801cb23a 000c253f 8300d85fc000 
 0001
 (XEN)00c2 83040eb37298 82d0801cb410 
 000c253f
 (XEN) 00010001 0100 
 83040eb37328
 (XEN)82d0801c2403 83040eb37394 83040eb3 
 
 (XEN)83040eb37360 00c2 8304054cb000 
 053f
 (XEN)0002  83040eb373f4 
 00c2
 (XEN)83040eb373d8   
 82d08028c620
 (XEN) 83040eb37338 82d0801c3e5d 
 83040eb37398
 (XEN)82d0801cb107 00010eb37394 830403791bf0 
 830403791bf0
 (XEN)83041e687000 83040eb37398 830403791bf0 
 0001
 (XEN)83040eb373d8 0001 000c253f 
 83040eb373c8
 (XEN)82d0801cb291 83040eb37b30 8300d85fc000 
 0001
 (XEN) 83040eb37428 82d0801bb440 
 000a0001
 (XEN)000c253f 00010001 0111 
 83040eb37478
 (XEN)0001   
 0001
 (XEN)0001 83040eb374a8 82d0801bc0b9 
 0001
 (XEN)000c253f 8300d85fc000 000a0001 
 0100
 (XEN)83040eb37728 82e00819dc60  
 83040eb374c8
 (XEN) Xen call trace:
 (XEN)[] _spin_lock+0x31/0x54
 (XEN)[] stdvga_mem_accept+0x3b/0x125
 (XEN)[] hvm_find_io_handler+0x68/0x8a
 (XEN)[] hvm_mmio_internal+0x37/0x67
 (XEN)[] __hvm_copy+0xe9/0x37d
 (XEN)[] hvm_copy_from_guest_phys+0x14/0x16
 (XEN)[] hvm_process_io_intercept+0x10b/0x1d6
 (XEN)[] hvm_io_intercept+0x35/0x5b
 (XEN)[] hvmemul_do_io+0x1ff/0x2c1
 (XEN)[] hvmemul_do_io_addr+0x117/0x163
 (XEN)[] hvmemul_do_mmio_addr+0x24/0x26
 (XEN)[] hvmemul_rep_movs+0x1ef/0x335
 (XEN)[] x86_emulate+0x56c9/0x13088
 (XEN)[] _hvm_emulate_one+0x186/0x281
 (XEN)[] hvm_emulate_one+0x10/0x12
 (XEN)[] handle_mmio+0x54/0xd2
 (XEN)[] handle_mmio_with_translation+0x44/0x46
 (XEN)[] hvm_hap_nested_page_fault+0x15f/0x589
 (XEN)[] vmx_vmexit_handler+0x150e/0x188d
 (XEN)[] vmx_asm_vmexit_handler+0x41/0xc0
 (XEN)
 (XEN)
 (XEN) 
 (XEN) Panic on CPU 7:
 (XEN) FATAL TRAP: vector = 2 (nmi)
 (XEN) [error_code=]
 (XEN) 

 At first I thought it was caused by V5 of the vm_event-based
 introspection series, but I've rolled it back enough to apply V4 on top
 of it (which has been thoroughly tested on Thursday), and it still
 happens, so this would at least appear to be unrelated at this point
 (other than the fact that our use case is maybe somewhat unusual with
 heavy emulation).

 I'll keep digging, but since this is a busy time for Xen I thought I'd
 issue a heads-up here as soon as possible, in case the problem is
 obvious for somebody and it helps getting it fixed sooner.
>>> In c/s 3bbaaec09b1b942f5624dee176da6e416d31f982 there is now a
>>> d

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Paul Durrant]

> -Original Message-
> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
> Sent: 13 July 2015 09:50
> To: Andrew Cooper; xen-devel@lists.xen.org
> Cc: Keir (Xen.org); Jan Beulich; Paul Durrant
> Subject: Re: Deadlock in stdvga_mem_accept() with emulation
> 
> On 07/13/2015 11:10 AM, Andrew Cooper wrote:
> > On 13/07/2015 08:48, Razvan Cojocaru wrote:
> >> Hello,
> >>
> >> I'm battling the following hypervisor crash with current staging:
> >>
> >> (d2) Invoking ROMBIOS ...
> >> (XEN) stdvga.c:147:d2v0 entering stdvga and caching modes
> >> (d2) VGABios $Id: vgabios.c,v 1.67 2008/01/27 09:44:12 vruppert Exp $
> >> (XEN) Watchdog timer detects that CPU7 is stuck!
> >> (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Not tainted ]
> >> (XEN) CPU:7
> >> (XEN) RIP:e008:[] _spin_lock+0x31/0x54
> >> (XEN) RFLAGS: 0202   CONTEXT: hypervisor (d2v0)
> >> (XEN) rax: c11d   rbx: 83041e687970   rcx:
> c11e
> >> (XEN) rdx: 83041e687970   rsi: c11e   rdi: 83041e687978
> >> (XEN) rbp: 83040eb37208   rsp: 83040eb37200   r8:
> 
> >> (XEN) r9:     r10: 82d08028c3c0   r11:
> 
> >> (XEN) r12: 83041e687000   r13: 83041e687970   r14: 83040eb37278
> >> (XEN) r15: 000c253f   cr0: 8005003b   cr4:
> 001526e0
> >> (XEN) cr3: 0004054a   cr2: 
> >> (XEN) ds:    es:    fs:    gs:    ss:    cs: e008
> >> (XEN) Xen stack trace from rsp=83040eb37200:
> >> (XEN)83040eb37278 83040eb37238 82d0801d09b6
> 0282
> >> (XEN)0008 830403791bf0 83041e687000
> 83040eb37268
> >> (XEN)82d0801cb23a 000c253f 8300d85fc000
> 0001
> >> (XEN)00c2 83040eb37298 82d0801cb410
> 000c253f
> >> (XEN) 00010001 0100
> 83040eb37328
> >> (XEN)82d0801c2403 83040eb37394 83040eb3
> 
> >> (XEN)83040eb37360 00c2 8304054cb000
> 053f
> >> (XEN)0002  83040eb373f4
> 00c2
> >> (XEN)83040eb373d8  
> 82d08028c620
> >> (XEN) 83040eb37338 82d0801c3e5d
> 83040eb37398
> >> (XEN)82d0801cb107 00010eb37394 830403791bf0
> 830403791bf0
> >> (XEN)83041e687000 83040eb37398 830403791bf0
> 0001
> >> (XEN)83040eb373d8 0001 000c253f
> 83040eb373c8
> >> (XEN)82d0801cb291 83040eb37b30 8300d85fc000
> 0001
> >> (XEN) 83040eb37428 82d0801bb440
> 000a0001
> >> (XEN)000c253f 00010001 0111
> 83040eb37478
> >> (XEN)0001  
> 0001
> >> (XEN)0001 83040eb374a8 82d0801bc0b9
> 0001
> >> (XEN)000c253f 8300d85fc000 000a0001
> 0100
> >> (XEN)83040eb37728 82e00819dc60 
> 83040eb374c8
> >> (XEN) Xen call trace:
> >> (XEN)[] _spin_lock+0x31/0x54
> >> (XEN)[] stdvga_mem_accept+0x3b/0x125
> >> (XEN)[] hvm_find_io_handler+0x68/0x8a
> >> (XEN)[] hvm_mmio_internal+0x37/0x67
> >> (XEN)[] __hvm_copy+0xe9/0x37d
> >> (XEN)[] hvm_copy_from_guest_phys+0x14/0x16
> >> (XEN)[] hvm_process_io_intercept+0x10b/0x1d6
> >> (XEN)[] hvm_io_intercept+0x35/0x5b
> >> (XEN)[] hvmemul_do_io+0x1ff/0x2c1
> >> (XEN)[] hvmemul_do_io_addr+0x117/0x163
> >> (XEN)[] hvmemul_do_mmio_addr+0x24/0x26
> >> (XEN)[] hvmemul_rep_movs+0x1ef/0x335
> >> (XEN)[] x86_emulate+0x56c9/0x13088
> >> (XEN)[] _hvm_emulate_one+0x186/0x281
> >> (XEN)[] hvm_emulate_one+0x10/0x12
> >> (XEN)[] handle_mmio+0x54/0xd2
> >> (XEN)[] handle_mmio_with_translation+0x44/0x46
> >> (XEN)[] hvm_hap_nested_page_fault+0x15f/0x589
> >> (XEN)[] vmx_vmexit_handler+0x150e/0x188d
> >> (XEN)[] vmx_asm_vmexit_handler+0x41/0xc0
> >> (XEN)
> >> (XEN)
> >> (XEN) 
> >> (XEN) Panic on CPU 7:
> >> (XEN) FATAL TRAP: vector = 2 (nmi)
> >> (XEN) [error_code=]
> >> (XEN) 
> >>
> >> At first I thought it was caused by V5 of the vm_event-based
> >> introspection series, but I've rolled it back enough to apply V4 on top
> >> of it (which has been thoroughly tested on Thursday), and it still
> >> happens, so this would at least appear to be unrelated at this point
> >> (other than the fact that our use case is maybe somewhat unusual with
> >> heavy emulation).
> >>
> >> I'll keep digging, but since this is a busy time for Xen I thought I'd
> >> issue a heads-up here as soon as possible, in case the problem is
> >> obvious for somebody 

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Razvan Cojocaru]

On 07/13/2015 12:01 PM, Paul Durrant wrote:
>> -Original Message-
>> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
>> Sent: 13 July 2015 09:50
>> To: Andrew Cooper; xen-devel@lists.xen.org
>> Cc: Keir (Xen.org); Jan Beulich; Paul Durrant
>> Subject: Re: Deadlock in stdvga_mem_accept() with emulation
>>
>> On 07/13/2015 11:10 AM, Andrew Cooper wrote:
>>> On 13/07/2015 08:48, Razvan Cojocaru wrote:
 Hello,

 I'm battling the following hypervisor crash with current staging:

 (d2) Invoking ROMBIOS ...
 (XEN) stdvga.c:147:d2v0 entering stdvga and caching modes
 (d2) VGABios $Id: vgabios.c,v 1.67 2008/01/27 09:44:12 vruppert Exp $
 (XEN) Watchdog timer detects that CPU7 is stuck!
 (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Not tainted ]
 (XEN) CPU:7
 (XEN) RIP:e008:[] _spin_lock+0x31/0x54
 (XEN) RFLAGS: 0202   CONTEXT: hypervisor (d2v0)
 (XEN) rax: c11d   rbx: 83041e687970   rcx:
>> c11e
 (XEN) rdx: 83041e687970   rsi: c11e   rdi: 83041e687978
 (XEN) rbp: 83040eb37208   rsp: 83040eb37200   r8:
>> 
 (XEN) r9:     r10: 82d08028c3c0   r11:
>> 
 (XEN) r12: 83041e687000   r13: 83041e687970   r14: 83040eb37278
 (XEN) r15: 000c253f   cr0: 8005003b   cr4:
>> 001526e0
 (XEN) cr3: 0004054a   cr2: 
 (XEN) ds:    es:    fs:    gs:    ss:    cs: e008
 (XEN) Xen stack trace from rsp=83040eb37200:
 (XEN)83040eb37278 83040eb37238 82d0801d09b6
>> 0282
 (XEN)0008 830403791bf0 83041e687000
>> 83040eb37268
 (XEN)82d0801cb23a 000c253f 8300d85fc000
>> 0001
 (XEN)00c2 83040eb37298 82d0801cb410
>> 000c253f
 (XEN) 00010001 0100
>> 83040eb37328
 (XEN)82d0801c2403 83040eb37394 83040eb3
>> 
 (XEN)83040eb37360 00c2 8304054cb000
>> 053f
 (XEN)0002  83040eb373f4
>> 00c2
 (XEN)83040eb373d8  
>> 82d08028c620
 (XEN) 83040eb37338 82d0801c3e5d
>> 83040eb37398
 (XEN)82d0801cb107 00010eb37394 830403791bf0
>> 830403791bf0
 (XEN)83041e687000 83040eb37398 830403791bf0
>> 0001
 (XEN)83040eb373d8 0001 000c253f
>> 83040eb373c8
 (XEN)82d0801cb291 83040eb37b30 8300d85fc000
>> 0001
 (XEN) 83040eb37428 82d0801bb440
>> 000a0001
 (XEN)000c253f 00010001 0111
>> 83040eb37478
 (XEN)0001  
>> 0001
 (XEN)0001 83040eb374a8 82d0801bc0b9
>> 0001
 (XEN)000c253f 8300d85fc000 000a0001
>> 0100
 (XEN)83040eb37728 82e00819dc60 
>> 83040eb374c8
 (XEN) Xen call trace:
 (XEN)[] _spin_lock+0x31/0x54
 (XEN)[] stdvga_mem_accept+0x3b/0x125
 (XEN)[] hvm_find_io_handler+0x68/0x8a
 (XEN)[] hvm_mmio_internal+0x37/0x67
 (XEN)[] __hvm_copy+0xe9/0x37d
 (XEN)[] hvm_copy_from_guest_phys+0x14/0x16
 (XEN)[] hvm_process_io_intercept+0x10b/0x1d6
 (XEN)[] hvm_io_intercept+0x35/0x5b
 (XEN)[] hvmemul_do_io+0x1ff/0x2c1
 (XEN)[] hvmemul_do_io_addr+0x117/0x163
 (XEN)[] hvmemul_do_mmio_addr+0x24/0x26
 (XEN)[] hvmemul_rep_movs+0x1ef/0x335
 (XEN)[] x86_emulate+0x56c9/0x13088
 (XEN)[] _hvm_emulate_one+0x186/0x281
 (XEN)[] hvm_emulate_one+0x10/0x12
 (XEN)[] handle_mmio+0x54/0xd2
 (XEN)[] handle_mmio_with_translation+0x44/0x46
 (XEN)[] hvm_hap_nested_page_fault+0x15f/0x589
 (XEN)[] vmx_vmexit_handler+0x150e/0x188d
 (XEN)[] vmx_asm_vmexit_handler+0x41/0xc0
 (XEN)
 (XEN)
 (XEN) 
 (XEN) Panic on CPU 7:
 (XEN) FATAL TRAP: vector = 2 (nmi)
 (XEN) [error_code=]
 (XEN) 

 At first I thought it was caused by V5 of the vm_event-based
 introspection series, but I've rolled it back enough to apply V4 on top
 of it (which has been thoroughly tested on Thursday), and it still
 happens, so this would at least appear to be unrelated at this point
 (other than the fact that our use case is maybe somewhat unusual with
 heavy emulation).

 I'll keep digging, but since this is a busy time for Xen I thought I'd
 issue a heads-up 

Re: [Xen-devel] Question about mapping between domains

[Ian Campbell]

On Thu, 2015-07-09 at 16:31 +0300, Oleksandr Dmytryshyn wrote:
> I have some questions:
> 1. Is this a correct solution?
> 2. Could this solution be considered as a normal (not hack)?
> 3. If not then could anybody help me to implement this in the right way?

The way we deal with this elsewhere in the kernel is that we only ever
do grant mappings over ballooned out pages, which are allocated via
gnttab_alloc_pages. That way when they are unmapped the page is expected
to be entry and no backing mfn is lost. The page can then subsequently
be ballooned back in as normal.

There is an additional quirk for a 1:1 mapped dom0 which is that we
don't actually decrease reservation when ballooning, but keep the 1:1
mfn in anticipation of ballooning it back in later.

If you can't arrange to use already ballooned buffers for your DMA
buffer then you will need to manually balloon it out before and balloon
it back in later.

You may also want to extend the dom0 1:1 quirk described above to your
1:1 mapped domD.

If you have sufficient control over/knowledge of the domD IPA space then
you could also try and arrange that the region used for these mappings
does not correspond to any real RAM in the guest (i.e. stick it in an
MMIO hole). That depends on you never needing to find an associated
struct page though, which will depend on your use case.

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Requesting for freeze exception for VT-d posted-interrupts

[Jan Beulich]

>>> On 13.07.15 at 08:55,  wrote:
> There are two main outstanding issues so far:
> 1. Jan's security concern. I have proposed some solutions but Jan still has
> some problems with my proposals. It would be great if Jan can give a clear
> proposal so that we can discuss and keep making progress.

My proposal was quite clear: The functionality remains experimental,
default off until you can come up with a satisfactory model here.
Giving the impression that I'm the one to propose a model is simply
inadequate: You want the functionality in, so it's primarily you who
should find an implementation that's free of (latent) security issues.
While in general other maintainers may help with this, implying that
if they can't suggest a suitable model code with recognized potential
for security problems can go in _and_ become supported is wrong.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Paul Durrant]

> -Original Message-
> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
> Sent: 13 July 2015 10:03
> To: Paul Durrant; Andrew Cooper; xen-devel@lists.xen.org
> Cc: Keir (Xen.org); Jan Beulich
> Subject: Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation
> 
> On 07/13/2015 12:01 PM, Paul Durrant wrote:
> >> -Original Message-
> >> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
> >> Sent: 13 July 2015 09:50
> >> To: Andrew Cooper; xen-devel@lists.xen.org
> >> Cc: Keir (Xen.org); Jan Beulich; Paul Durrant
> >> Subject: Re: Deadlock in stdvga_mem_accept() with emulation
> >>
> >> On 07/13/2015 11:10 AM, Andrew Cooper wrote:
> >>> On 13/07/2015 08:48, Razvan Cojocaru wrote:
>  Hello,
> 
>  I'm battling the following hypervisor crash with current staging:
> 
>  (d2) Invoking ROMBIOS ...
>  (XEN) stdvga.c:147:d2v0 entering stdvga and caching modes
>  (d2) VGABios $Id: vgabios.c,v 1.67 2008/01/27 09:44:12 vruppert Exp $
>  (XEN) Watchdog timer detects that CPU7 is stuck!
>  (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Not tainted ]
>  (XEN) CPU:7
>  (XEN) RIP:e008:[] _spin_lock+0x31/0x54
>  (XEN) RFLAGS: 0202   CONTEXT: hypervisor (d2v0)
>  (XEN) rax: c11d   rbx: 83041e687970   rcx:
> >> c11e
>  (XEN) rdx: 83041e687970   rsi: c11e   rdi:
> 83041e687978
>  (XEN) rbp: 83040eb37208   rsp: 83040eb37200   r8:
> >> 
>  (XEN) r9:     r10: 82d08028c3c0   r11:
> >> 
>  (XEN) r12: 83041e687000   r13: 83041e687970   r14:
> 83040eb37278
>  (XEN) r15: 000c253f   cr0: 8005003b   cr4:
> >> 001526e0
>  (XEN) cr3: 0004054a   cr2: 
>  (XEN) ds:    es:    fs:    gs:    ss:    cs: e008
>  (XEN) Xen stack trace from rsp=83040eb37200:
>  (XEN)83040eb37278 83040eb37238 82d0801d09b6
> >> 0282
>  (XEN)0008 830403791bf0 83041e687000
> >> 83040eb37268
>  (XEN)82d0801cb23a 000c253f 8300d85fc000
> >> 0001
>  (XEN)00c2 83040eb37298 82d0801cb410
> >> 000c253f
>  (XEN) 00010001 0100
> >> 83040eb37328
>  (XEN)82d0801c2403 83040eb37394 83040eb3
> >> 
>  (XEN)83040eb37360 00c2 8304054cb000
> >> 053f
>  (XEN)0002  83040eb373f4
> >> 00c2
>  (XEN)83040eb373d8  
> >> 82d08028c620
>  (XEN) 83040eb37338 82d0801c3e5d
> >> 83040eb37398
>  (XEN)82d0801cb107 00010eb37394 830403791bf0
> >> 830403791bf0
>  (XEN)83041e687000 83040eb37398 830403791bf0
> >> 0001
>  (XEN)83040eb373d8 0001 000c253f
> >> 83040eb373c8
>  (XEN)82d0801cb291 83040eb37b30 8300d85fc000
> >> 0001
>  (XEN) 83040eb37428 82d0801bb440
> >> 000a0001
>  (XEN)000c253f 00010001 0111
> >> 83040eb37478
>  (XEN)0001  
> >> 0001
>  (XEN)0001 83040eb374a8 82d0801bc0b9
> >> 0001
>  (XEN)000c253f 8300d85fc000 000a0001
> >> 0100
>  (XEN)83040eb37728 82e00819dc60 
> >> 83040eb374c8
>  (XEN) Xen call trace:
>  (XEN)[] _spin_lock+0x31/0x54
>  (XEN)[] stdvga_mem_accept+0x3b/0x125
>  (XEN)[] hvm_find_io_handler+0x68/0x8a
>  (XEN)[] hvm_mmio_internal+0x37/0x67
>  (XEN)[] __hvm_copy+0xe9/0x37d
>  (XEN)[] hvm_copy_from_guest_phys+0x14/0x16
>  (XEN)[] hvm_process_io_intercept+0x10b/0x1d6
>  (XEN)[] hvm_io_intercept+0x35/0x5b
>  (XEN)[] hvmemul_do_io+0x1ff/0x2c1
>  (XEN)[] hvmemul_do_io_addr+0x117/0x163
>  (XEN)[] hvmemul_do_mmio_addr+0x24/0x26
>  (XEN)[] hvmemul_rep_movs+0x1ef/0x335
>  (XEN)[] x86_emulate+0x56c9/0x13088
>  (XEN)[] _hvm_emulate_one+0x186/0x281
>  (XEN)[] hvm_emulate_one+0x10/0x12
>  (XEN)[] handle_mmio+0x54/0xd2
>  (XEN)[]
> handle_mmio_with_translation+0x44/0x46
>  (XEN)[]
> hvm_hap_nested_page_fault+0x15f/0x589
>  (XEN)[] vmx_vmexit_handler+0x150e/0x188d
>  (XEN)[] vmx_asm_vmexit_handler+0x41/0xc0
>  (XEN)
>  (XEN)
>  (XEN) 
>  (XEN) Panic on CPU 7:
>  (XEN) FATAL TRAP: vector = 2 (nmi)
>  (XEN) [error_code=]
>  (XEN) *

Re: [Xen-devel] [PATCH v8 05/11] xen: grant_table: implement grant_table_warn_active_grants()

[Ian Campbell]

On Mon, 2015-07-13 at 09:45 +0100, Jan Beulich wrote:
> >>> On 10.07.15 at 18:24,  wrote:
> > On Tue, Jun 23, 2015 at 06:11:47PM +0200, Vitaly Kuznetsov wrote:
> >> Log first 10 active grants of a domain. This function is going to be used
> >> for soft reset, active grants on this path usually mean misbehaving 
> >> backends
> >> refusing to release their mappings on shutdown.
> > 
> > Is there an particular reason 10 was choosen instead of 42 for example :-)
> > 
> > Also the 10 should probably have an #define for it.
> 
> Or even be command line controllable.

That sounds like overkill to me, what's wrong with some random hardcoded
number for a simple debug aid like this?


Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 05/17] xen/arm: ITS: implement hw_irq_controller for LPIs

[Ian Campbell]

On Sat, 2015-07-11 at 20:10 +0530, Vijay Kilari wrote:
> On Fri, Jul 10, 2015 at 7:16 PM, Ian Campbell  wrote:
> > On Fri, 2015-07-10 at 13:12 +0530, vijay.kil...@gmail.com wrote:
> >> From: Vijaya Kumar K 
> >>
> >> Implements hw_irq_controller api's required
> >> to handle LPI's
> >>
> >> Signed-off-by: Vijaya Kumar K 
> >> ---
> >> v4: - Implement separate hw_irq_controller for LPIs
> >> - Drop setting LPI affinity
> >> - virq and vid are moved under union
> >> - Introduced inv command handling
> >> - its_device is stored in irq_desc
> >> ---
> >>  xen/arch/arm/gic-v3-its.c |  132 
> >> +
> >>  xen/arch/arm/gic-v3.c |5 +-
> >>  xen/arch/arm/gic.c|   32 +++--
> >>  xen/arch/arm/irq.c|   40 ++-
> >>  xen/include/asm-arm/gic-its.h |4 ++
> >>  xen/include/asm-arm/gic.h |   13 
> >>  xen/include/asm-arm/gic_v3_defs.h |1 +
> >>  xen/include/asm-arm/irq.h |8 ++-
> >>  8 files changed, 227 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> >> index b421a6f..b98d396 100644
> >> --- a/xen/arch/arm/gic-v3-its.c
> >> +++ b/xen/arch/arm/gic-v3-its.c
> >> @@ -295,6 +295,19 @@ post:
> >>  its_wait_for_range_completion(its, cmd, next_cmd);
> >>  }
> >>
> >> +static void its_send_inv(struct its_device *dev, struct its_collection 
> >> *col,
> >> + u32 event_id)
> >> +{
> >> +its_cmd_block cmd;
> >> +
> >> +memset(&cmd, 0x0, sizeof(its_cmd_block));
> >> +cmd.inv.cmd = GITS_CMD_INV;
> >> +cmd.inv.devid = dev->device_id;
> >> +cmd.inv.event = event_id;
> >> +
> >> +its_send_single_command(dev->its, &cmd, col);
> >> +}
> >
> > This ought to be in the prior patch doing such things I think.
> >
> > Oh I see, you didn't have struct its_device defined back then. I think
> > you can just reorder patches #3 and #4 to solve that.
> 
>   INV is used only in this patch in lpi_set_config().
> So introduced in this patch

And the other patch introduces every (almost) every other cmd handler.
Having one patch do a bulk add of most commands and then other commands
dribbled in later as they are used just makes the series harder to
follow.

> >> @@ -114,11 +137,12 @@ void gic_route_irq_to_xen(struct irq_desc *desc, 
> >> const cpumask_t *cpu_mask,
> >>unsigned int priority)
> >>  {
> >>  ASSERT(priority <= 0xff); /* Only 8 bits of priority */
> >> -ASSERT(desc->irq < gic_number_lines());/* Can't route interrupts that 
> >> don't exist */
> >> +/* Can't route interrupts that don't exist */
> >> +ASSERT(desc->irq < gic_number_lines() || is_lpi(desc->irq));
> >
> > As discussed in <1436284206.25646.258.ca...@citrix.com> please make some
> > sort of is_valid_irq(irq) helper to encapsulate this logic.
> 
>   I have added it patch#12. I remove this change from this patch

Please fix the ordering of the series so that you don't need to do
things like this. It just wastes review bandwidth since people reading
patch #5 have no idea what is going to happen in #12 and in any case bad
or redundant code shouldn't be added only to be removed later unless
there is really no option (a rare occurrence)

> >> +unsigned int irq_to_vid(struct irq_desc *desc)
> >> +{
> >> +return irq_get_guest_info(desc)->vid;
> >> +}
> >> +
> >> +unsigned int irq_to_virq(struct irq_desc *desc)
> >> +{
> >> +return irq_get_guest_info(desc)->virq;
> >> +}
> >
> > Please assert that irq_desc->arch.its_device is (non-)NULL as
> > appropriate in these two cases.
> 
>These two functions are accessing irq_guest structure not arch.its_device

->vid and ->virq are members of a union. The distinguishing feature
which tells us which one is valid is whether or not
irq_desc->arch.its_device is NULL or not.

Therefore an assertion in each function should be added to catch cases
where people try to get the vid of an SPI or the virq of an LPI.

> >>  #define NR_GIC_LOCAL_IRQS  NR_LOCAL_IRQS
> >>  #define NR_GIC_SGI 16
> >> +#define FIRST_GIC_LPI  8192
> >> +#define NR_GIC_LPI 4096
> >> +#define MAX_LPI(FIRST_GIC_LPI + NR_GIC_LPI)
> >
> > MAX_LPI and NR_GIC_LPI should be obtained from the hardware at init time
> > and put somewhere, like a global nr_lpis perhaps, to be used throughout.
> 
>  This MAX_LPI and NR_GIC_LPI is Xen limitation where in we
> are allocating irq_descriptors statically upto NR_GIC_LPI.

As I said later on, please make this allocation dynamic as described in
the design doc. The static LPI descriptor array used in this series is
not acceptable.

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [v7][PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

[Chen, Tiejun]

Do you mean I should merge them as one as possible?


"Factor it out" means to break out into a separate function (or maybe
a macro or something, but in this case a function is appropriate).  So
in this case take the two sets of similar code, combine them into a
function with appropriate arguments, and then call that function in
both places.

Finding multiple occurrences of very similar code is usually a sign
that refactoring is needed.



Thanks for you explanation.


But seems not be possible because we have seveal combinations of these
two conditions, strategy = LIBXL_RDM_RESERVE_STRATEGY_HOST and one or
pci devices are also passes through.




[snip]


Sorry I can't figure out a good name here :) Any suggestions?


The hypervisor seems to call this `pfn_to_paddr'.


Okay.


Thanks
Tiejun

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 07/17] xen/arm: ITS: Add virtual ITS commands support

[Ian Campbell]

On Sat, 2015-07-11 at 20:19 +0530, Vijay Kilari wrote:
> >> +int vgic_its_process_cmd(struct vcpu *v, struct vgic_its *vits)
> >> +{
> >> +its_cmd_block virt_cmd;
> >> +
> >> +ASSERT(spin_is_locked(&vits->lock));
> >> +
> >> +do {
> >> +if ( vgic_its_read_virt_cmd(v, vits, &virt_cmd) )
> >> +goto err;
> >> +if ( vgic_its_parse_its_command(v, vits, &virt_cmd) )
> >> +goto err;
> >> +vgic_its_update_read_ptr(v, vits);
> >> +} while ( vits->cmd_write != vits->cmd_write_save );
> >
> > I can't find anywhere other than here where vits->cmd_write is touched.
> > What am I missing?
> 
>It is written by guest by GITS_CWRITER emulation in patch #9

Ah, then please reverse the order so that the variable comes first and
the target comes second.

Also I think you need to find a better name that "cmd_write_save".
Something which indicates the progress made perhaps? But why isn't this
just cmd_read? Why the separate progress pointer?

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 06/17] xen/arm: ITS: Add virtual ITS driver

[Ian Campbell]

On Sat, 2015-07-11 at 20:18 +0530, Vijay Kilari wrote:
> On Fri, Jul 10, 2015 at 7:45 PM, Ian Campbell  wrote:
> > On Fri, 2015-07-10 at 13:12 +0530, vijay.kil...@gmail.com wrote:
> >> +static int vits_entry(struct domain *d, paddr_t entry, void *addr,
> >> +  uint32_t size, bool_t set)
> >> +{
> >> [...]
> >> +}
> >> +
> >> +/* ITS device table helper functions */
> >> +static int vits_vdevice_entry(struct domain *d, uint32_t dev_id,
> >> +  struct vdevice_table *entry, bool_t set)
> >> +{
> >> +uint64_t offset;
> >> +paddr_t dt_entry;
> >> +
> >> +BUILD_BUG_ON(sizeof(struct vdevice_table) != 16);
> >> +
> >> +offset = dev_id * sizeof(struct vdevice_table);
> >> +if ( offset > d->arch.vits->dt_size )
> >> +{
> >> +dprintk(XENLOG_G_ERR,
> >> +"%pv: vITS: Out of range offset %ld id 0x%x size %ld\n",
> >> +current, offset, dev_id, d->arch.vits->dt_size);
> >> +return -EINVAL;
> >> +}
> >> +
> >> +dt_entry = d->arch.vits->dt_ipa + offset;
> >> +
> >> +return vits_entry(d, dt_entry, (void *)entry,
> >> +  sizeof(struct vdevice_table),
> >
> > Please drop the (void *) cast here, you can pass a "foo *" to a "void *"
> > without one.
> >
> > It took me a little while to work out why this was void * before I
> > realised that vits_entry was a generic helper used for different types
> > of table. "vits_access_guest_table" to make it clear what it is doing.
> 
>This is also used in later patches read virtual ITS command and also
> property pending table. I prefer to move it to some generic file like
> guestcopy.c/p2m.c?
> and should be named as copy_{from|to}guest_gfn()?

I nearly suggested using the existing copy to/from guest functions but:

Why do the existing copy to/from guest helpers not check the page has
memory type. If it did they would be closer to being directly usable.

Those functions check for guest read/write access as appropriate, but
those do not apply to this case (which is in effect a privileged DMA
from outside the virtual CPU).

In particular due to the second thing I think we would be best off
keeping this as a specific helper for the VITS, having general helper
functions with lax security checks in them just invites people to use
them inappropriately.

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 06/17] xen/arm: ITS: Add virtual ITS driver

[Ian Campbell]

On Sat, 2015-07-11 at 20:18 +0530, Vijay Kilari wrote:
> Hi Ian,
> 
> On Fri, Jul 10, 2015 at 7:24 PM, Ian Campbell  wrote:
> > On Fri, 2015-07-10 at 13:12 +0530, vijay.kil...@gmail.com wrote:
> >> +/* RB-tree helpers for vits_device attached to a domain */
> >
> > In the rest of the series I found this used in three places:
> >   * On assignment, to insert the device into the tree
> >   * On deassignment, to remove it again
> >   * In vgic_vcpu_inject_lpi, where the device is looked up and then
> > never used.
> >
> > I don't see any other use and therefore I don't think this RB tree
> > serves any purpose, which is consistent with the design which doesn't
> > require this lookup anywhere. Please remove it.
> >
> > If there is some use of it in some future series (e.g. perhaps the PCI
> > one) then please still remove it and add a patch to that series to
> > introduce it.
> >
> 
> You mean for now we will remove RB-tree for managing devices assigned
> to domain

Yes, it isn't needed for ITS at all AFAICT and having it around has just
tempted you into using it incorrectly during vpli injection.

>  and introduce RB-tree and do look up when pci-passthrough is
> introduced?.

If it is needed then yes.

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Jan Beulich]

>>> On 13.07.15 at 11:05,  wrote:
> --- a/xen/arch/x86/hvm/stdvga.c
> +++ b/xen/arch/x86/hvm/stdvga.c
> @@ -490,11 +490,18 @@ static bool_t stdvga_mem_accept(const struct 
> hvm_io_handle
>  {
>  struct hvm_hw_stdvga *s = ¤t->domain->arch.hvm_domain.stdvga;
> 
> +/*
> + * The range check must be done without taking any locks, to avoid
> + * deadlock when hvm_mmio_internal() is called from
> + * hvm_copy_to/from_guest_phys() in hvm_process_io_intercept().
> + */
> +if ( (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
> + (hvm_mmio_last_byte(p) >= (VGA_MEM_BASE + VGA_MEM_SIZE)) )
> +return 0;
> +
>  spin_lock(&s->lock);
> 
> -if ( !s->stdvga ||
> - (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
> - (hvm_mmio_last_byte(p) >= (VGA_MEM_BASE + VGA_MEM_SIZE)) )
> +if ( !s->stdvga )
>  goto reject;
> 
>  if ( p->dir == IOREQ_WRITE && p->count > 1 )

But won't the problem continue to exist if the address falls within the
VGA range? I.e. isn't the problem that the two uses of
hvm_mmio_internal() are quite different - while
hvm_hap_nested_page_fault() immediately afterwards calls a
handle_mmio() variant (which would even seem to call for the lock not
getting dropped between them), __hvm_copy() uses it as just a check.

I.e. perhaps better to convert the lock to a recursive one?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 11/17] xen/arm: ITS: Add GICR register emulation

[Ian Campbell]

On Sat, 2015-07-11 at 20:25 +0200, Julien Grall wrote:
> Hi,
> 
> On 10/07/2015 17:10, Ian Campbell wrote:
> > Extra space after the &.
> >> @@ -694,6 +755,14 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, 
> >> mmio_info_t *info)
> >>   *r = ((ncpus - 1) << GICD_TYPE_CPUS_SHIFT |
> >> DIV_ROUND_UP(v->domain->arch.vgic.nr_spis, 32));
> >>
> >> +if ( gic_lpi_supported() )
> >> +{
> >> +irq_bits = gic_nr_id_bits();
> >> +*r |= GICD_TYPE_LPIS;
> >> +}
> >> +else
> >> +irq_bits = get_count_order(vgic_num_irqs(v->domain));
> >
> > I think gic_nr_id_bits should return the correct thing whether or not
> > LPIs are supported, i.e.
> >
> >  if ( gic_lpi_supported() )
> >  *r |= GICD_TYPE_LPIS;
> >  irq_bits = gic_nr_id_bits();
> >
> > should be sufficient.
> 
> Well no. The field GICD_TYPER.IDbits represents the number of bits 
> supported for the interrupt identifier.
> 
> The guest may have a different number of IDbits than the hardware which 
> could be higher (for instance a guest where emulated SPI is supported).

Yes, I really meant vgic_nr_id_bits(), which might for the dom0 case end
up returning something related to the h/w value from the appropriate
vgic hw cfg struct.



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] x86/hvm: Fix deadlock in emulation of rep mov to or from VRAM.

[Paul Durrant]

Razvan Cojocaru reported a hypervisor deadlock with the following stack:

(XEN)[] _spin_lock+0x31/0x54
(XEN)[] stdvga_mem_accept+0x3b/0x125
(XEN)[] hvm_find_io_handler+0x68/0x8a
(XEN)[] hvm_mmio_internal+0x37/0x67
(XEN)[] __hvm_copy+0xe9/0x37d
(XEN)[] hvm_copy_from_guest_phys+0x14/0x16
(XEN)[] hvm_process_io_intercept+0x10b/0x1d6
(XEN)[] hvm_io_intercept+0x35/0x5b
(XEN)[] hvmemul_do_io+0x1ff/0x2c1
(XEN)[] hvmemul_do_io_addr+0x117/0x163
(XEN)[] hvmemul_do_mmio_addr+0x24/0x26
(XEN)[] hvmemul_rep_movs+0x1ef/0x335
(XEN)[] x86_emulate+0x56c9/0x13088
(XEN)[] _hvm_emulate_one+0x186/0x281
(XEN)[] hvm_emulate_one+0x10/0x12
(XEN)[] handle_mmio+0x54/0xd2
(XEN)[] handle_mmio_with_translation+0x44/0x46
(XEN)[] hvm_hap_nested_page_fault+0x15f/0x589
(XEN)[] vmx_vmexit_handler+0x150e/0x188d
(XEN)[] vmx_asm_vmexit_handler+0x41/0xc0

The problem here is the call to hvm_mmio_internal() being made by
__hvm_copy().

When the emulated VRAM access was originally started by
hvm_io_intercept() a few frames up the stack, it would have called
stdvga_mem_accept() which would then have acquired the per-domain
stdvga lock. Unfortunately the call to hvm_mmio_internal(), to avoid
a costly P2M walk, speculatively calls stdvga_mem_accept() again to
see if the page handed to __hvm_copy() is actually an internally
emulated page and hence the vcpu deadlocks.

The fix is to do the range-check in stdvga_mem_accept() without taking
the stdvga lock. This is safe because the range is constant and we know
the I/O will never actually be accepted by the stdvga device model
because hvmemul_do_io_addr() makes sure that the source of the I/O is
actually RAM.

Reported-by: Razvan Cojocaru 
Signed-off-by: Paul Durrant 
Cc: Keir Fraser 
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 xen/arch/x86/hvm/stdvga.c |   13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/stdvga.c b/xen/arch/x86/hvm/stdvga.c
index ebb3b42..6306fa2 100644
--- a/xen/arch/x86/hvm/stdvga.c
+++ b/xen/arch/x86/hvm/stdvga.c
@@ -490,11 +490,18 @@ static bool_t stdvga_mem_accept(const struct 
hvm_io_handler *handler,
 {
 struct hvm_hw_stdvga *s = ¤t->domain->arch.hvm_domain.stdvga;
 
+/*
+ * The range check must be done without taking the lock, to avoid
+ * deadlock when hvm_mmio_internal() is called from
+ * hvm_copy_to/from_guest_phys() in hvm_process_io_intercept().
+ */
+if ( (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
+ (hvm_mmio_last_byte(p) >= (VGA_MEM_BASE + VGA_MEM_SIZE)) )
+return 0;
+
 spin_lock(&s->lock);
 
-if ( !s->stdvga ||
- (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
- (hvm_mmio_last_byte(p) >= (VGA_MEM_BASE + VGA_MEM_SIZE)) )
+if ( !s->stdvga )
 goto reject;
 
 if ( p->dir == IOREQ_WRITE && p->count > 1 )
-- 
1.7.10.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 13 July 2015 10:28
> To: Paul Durrant
> Cc: Razvan Cojocaru; Andrew Cooper; xen-devel@lists.xen.org; Keir
> (Xen.org)
> Subject: RE: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation
> 
> >>> On 13.07.15 at 11:05,  wrote:
> > --- a/xen/arch/x86/hvm/stdvga.c
> > +++ b/xen/arch/x86/hvm/stdvga.c
> > @@ -490,11 +490,18 @@ static bool_t stdvga_mem_accept(const struct
> > hvm_io_handle
> >  {
> >  struct hvm_hw_stdvga *s = ¤t->domain-
> >arch.hvm_domain.stdvga;
> >
> > +/*
> > + * The range check must be done without taking any locks, to avoid
> > + * deadlock when hvm_mmio_internal() is called from
> > + * hvm_copy_to/from_guest_phys() in hvm_process_io_intercept().
> > + */
> > +if ( (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
> > + (hvm_mmio_last_byte(p) >= (VGA_MEM_BASE + VGA_MEM_SIZE))
> )
> > +return 0;
> > +
> >  spin_lock(&s->lock);
> >
> > -if ( !s->stdvga ||
> > - (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
> > - (hvm_mmio_last_byte(p) >= (VGA_MEM_BASE + VGA_MEM_SIZE)) )
> > +if ( !s->stdvga )
> >  goto reject;
> >
> >  if ( p->dir == IOREQ_WRITE && p->count > 1 )
> 
> But won't the problem continue to exist if the address falls within the
> VGA range? I.e. isn't the problem that the two uses of
> hvm_mmio_internal() are quite different - while
> hvm_hap_nested_page_fault() immediately afterwards calls a
> handle_mmio() variant (which would even seem to call for the lock not
> getting dropped between them), __hvm_copy() uses it as just a check.
> 
> I.e. perhaps better to convert the lock to a recursive one?
> 

I think we are ok because the stdvga model will never actually accept the I/O 
since MMIO <-> MMIO rep mov is explicitly disallowed.

  Paul

> Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v8 05/11] xen: grant_table: implement grant_table_warn_active_grants()

[Jan Beulich]

>>> On 13.07.15 at 11:08,  wrote:
> On Mon, 2015-07-13 at 09:45 +0100, Jan Beulich wrote:
>> >>> On 10.07.15 at 18:24,  wrote:
>> > On Tue, Jun 23, 2015 at 06:11:47PM +0200, Vitaly Kuznetsov wrote:
>> >> Log first 10 active grants of a domain. This function is going to be used
>> >> for soft reset, active grants on this path usually mean misbehaving 
> backends
>> >> refusing to release their mappings on shutdown.
>> > 
>> > Is there an particular reason 10 was choosen instead of 42 for example :-)
>> > 
>> > Also the 10 should probably have an #define for it.
>> 
>> Or even be command line controllable.
> 
> That sounds like overkill to me, what's wrong with some random hardcoded
> number for a simple debug aid like this?

>From briefly looking at the code it seemed to be more than just a
debug aid (i.e. failing the operation if the count was exceeded). If
the number indeed only controls how many entries get printed,
then a #define certainly is fine.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [v7][PATCH 16/16] tools: parse to enable new rdm policy parameters

[Chen, Tiejun]

+}else if ( !strcmp(optkey, "rdm_policy") ) {
+if ( !strcmp(tok, "strict") ) {
+pcidev->rdm_policy = LIBXL_RDM_RESERVE_POLICY_STRICT;
+} else if ( !strcmp(tok, "relaxed") ) {
+pcidev->rdm_policy = LIBXL_RDM_RESERVE_POLICY_RELAXED;
+} else {
+XLU__PCI_ERR(cfg, "%s is not an valid PCI RDM property"
+  " policy: 'strict' or 'relaxed'.",
+ tok);
+goto parse_error;
+}


This section has coding style (whitespace) problems and long lines.
If you need to respin, please fix them.


Are you saying this?

} else if (  -> }else if (
} else { -> }else {


Also spurious spaces inside brackets.  Please see CODING_STYLE.


I still can't understand what I'm missing here after compared to other 
contexts inside xlu_pci_parse_bdf(). So I have to paste this entirely,


}else if ( !strcmp(optkey, "rdm_policy") ) {
if ( !strcmp(tok, "strict") ) {
pcidev->rdm_policy = 
LIBXL_RDM_RESERVE_POLICY_STRICT;

}else if ( !strcmp(tok, "relaxed") ) {
pcidev->rdm_policy = 
LIBXL_RDM_RESERVE_POLICY_RELAXED;

}else{
XLU__PCI_ERR(cfg, "%s is not an valid PCI RDM 
property"
  " policy: 'strict' or 
'relaxed'.",

 tok);
goto parse_error;
}
}else{

This is not a long code segment, so could you point them just one by one?




Additionally I don't found which line is over 80 characters.




[snip]


Really I would prefer that this parsing was done with a miniature flex
parser, rather than ad-hoc pointer arithmetic and use of strtok.


Sorry, could you show this explicitly?


Something like what was done for disk devices.  See libxlu_disk_l.l
for an example.  In this case your code would be a lot less
complicated than what you see there.

After the codefreeze I would probably have some time to write it for


Sounds yourself would do this so currently I just keep the original, right?

Thanks
Tiejun


you.  (I think that would be valuable because libxlu_disk_l.l is a
very complicated example, and I want be able to point future
submitters at something simpler.)

Ian.



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v8 05/11] xen: grant_table: implement grant_table_warn_active_grants()

[Vitaly Kuznetsov]

"Jan Beulich"  writes:

 On 13.07.15 at 11:08,  wrote:
>> On Mon, 2015-07-13 at 09:45 +0100, Jan Beulich wrote:
>>> >>> On 10.07.15 at 18:24,  wrote:
>>> > On Tue, Jun 23, 2015 at 06:11:47PM +0200, Vitaly Kuznetsov wrote:
>>> >> Log first 10 active grants of a domain. This function is going to be used
>>> >> for soft reset, active grants on this path usually mean misbehaving 
>> backends
>>> >> refusing to release their mappings on shutdown.
>>> > 
>>> > Is there an particular reason 10 was choosen instead of 42 for example :-)
>>> > 
>>> > Also the 10 should probably have an #define for it.
>>> 
>>> Or even be command line controllable.
>> 
>> That sounds like overkill to me, what's wrong with some random hardcoded
>> number for a simple debug aid like this?
>
> From briefly looking at the code it seemed to be more than just a
> debug aid (i.e. failing the operation if the count was exceeded). If
> the number indeed only controls how many entries get printed,
> then a #define certainly is fine.

Yes, it is just a debug aid in cases something goes wrong in
future. This info is supposed to be useful for hardware domain admin to
help finding misbehaving backends.

-- 
  Vitaly

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [v7][PATCH 16/16] tools: parse to enable new rdm policy parameters

[Ian Campbell]

On Mon, 2015-07-13 at 17:31 +0800, Chen, Tiejun wrote:
> I still can't understand what I'm missing here after compared to other 
> contexts inside xlu_pci_parse_bdf().

Perhaps comparing to the CODING_STYLE document would help?

>  So I have to paste this entirely,
> 
>  }else if ( !strcmp(optkey, "rdm_policy") ) {

Should be:
 } else if (!strcmp(optkey, "rdm_policy")) {

i.e. space after } before "else" and no extra spaces inside the if
condition.

>  if ( !strcmp(tok, "strict") ) {

 if (!strcmp(tok, "strict")) {

Again no spaces within the if.

>  pcidev->rdm_policy = LIBXL_RDM_RESERVE_POLICY_STRICT;
>  }else if ( !strcmp(tok, "relaxed") ) {

Again add a space after } and remove those inside the if condition.

>  pcidev->rdm_policy = 
> LIBXL_RDM_RESERVE_POLICY_RELAXED;
>  }else{

Should be:
 } else {

>  XLU__PCI_ERR(cfg, "%s is not an valid PCI RDM 
> property"
>" policy: 'strict' or 
> 'relaxed'.",
>   tok);
>  goto parse_error;
>  }
>  }else{

and again "} else {"

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Jan Beulich]

>>> On 13.07.15 at 11:30,  wrote:
>>  -Original Message-
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: 13 July 2015 10:28
>> To: Paul Durrant
>> Cc: Razvan Cojocaru; Andrew Cooper; xen-devel@lists.xen.org; Keir
>> (Xen.org)
>> Subject: RE: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation
>> 
>> >>> On 13.07.15 at 11:05,  wrote:
>> > --- a/xen/arch/x86/hvm/stdvga.c
>> > +++ b/xen/arch/x86/hvm/stdvga.c
>> > @@ -490,11 +490,18 @@ static bool_t stdvga_mem_accept(const struct
>> > hvm_io_handle
>> >  {
>> >  struct hvm_hw_stdvga *s = ¤t->domain-
>> >arch.hvm_domain.stdvga;
>> >
>> > +/*
>> > + * The range check must be done without taking any locks, to avoid
>> > + * deadlock when hvm_mmio_internal() is called from
>> > + * hvm_copy_to/from_guest_phys() in hvm_process_io_intercept().
>> > + */
>> > +if ( (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
>> > + (hvm_mmio_last_byte(p) >= (VGA_MEM_BASE + VGA_MEM_SIZE))
>> )
>> > +return 0;
>> > +
>> >  spin_lock(&s->lock);
>> >
>> > -if ( !s->stdvga ||
>> > - (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
>> > - (hvm_mmio_last_byte(p) >= (VGA_MEM_BASE + VGA_MEM_SIZE)) )
>> > +if ( !s->stdvga )
>> >  goto reject;
>> >
>> >  if ( p->dir == IOREQ_WRITE && p->count > 1 )
>> 
>> But won't the problem continue to exist if the address falls within the
>> VGA range? I.e. isn't the problem that the two uses of
>> hvm_mmio_internal() are quite different - while
>> hvm_hap_nested_page_fault() immediately afterwards calls a
>> handle_mmio() variant (which would even seem to call for the lock not
>> getting dropped between them), __hvm_copy() uses it as just a check.
>> 
>> I.e. perhaps better to convert the lock to a recursive one?
> 
> I think we are ok because the stdvga model will never actually accept the 
> I/O since MMIO <-> MMIO rep mov is explicitly disallowed.

True, for now at least.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Razvan Cojocaru]

On 07/13/2015 12:05 PM, Paul Durrant wrote:
>> -Original Message-
>> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
>> Sent: 13 July 2015 10:03
>> To: Paul Durrant; Andrew Cooper; xen-devel@lists.xen.org
>> Cc: Keir (Xen.org); Jan Beulich
>> Subject: Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation
>>
>> On 07/13/2015 12:01 PM, Paul Durrant wrote:
 -Original Message-
 From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
 Sent: 13 July 2015 09:50
 To: Andrew Cooper; xen-devel@lists.xen.org
 Cc: Keir (Xen.org); Jan Beulich; Paul Durrant
 Subject: Re: Deadlock in stdvga_mem_accept() with emulation

 On 07/13/2015 11:10 AM, Andrew Cooper wrote:
> On 13/07/2015 08:48, Razvan Cojocaru wrote:
>> Hello,
>>
>> I'm battling the following hypervisor crash with current staging:
>>
>> (d2) Invoking ROMBIOS ...
>> (XEN) stdvga.c:147:d2v0 entering stdvga and caching modes
>> (d2) VGABios $Id: vgabios.c,v 1.67 2008/01/27 09:44:12 vruppert Exp $
>> (XEN) Watchdog timer detects that CPU7 is stuck!
>> (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Not tainted ]
>> (XEN) CPU:7
>> (XEN) RIP:e008:[] _spin_lock+0x31/0x54
>> (XEN) RFLAGS: 0202   CONTEXT: hypervisor (d2v0)
>> (XEN) rax: c11d   rbx: 83041e687970   rcx:
 c11e
>> (XEN) rdx: 83041e687970   rsi: c11e   rdi:
>> 83041e687978
>> (XEN) rbp: 83040eb37208   rsp: 83040eb37200   r8:
 
>> (XEN) r9:     r10: 82d08028c3c0   r11:
 
>> (XEN) r12: 83041e687000   r13: 83041e687970   r14:
>> 83040eb37278
>> (XEN) r15: 000c253f   cr0: 8005003b   cr4:
 001526e0
>> (XEN) cr3: 0004054a   cr2: 
>> (XEN) ds:    es:    fs:    gs:    ss:    cs: e008
>> (XEN) Xen stack trace from rsp=83040eb37200:
>> (XEN)83040eb37278 83040eb37238 82d0801d09b6
 0282
>> (XEN)0008 830403791bf0 83041e687000
 83040eb37268
>> (XEN)82d0801cb23a 000c253f 8300d85fc000
 0001
>> (XEN)00c2 83040eb37298 82d0801cb410
 000c253f
>> (XEN) 00010001 0100
 83040eb37328
>> (XEN)82d0801c2403 83040eb37394 83040eb3
 
>> (XEN)83040eb37360 00c2 8304054cb000
 053f
>> (XEN)0002  83040eb373f4
 00c2
>> (XEN)83040eb373d8  
 82d08028c620
>> (XEN) 83040eb37338 82d0801c3e5d
 83040eb37398
>> (XEN)82d0801cb107 00010eb37394 830403791bf0
 830403791bf0
>> (XEN)83041e687000 83040eb37398 830403791bf0
 0001
>> (XEN)83040eb373d8 0001 000c253f
 83040eb373c8
>> (XEN)82d0801cb291 83040eb37b30 8300d85fc000
 0001
>> (XEN) 83040eb37428 82d0801bb440
 000a0001
>> (XEN)000c253f 00010001 0111
 83040eb37478
>> (XEN)0001  
 0001
>> (XEN)0001 83040eb374a8 82d0801bc0b9
 0001
>> (XEN)000c253f 8300d85fc000 000a0001
 0100
>> (XEN)83040eb37728 82e00819dc60 
 83040eb374c8
>> (XEN) Xen call trace:
>> (XEN)[] _spin_lock+0x31/0x54
>> (XEN)[] stdvga_mem_accept+0x3b/0x125
>> (XEN)[] hvm_find_io_handler+0x68/0x8a
>> (XEN)[] hvm_mmio_internal+0x37/0x67
>> (XEN)[] __hvm_copy+0xe9/0x37d
>> (XEN)[] hvm_copy_from_guest_phys+0x14/0x16
>> (XEN)[] hvm_process_io_intercept+0x10b/0x1d6
>> (XEN)[] hvm_io_intercept+0x35/0x5b
>> (XEN)[] hvmemul_do_io+0x1ff/0x2c1
>> (XEN)[] hvmemul_do_io_addr+0x117/0x163
>> (XEN)[] hvmemul_do_mmio_addr+0x24/0x26
>> (XEN)[] hvmemul_rep_movs+0x1ef/0x335
>> (XEN)[] x86_emulate+0x56c9/0x13088
>> (XEN)[] _hvm_emulate_one+0x186/0x281
>> (XEN)[] hvm_emulate_one+0x10/0x12
>> (XEN)[] handle_mmio+0x54/0xd2
>> (XEN)[]
>> handle_mmio_with_translation+0x44/0x46
>> (XEN)[]
>> hvm_hap_nested_page_fault+0x15f/0x589
>> (XEN)[] vmx_vmexit_handler+0x150e/0x188d
>> (XEN)[] vmx_asm_vmexit_handler+0x41/0xc0
>> (XEN)
>> (XEN)
>> (XEN) 
>> (XEN) Panic on CPU 7:
>> (XEN) FATAL TRAP: vector = 2 (nmi)
>> (XEN) [error_code=0

Re: [Xen-devel] Interested in taking up a project

[Dario Faggioli]

On Sat, 2015-07-11 at 02:03 +0530, Abhinav Gupta wrote:
> Hi everyone,
>
Hey, :-)

>   I'm sorry for the late update. Actually I had another  project going
> on in parallel, didn't want to distribute efforts.
>
Sure, no problem.

> I went through the implementation approach of powerclamp, it controls
> power consumption by managing C states of the core. This was my
> learning so far. Code makes a  little sense to me, I'll need some more
> time to get hands on powerclamp's code ( I'hv no experience with linux
> kernel code). After this I'll start exploring Xen. 
>
Right. Bear in mind that, with respect to this, Linux and Xen are quite
different. Or at least, that's certainly true for scheduling... for
ACPI, there might be similarities due to the fact that ACPI support in
Xen is inspired to Linux one, but I'm no expert in that, so I don't
really know.

The point I wanted to make was, although some understanding on how
things work in Linux, in order to figure out what PowerClamp really
does, is necessary, start focusing on Xen ASAP, as that is your
target! :-)

> @Dario I'll look into how popular it is in the linux world and if
> there are some real popular real space applications built on top of
> it.  I'll put my findings here.
>
Ok, that would be great.

Thanks and Regards,
Dario

-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [v7][PATCH 13/16] libxl: construct e820 map with RDM information for HVM guest

[Chen, Tiejun]

This approach looks like it should work, and I think given the point in
the release it would be acceptable for 4.6.

However long term I think it might make sense to try and reuse one of
the existing libxl__arch hooks, i.e.
libxl__arch_domain_init_hw_description or
libxl__arch_domain_finalise_hw_description. On ARM these are to do with
setting the Device Tree Blob, which included the memory map, so it is
somewhat morally equivalent to configuring the e820 on x86, I think.

Those hooks are only called from libxl__build_pv today, but calling them
from libxl__build_hvm seems like it would be good too.


But seems this is raising some potential risks, isn't this? Although 
libxl__arch_domain_init_hw_description() and 
libxl__arch_domain_finalise_hw_description() are NOP to x86, they're 
really working on ARM side. So if we call them inside 
libxl__build_hvm(), any affects to ARM? I'm not very sure at this point 
unless anyone can validate this change on ARM, or you really ensure my 
concerns is unnecessary.




In particular I think a call to
libxl__arch_domain_finalise_hw_description could be inserted just before
xc_hvm_build, which is similar to PV where it precedes
xc_dom_build_image, and is where you would want to setup the e820.

libxl__arch_domain_init_hw_description I think would still be a NOP on
x86, but it should probably go either just after the call to
libxl__domain_firmware.

Tiejun, would you be willing to commit to refactoring this and the
issues which Ian raised in response to #11 and #16 a subsequent clean up
series? I don't think it would even need to wait for the freeze to be
over to be posted (although it may need to wait to be applied).



Yes, I'd like to follow this once my concerns above can be eliminated.

Thanks
Tiejun

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [linux-3.4 test] 59477: regressions - trouble: broken/fail/pass

[osstest service owner]

flight 59477 linux-3.4 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/59477/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemut-win7-amd64  6 xen-boot fail in 58831 REGR. vs. 30511

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl-qemut-winxpsp3  3 host-install(3)  broken pass in 59456
 test-amd64-amd64-xl-sedf-pin  6 xen-boot   fail in 58831 pass in 58798
 test-amd64-amd64-pair10 xen-boot/dst_host   fail pass in 58798
 test-amd64-amd64-pair 9 xen-boot/src_host   fail pass in 58798
 test-amd64-i386-pair 10 xen-boot/dst_host   fail pass in 58831
 test-amd64-i386-pair  9 xen-boot/src_host   fail pass in 58831
 test-amd64-i386-xl-qemuu-win7-amd64  9 windows-install  fail pass in 59456

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-xsm   3 host-install(3)   broken baseline untested
 test-amd64-amd64-xl-qemut-win7-amd64  3 host-install(3)  broken like 30496
 test-amd64-i386-libvirt   3 host-install(3)   broken like 32166-bisect
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 6 xen-boot fail baseline untested
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 6 xen-boot fail baseline untested
 test-amd64-amd64-xl-multivcpu  6 xen-boot   fail baseline untested
 test-amd64-amd64-xl-credit2   6 xen-bootfail baseline untested
 test-amd64-amd64-libvirt-xsm  6 xen-bootfail baseline untested
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 6 xen-boot fail baseline untested
 test-amd64-i386-libvirt-xsm   6 xen-bootfail baseline untested
 test-amd64-i386-xl-xsm6 xen-bootfail baseline untested
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 14 guest-localmigrate.2 
fail baseline untested
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 6 xen-boot fail baseline untested
 test-amd64-amd64-xl-rtds  6 xen-bootfail baseline untested
 test-amd64-amd64-xl-sedf  6 xen-boot  fail in 58831 like 30406
 test-amd64-i386-xl-qemut-winxpsp3  6 xen-boot  fail in 58831 like 58808-bisect
 test-amd64-amd64-xl-xsm   6 xen-boot   fail in 59456 baseline untested
 test-amd64-i386-libvirt  11 guest-start   fail in 59456 like 30511
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 30511
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 30511
 test-amd64-amd64-libvirt 11 guest-start  fail   like 30511
 test-amd64-amd64-xl-qemuu-ovmf-amd64  6 xen-bootfail like 53709-bisect
 test-amd64-i386-xl6 xen-bootfail like 53725-bisect
 test-amd64-i386-freebsd10-amd64  6 xen-boot fail like 58780-bisect
 test-amd64-i386-xl-qemuu-winxpsp3  6 xen-boot   fail like 58786-bisect
 test-amd64-i386-qemut-rhel6hvm-intel  6 xen-bootfail like 58788-bisect
 test-amd64-i386-rumpuserxen-i386  6 xen-bootfail like 58799-bisect
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  6 xen-bootfail like 58801-bisect
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  6 xen-boot   fail like 58803-bisect
 test-amd64-amd64-xl-qemut-winxpsp3  6 xen-boot  fail like 58804-bisect
 test-amd64-i386-freebsd10-i386  6 xen-boot  fail like 58805-bisect
 test-amd64-i386-xl-qemuu-ovmf-amd64  6 xen-boot fail like 58806-bisect
 test-amd64-amd64-xl-qemuu-winxpsp3  6 xen-boot  fail like 58807-bisect
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1  6 xen-bootfail like 58809-bisect
 test-amd64-amd64-rumpuserxen-amd64  6 xen-boot  fail like 58810-bisect
 test-amd64-i386-xl-qemuu-debianhvm-amd64  6 xen-bootfail like 58811-bisect
 test-amd64-amd64-xl-qemut-debianhvm-amd64  6 xen-boot   fail like 58813-bisect
 test-amd64-i386-qemuu-rhel6hvm-intel  6 xen-bootfail like 58814-bisect
 test-amd64-i386-xl-qemut-debianhvm-amd64  6 xen-bootfail like 58815-bisect

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt-xsm 12 migrate-support-check fail in 58831 never pass
 test-amd64-amd64-libvirt 12 migrate-support-check fail in 58831 never pass
 test-amd64-i386-libvirt  12 migrate-support-check fail in 58831 never pass
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail in 59456 never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass

version targeted for testing:
 linuxcf1b3dad6c5699b977273276bada8597636ef3e2
baseline version:
 linuxbb4a05a0400ed6d2f1e13d1f82f289ff74300a70

Last test of basis30511  2014-09-29 16:37:46 Z  286 days
Failing since 32004  2014-12-02 04:10:03 Z  223 days  173 attempts
Testing same since58781  2015-06-20 14:15:50 Z   22 day

Re: [Xen-devel] [PATCH v4 11/17] xen/arm: ITS: Add GICR register emulation

[Ian Campbell]

On Mon, 2015-07-13 at 10:28 +0100, Ian Campbell wrote:
> On Sat, 2015-07-11 at 20:25 +0200, Julien Grall wrote:
> > Hi,
> > 
> > On 10/07/2015 17:10, Ian Campbell wrote:
> > > Extra space after the &.
> > >> @@ -694,6 +755,14 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, 
> > >> mmio_info_t *info)
> > >>   *r = ((ncpus - 1) << GICD_TYPE_CPUS_SHIFT |
> > >> DIV_ROUND_UP(v->domain->arch.vgic.nr_spis, 32));
> > >>
> > >> +if ( gic_lpi_supported() )
> > >> +{
> > >> +irq_bits = gic_nr_id_bits();
> > >> +*r |= GICD_TYPE_LPIS;
> > >> +}
> > >> +else
> > >> +irq_bits = get_count_order(vgic_num_irqs(v->domain));
> > >
> > > I think gic_nr_id_bits should return the correct thing whether or not
> > > LPIs are supported, i.e.
> > >
> > >  if ( gic_lpi_supported() )
> > >  *r |= GICD_TYPE_LPIS;
> > >  irq_bits = gic_nr_id_bits();
> > >
> > > should be sufficient.
> > 
> > Well no. The field GICD_TYPER.IDbits represents the number of bits 
> > supported for the interrupt identifier.
> > 
> > The guest may have a different number of IDbits than the hardware which 
> > could be higher (for instance a guest where emulated SPI is supported).
> 
> Yes, I really meant vgic_nr_id_bits(), which might for the dom0 case end
> up returning something related to the h/w value from the appropriate
> vgic hw cfg struct.

Vijay, to be more specific, the number of idbits should be added to
xen/arch/arm/vgic-v3.c:vgic_v3_hw and as a new argument to
vgic_v3_setup_hw to initialise it.

Then vgic_v3_domain_init() should consult vgic_v3_hw in the
is_hardware_domain case to initialise a new field
d->arch.vgic.nr_id_bits.

For the !is_hardware_domain case I suppose it ought to be some hardcoded
value corresponding to whatever the right value is when LPIs are not
supported.

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86: avoid invalid phys_proc_id reference

[Chao Peng]

On Mon, Jul 13, 2015 at 09:55:39AM +0100, Jan Beulich wrote:
> >>> On 13.07.15 at 05:36,  wrote:
> > phys_proc_id is invalidated in remove_siblinginfo() which gets called
> > before cpu_smpboot_free(). This means calling cpu_to_socket(cpu) in
> > cpu_smpboot_free() is not possible to be correct.
> > 
> > This patch invokes remove_siblinginfo() in cpu_smpboot_free(),
> > immediately after the use for cpu_to_socket(cpu).
> 
> You having picked that variant of the two I proposed, did you verify
> that (as I said when talking about the alternative) there are no
> hidden dependencies? If you didn't, or if for whatever else reason
> there is any doubt, the less intrusive variant should be chosen at
> least for now.

I just did some basic tests but I don't think I can conclude that I
verified all the cases.

Because of this, I'm glad to follow your advice to have a gentle fix.

Chao

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [v7][PATCH 16/16] tools: parse to enable new rdm policy parameters

[Chen, Tiejun]

On 2015/7/13 17:40, Ian Campbell wrote:

On Mon, 2015-07-13 at 17:31 +0800, Chen, Tiejun wrote:

I still can't understand what I'm missing here after compared to other
contexts inside xlu_pci_parse_bdf().


Perhaps comparing to the CODING_STYLE document would help?


Looks the whole xlu_pci_parse_bdf() doesn't follow that,

if ( !strcmp(optkey, "msitranslate") ) {
pcidev->msitranslate = atoi(tok);
}else if ( !strcmp(optkey, "power_mgmt") ) {
pcidev->power_mgmt = atoi(tok);
}else if ( !strcmp(optkey, "permissive") ) {
pcidev->permissive = atoi(tok);
}else if ( !strcmp(optkey, "seize") ) {
pcidev->seize = atoi(tok);
}else if ( !strcmp(optkey, "rdm_policy") ) {

So I can do this as you're expecting now, but seems our change would 
make the code style very inconsistent inside this function.


Thanks
Tiejun





  So I have to paste this entirely,

  }else if ( !strcmp(optkey, "rdm_policy") ) {


Should be:
  } else if (!strcmp(optkey, "rdm_policy")) {

i.e. space after } before "else" and no extra spaces inside the if
condition.


  if ( !strcmp(tok, "strict") ) {


  if (!strcmp(tok, "strict")) {

Again no spaces within the if.


  pcidev->rdm_policy = LIBXL_RDM_RESERVE_POLICY_STRICT;
  }else if ( !strcmp(tok, "relaxed") ) {


Again add a space after } and remove those inside the if condition.


  pcidev->rdm_policy = LIBXL_RDM_RESERVE_POLICY_RELAXED;
  }else{


Should be:
  } else {


  XLU__PCI_ERR(cfg, "%s is not an valid PCI RDM
property"
" policy: 'strict' or
'relaxed'.",
   tok);
  goto parse_error;
  }
  }else{


and again "} else {"

Ian.





___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v3] xen/blkfront: convert to blk-mq APIs

[Bob Liu]

Note: This patch is based on original work of Arianna's internship for
GNOME's Outreach Program for Women.

Only one hardware queue is used now, so there is no performance change.

The legacy non-mq code is deleted completely which is the same as other
drivers like virtio, mtip, and nvme.

Also dropped one unnecessary holding of info->io_lock when calling
blk_mq_stop_hw_queues().

Changes in v2:
 - Reorganized blk_mq_queue_rq()
 - Restored most io_locks in place

Change in v3:
 - Rename blk_mq_queue_rq to blkif_queue_rq

Signed-off-by: Arianna Avanzini 
Signed-off-by: Bob Liu 
Reviewed-by: Christoph Hellwig 
Acked-by: Jens Axboe 
---
 drivers/block/xen-blkfront.c |  146 +-
 1 file changed, 60 insertions(+), 86 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 6d89ed3..5b45ee5 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -37,6 +37,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -148,6 +149,7 @@ struct blkfront_info
unsigned int feature_persistent:1;
unsigned int max_indirect_segments;
int is_ready;
+   struct blk_mq_tag_set tag_set;
 };
 
 static unsigned int nr_minors;
@@ -616,54 +618,41 @@ static inline bool blkif_request_flush_invalid(struct 
request *req,
 !(info->feature_flush & REQ_FUA)));
 }
 
-/*
- * do_blkif_request
- *  read a block; request is in a request queue
- */
-static void do_blkif_request(struct request_queue *rq)
+static int blkif_queue_rq(struct blk_mq_hw_ctx *hctx,
+  const struct blk_mq_queue_data *qd)
 {
-   struct blkfront_info *info = NULL;
-   struct request *req;
-   int queued;
-
-   pr_debug("Entered do_blkif_request\n");
-
-   queued = 0;
+   struct blkfront_info *info = qd->rq->rq_disk->private_data;
 
-   while ((req = blk_peek_request(rq)) != NULL) {
-   info = req->rq_disk->private_data;
-
-   if (RING_FULL(&info->ring))
-   goto wait;
+   blk_mq_start_request(qd->rq);
+   spin_lock_irq(&info->io_lock);
+   if (RING_FULL(&info->ring))
+   goto out_busy;
 
-   blk_start_request(req);
+   if (blkif_request_flush_invalid(qd->rq, info))
+   goto out_err;
 
-   if (blkif_request_flush_invalid(req, info)) {
-   __blk_end_request_all(req, -EOPNOTSUPP);
-   continue;
-   }
+   if (blkif_queue_request(qd->rq))
+   goto out_busy;
 
-   pr_debug("do_blk_req %p: cmd %p, sec %lx, "
-"(%u/%u) [%s]\n",
-req, req->cmd, (unsigned long)blk_rq_pos(req),
-blk_rq_cur_sectors(req), blk_rq_sectors(req),
-rq_data_dir(req) ? "write" : "read");
-
-   if (blkif_queue_request(req)) {
-   blk_requeue_request(rq, req);
-wait:
-   /* Avoid pointless unplugs. */
-   blk_stop_queue(rq);
-   break;
-   }
+   flush_requests(info);
+   spin_unlock_irq(&info->io_lock);
+   return BLK_MQ_RQ_QUEUE_OK;
 
-   queued++;
-   }
+out_err:
+   spin_unlock_irq(&info->io_lock);
+   return BLK_MQ_RQ_QUEUE_ERROR;
 
-   if (queued != 0)
-   flush_requests(info);
+out_busy:
+   spin_unlock_irq(&info->io_lock);
+   blk_mq_stop_hw_queue(hctx);
+   return BLK_MQ_RQ_QUEUE_BUSY;
 }
 
+static struct blk_mq_ops blkfront_mq_ops = {
+   .queue_rq = blkif_queue_rq,
+   .map_queue = blk_mq_map_queue,
+};
+
 static int xlvbd_init_blk_queue(struct gendisk *gd, u16 sector_size,
unsigned int physical_sector_size,
unsigned int segments)
@@ -671,9 +660,22 @@ static int xlvbd_init_blk_queue(struct gendisk *gd, u16 
sector_size,
struct request_queue *rq;
struct blkfront_info *info = gd->private_data;
 
-   rq = blk_init_queue(do_blkif_request, &info->io_lock);
-   if (rq == NULL)
+   memset(&info->tag_set, 0, sizeof(info->tag_set));
+   info->tag_set.ops = &blkfront_mq_ops;
+   info->tag_set.nr_hw_queues = 1;
+   info->tag_set.queue_depth =  BLK_RING_SIZE(info);
+   info->tag_set.numa_node = NUMA_NO_NODE;
+   info->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE;
+   info->tag_set.cmd_size = 0;
+   info->tag_set.driver_data = info;
+
+   if (blk_mq_alloc_tag_set(&info->tag_set))
return -1;
+   rq = blk_mq_init_queue(&info->tag_set);
+   if (IS_ERR(rq)) {
+   blk_mq_free_tag_set(&info->tag_set);
+   return -1;
+   }
 
queue_flag_set_unlocked(QUEUE_FLAG_VIRT, rq);
 
@@ -901,19 +903,15 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
 static void xlvbd_rele

Re: [Xen-devel] [PATCH 1/9] libxl: fix libxl__abs_path

[Ian Campbell]

On Fri, 2015-07-10 at 19:00 +0100, Wei Liu wrote:

I rather dislike subjects of the form "fix $function", since it gives
very little clue to someone reading the shortlog what is going on.

In this case I think "libxl: make libxl__abs_path correctly handle a
NULL argument" would be an accurate description.

> If s is NULL, just return NULL to avoid libxl__strdup dereferencing NULL
> pointer.
> 
> Signed-off-by: Wei Liu 

For the change itself:
Acked-by: Ian Campbell 

> ---
>  tools/libxl/libxl_internal.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index 42d548e..6402c1b 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -233,8 +233,8 @@ void libxl__log(libxl_ctx *ctx, xentoollog_level 
> msglevel, int errnoval,
>  
>  char *libxl__abs_path(libxl__gc *gc, const char *s, const char *path)
>  {
> -if (!s || s[0] == '/')
> -return libxl__strdup(gc, s);
> +if (!s) return NULL;
> +if (s[0] == '/') return libxl__strdup(gc, s);
>  return libxl__sprintf(gc, "%s/%s", path, s);
>  }
>  



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/9] libxl: fix libxl__abs_path

[Wei Liu]

On Mon, Jul 13, 2015 at 10:57:32AM +0100, Ian Campbell wrote:
> On Fri, 2015-07-10 at 19:00 +0100, Wei Liu wrote:
> 
> I rather dislike subjects of the form "fix $function", since it gives
> very little clue to someone reading the shortlog what is going on.
> 
> In this case I think "libxl: make libxl__abs_path correctly handle a
> NULL argument" would be an accurate description.

Ack.

I can resend the whole series after updating subject for patch #1, #8
and #9.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/9] libxl: turn two malloc's to libxl__malloc

[Ian Campbell]

On Fri, 2015-07-10 at 19:00 +0100, Wei Liu wrote:
> One is to combine malloc + libxl__alloc_failed. The other is to avoid
> dereferencing NULL pointer in case of malloc failure.

The non-use of a gc for the latter in particular looks a bit suspicious
to me, but nonetheless this is an improvement:

> Signed-off-by: Wei Liu 

Acked-by: Ian Campbell 

> ---
>  tools/libxl/libxl_aoutils.c | 3 +--
>  tools/libxl/libxl_dm.c  | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
> index 0931eee..0300396 100644
> --- a/tools/libxl/libxl_aoutils.c
> +++ b/tools/libxl/libxl_aoutils.c
> @@ -245,8 +245,7 @@ static void datacopier_readable(libxl__egc *egc, 
> libxl__ev_fd *ev,
>  
>  buf = LIBXL_TAILQ_LAST(&dc->bufs, libxl__datacopier_bufs);
>  if (!buf || buf->used >= sizeof(buf->buf)) {
> -buf = malloc(sizeof(*buf));
> -if (!buf) libxl__alloc_failed(CTX, __func__, 1, 
> sizeof(*buf));
> +buf = libxl__malloc(NOGC, sizeof(*buf));
>  buf->used = 0;
>  LIBXL_TAILQ_INSERT_TAIL(&dc->bufs, buf, entry);
>  }
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index ad434f0..0cc73be 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1010,7 +1010,7 @@ static int libxl__write_stub_dmargs(libxl__gc *gc,
>  i++;
>  }
>  dmargs_size++;
> -dmargs = (char *) malloc(dmargs_size);
> +dmargs = (char *) libxl__malloc(NOGC, dmargs_size);
>  i = 1;
>  dmargs[0] = '\0';
>  while (args[i] != NULL) {



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 3/9] libxl: json string object can be NULL

[Ian Campbell]

On Fri, 2015-07-10 at 19:00 +0100, Wei Liu wrote:
> Signed-off-by: Wei Liu 

Acked-by: Ian Campbell 

It occurs to me that since libxl__strdup can never return NULL due to
allocation failure we could consider make libxl__strdup(gc, NULL) be
well defined as returning NULL.

> ---
>  tools/libxl/libxl_json.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
> index 346929a..652b3f4 100644
> --- a/tools/libxl/libxl_json.c
> +++ b/tools/libxl/libxl_json.c
> @@ -433,8 +433,13 @@ int libxl__string_parse_json(libxl__gc *gc, const 
> libxl__json_object *o,
>  
>  if (libxl__json_object_is_null(o))
>  *p = NULL;
> -else
> -*p = libxl__strdup(NOGC, libxl__json_object_get_string(o));
> +else {
> +const char *s = libxl__json_object_get_string(o);
> +if (!s)
> +*p = NULL;
> +else
> +*p = libxl__strdup(NOGC, s);
> +}
>  
>  return 0;
>  }



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/9] libxl: dispose dominfo to avoid leaking resource

[Ian Campbell]

On Fri, 2015-07-10 at 19:00 +0100, Wei Liu wrote:
> Add libxl_dominfo_dispose to one return path that doesn't have it.
> 
> Signed-off-by: Wei Liu 

That return is a bit at odds with the generally correct error handling
in that function, but this improves things at least a little and I can
sort of see why this a slightly special case, so:

Acked-by: Ian Campbell 

> ---
>  tools/libxl/libxl_device.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 2493972..3f8b555 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -816,6 +816,8 @@ void libxl__initiate_device_remove(libxl__egc *egc,
> be_path);
>  goto out;
>  }
> +
> +libxl_dominfo_dispose(&info);
>  return;
>  }
>  }



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 5/9] libxl: avoid leaking string in cpupool_info

[Ian Campbell]

On Fri, 2015-07-10 at 19:00 +0100, Wei Liu wrote:
> Signed-off-by: Wei Liu 

Would an "if (rc) libxl_cpupoolinfo_dispose(info)" on the exit path be a
more robust alternative? Might require the addition of a
libxl_cpupoolinfo_init() somewhere before any possible error.

> ---
>  tools/libxl/libxl.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 38aff8d..4151dcb 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -740,8 +740,10 @@ static int cpupool_info(libxl__gc *gc,
>  info->sched = xcinfo->sched_id;
>  info->n_dom = xcinfo->n_dom;
>  rc = libxl_cpu_bitmap_alloc(CTX, &info->cpumap, 0);
> -if (rc)
> +if (rc) {
> +free(info->pool_name);
>  goto out;
> +}
>  
>  memcpy(info->cpumap.map, xcinfo->cpumap, info->cpumap.size);
>  



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V5 7/7] domcreate: support pvusb in configuration file

[Juergen Gross]

On 06/25/2015 12:07 PM, Chunyan Liu wrote:

Add code to support pvusb in domain config file. One could specify
usbctrl and usb in domain's configuration file and create domain,
then usb controllers will be created and usb device would be attached
to guest automatically.

One could specify usb controllers and usb devices in config file
like this:
usbctrl=['version=2,ports=4', 'version=1,ports=4', ]
usbdev=['2.1,controller=0,port=1', ]

Signed-off-by: Chunyan Liu 
Signed-off-by: Simon Cao 


I tested this patch and it still isn't working:

config file:

usbctrl = [ 'version=2,ports=4', ]
usbdev = [ '3.4,controller=0,port=1', ]

xl create output:

xl: libxl_event.c:1759: libxl__ao_inprogress_gc: Assertion 
`!ao->complete' failed.


I'm not sure, but comparing e.g. libxl__device_disk_add() with
libxl__device_usb_add() shows a significant difference regarding passing
of ao data.

I think you'll need a wrapper as in the disk case and pass NULL for ao
to the attach function being capable of async operation when you are
calling the attach function during domain creation.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V5 0/7] xen pvusb toolstack work

[Juergen Gross]

On 06/25/2015 12:07 PM, Chunyan Liu wrote:

This patch series is to add pvusb toolstack work, supporting hot add|remove
USB device to|from guest and specify USB device in domain configuration file.


Patches 1-6:

Tested-by: Juergen Gross 


Juergen



Changes to v4:
* use DEFINE_DEVICE_ADD and DEFINE_DEVICES_ADD to handle usbctrl adding
   and usb adding, define extended macro DEFINE_DEVICE_REMOVE_EXT to handle
   usbctrl remove.
* Change interfaces:
   libxl_device_usb only includes bus.addr, removing busid.
   'xl usb-detach' uses  to specify usb device instead of bus.addr.
   Adjusting all related codes.
* Other changes addring all other comments in v4.

V3 is here:
http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg01327.html

Related Discussion Threads:
http://www.redhat.com/archives/libvir-list/2014-June/msg00038.html
http://lists.xen.org/archives/html/xen-devel/2014-06/msg00086.html

   <<< pvusb work introduction >>>

1. Overview

There are two general methods for passing through individual host
devices to a guest. The first is via an emulated USB device
controller; the second is PVUSB.

Additionally, there are two ways to add USB devices to a guest: via
the config file at domain creation time, and via hot-plug while the VM
is running.

* Emulated USB

In emulated USB, the device model (qemu) presents an emulated USB
controller to the guest. The device model process then grabs control
of the device from domain 0 and and passes the USB commands between
the guest OS and the host USB device.

This method is only available to HVM domains, and is not available for
domains running with device model stubdomains.

* PVUSB

PVUSB uses a paravirtialized front-end/back-end interface, similar to
the traditional Xen PV network and disk protocols. In order to use
PVUSB, you need usbfront in your guest OS, and usbback in dom0 (or
your USB driver domain).

2. Specifying a host USB device

QEMU qmp commands allows USB devices to be specified either by their
bus address (in the form bus.device) or their device tag (in the form
vendorid:deviceid).

Each way of specifying has its advantages:

 Specifying by device tag will always get the same device,
regardless of where the device ends up in the USB bus topology.
However, if there are two identical devices, it will not allow you to
specify which one.

 Specifying by bus address will always allow you to choose a
specific device, even if you have duplicates. However, the bus address
may change depending on which port you plugged the device into, and
possibly also after a reboot.

To avoid duplication of vendorid:deviceid, we'll use bus address to
specify host USB device in xl toolstack.

You can use lsusb to list the USB devices on the system:

Bus 001 Device 003: ID 0424:2514 Standard Microsystems Corp. USB 2.0
Hub
Bus 003 Device 002: ID f617:0905
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 001 Device 004: ID 0424:2640 Standard Microsystems Corp. USB 2.0
Hub
Bus 001 Device 005: ID 0424:4060 Standard Microsystems Corp. Ultra
Fast Media Reader
Bus 001 Device 006: ID 046d:c016 Logitech, Inc. Optical Wheel Mouse

To pass through the Logitec mouse, for instance, you could specify
1.6 (remove leading zeroes).

Note: USB hubs can not be assigned to guest.

3. PVUSB toolstack

* Specify USB device in xl config file

You can just specify usb devices, like:
usbdev=['1.6']

Then it will create a USB controller automatically and attach the USB
device to the first available USB controller:port.

or, you can explicitly specify usb controllers and usb devices, like:
usbctrl=['verison=1, ports=4', 'version=2, ports=8', ]
usbdev=['1.6, controller=0, port=1']

Then it will create two USB controllers as you specified.
And if controller and port are specified in usb config, then it will
attach the USB device to that controller:port. About the controller
and port value:
Each USB controller has a index (or called devid) based on 0. The 1st
controller has index 0, the 2nd controller has index 1, ...
Under controller, each port has a port number based on 1. In above
configuration, the 1st controller will have port 1,2,3,4.

* Hot-Plug USB device

To attach a USB device, you should first create a USB controller.
e.g.
xl usb-ctrl-attach domain [version=1|2] [ports=value]
By default, it will create a USB2.0 controller with 8 ports.

Then you could attach a USB device.
e.g.
xl usb-attach domain 1.6 [controller=index port=number]
By default, it will find the 1st available controller:port to attach
the USB device.

You could view USB device status of the domain by usb-list.
e.g.
xl usb-list domain
It will list USB controllers and USB devices under each controller.

You could detach a USB device with usb-detach command.
e.g.
xl usb-detach domain 1.6

You can also remove the whole USB controller by usb-ctrl-detach
command.
e.g.
xl usb-ctrl-detach domain 0
It will remove the USB controller with index 0 and all USB devices
under it.

4. PVUSB Lib

Re: [Xen-devel] [PATCH 6/9] libxl: localtime(3) can return NULL

[Ian Campbell]

On Fri, 2015-07-10 at 19:00 +0100, Wei Liu wrote:
> Signed-off-by: Wei Liu 

Acked-by: Ian Campbell 



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 7/9] libxl: qmp_init_handler can return NULL

[Ian Campbell]

On Fri, 2015-07-10 at 19:00 +0100, Wei Liu wrote:
> Signed-off-by: Wei Liu 

Acked-by: Ian Campbell 

(although the only actual reason for a failure today is a memory
allocation failure, which ought to abort really).

> ---
>  tools/libxl/libxl_qmp.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index 6484f5e..965c507 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -694,6 +694,7 @@ libxl__qmp_handler *libxl__qmp_initialize(libxl__gc *gc, 
> uint32_t domid)
>  char *qmp_socket;
>  
>  qmp = qmp_init_handler(gc, domid);
> +if (!qmp) return NULL;
>  
>  qmp_socket = GCSPRINTF("%s/qmp-libxl-%d", libxl__run_dir_path(), domid);
>  if ((ret = qmp_open(qmp, qmp_socket, QMP_SOCKET_CONNECT_TIMEOUT)) < 0) {



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

[Paul Durrant]

> -Original Message-
> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
> Sent: 13 July 2015 10:42
> To: Paul Durrant; Andrew Cooper; xen-devel@lists.xen.org
> Cc: Keir (Xen.org); Jan Beulich
> Subject: Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation
> 
> On 07/13/2015 12:05 PM, Paul Durrant wrote:
> >> -Original Message-
> >> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
> >> Sent: 13 July 2015 10:03
> >> To: Paul Durrant; Andrew Cooper; xen-devel@lists.xen.org
> >> Cc: Keir (Xen.org); Jan Beulich
> >> Subject: Re: [Xen-devel] Deadlock in stdvga_mem_accept() with
> emulation
> >>
> >> On 07/13/2015 12:01 PM, Paul Durrant wrote:
>  -Original Message-
>  From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
>  Sent: 13 July 2015 09:50
>  To: Andrew Cooper; xen-devel@lists.xen.org
>  Cc: Keir (Xen.org); Jan Beulich; Paul Durrant
>  Subject: Re: Deadlock in stdvga_mem_accept() with emulation
> 
>  On 07/13/2015 11:10 AM, Andrew Cooper wrote:
> > On 13/07/2015 08:48, Razvan Cojocaru wrote:
> >> Hello,
> >>
> >> I'm battling the following hypervisor crash with current staging:
> >>
> >> (d2) Invoking ROMBIOS ...
> >> (XEN) stdvga.c:147:d2v0 entering stdvga and caching modes
> >> (d2) VGABios $Id: vgabios.c,v 1.67 2008/01/27 09:44:12 vruppert Exp
> $
> >> (XEN) Watchdog timer detects that CPU7 is stuck!
> >> (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Not tainted ]
> >> (XEN) CPU:7
> >> (XEN) RIP:e008:[] _spin_lock+0x31/0x54
> >> (XEN) RFLAGS: 0202   CONTEXT: hypervisor (d2v0)
> >> (XEN) rax: c11d   rbx: 83041e687970   rcx:
>  c11e
> >> (XEN) rdx: 83041e687970   rsi: c11e   rdi:
> >> 83041e687978
> >> (XEN) rbp: 83040eb37208   rsp: 83040eb37200   r8:
>  
> >> (XEN) r9:     r10: 82d08028c3c0   r11:
>  
> >> (XEN) r12: 83041e687000   r13: 83041e687970   r14:
> >> 83040eb37278
> >> (XEN) r15: 000c253f   cr0: 8005003b   cr4:
>  001526e0
> >> (XEN) cr3: 0004054a   cr2: 
> >> (XEN) ds:    es:    fs:    gs:    ss:    cs: e008
> >> (XEN) Xen stack trace from rsp=83040eb37200:
> >> (XEN)83040eb37278 83040eb37238 82d0801d09b6
>  0282
> >> (XEN)0008 830403791bf0 83041e687000
>  83040eb37268
> >> (XEN)82d0801cb23a 000c253f 8300d85fc000
>  0001
> >> (XEN)00c2 83040eb37298 82d0801cb410
>  000c253f
> >> (XEN) 00010001 0100
>  83040eb37328
> >> (XEN)82d0801c2403 83040eb37394 83040eb3
>  
> >> (XEN)83040eb37360 00c2 8304054cb000
>  053f
> >> (XEN)0002  83040eb373f4
>  00c2
> >> (XEN)83040eb373d8  
>  82d08028c620
> >> (XEN) 83040eb37338 82d0801c3e5d
>  83040eb37398
> >> (XEN)82d0801cb107 00010eb37394 830403791bf0
>  830403791bf0
> >> (XEN)83041e687000 83040eb37398 830403791bf0
>  0001
> >> (XEN)83040eb373d8 0001 000c253f
>  83040eb373c8
> >> (XEN)82d0801cb291 83040eb37b30 8300d85fc000
>  0001
> >> (XEN) 83040eb37428 82d0801bb440
>  000a0001
> >> (XEN)000c253f 00010001 0111
>  83040eb37478
> >> (XEN)0001  
>  0001
> >> (XEN)0001 83040eb374a8 82d0801bc0b9
>  0001
> >> (XEN)000c253f 8300d85fc000 000a0001
>  0100
> >> (XEN)83040eb37728 82e00819dc60 
>  83040eb374c8
> >> (XEN) Xen call trace:
> >> (XEN)[] _spin_lock+0x31/0x54
> >> (XEN)[] stdvga_mem_accept+0x3b/0x125
> >> (XEN)[] hvm_find_io_handler+0x68/0x8a
> >> (XEN)[] hvm_mmio_internal+0x37/0x67
> >> (XEN)[] __hvm_copy+0xe9/0x37d
> >> (XEN)[]
> hvm_copy_from_guest_phys+0x14/0x16
> >> (XEN)[]
> hvm_process_io_intercept+0x10b/0x1d6
> >> (XEN)[] hvm_io_intercept+0x35/0x5b
> >> (XEN)[] hvmemul_do_io+0x1ff/0x2c1
> >> (XEN)[] hvmemul_do_io_addr+0x117/0x163
> >> (XEN)[] hvmemul_do_mmio_addr+0x24/0x26
> >> (XEN)[] hvmemul_rep_movs+0x1ef/0x335
> >> (XEN)[] x86_emulate+0x56c9/0x13088
> >> (XEN)[] _hvm_emulate_one+0x186/0

Re: [Xen-devel] [PATCH 8/9] xl: fix main_cpupoolcreate

[Ian Campbell]

On Fri, 2015-07-10 at 19:00 +0100, Wei Liu wrote:
> Don't dereference extra_config if it's NULL. Don't leak extra_config in
> the end.

Subject should be more descriptive. "xl: correct handling of
extra_config in main_cpupoolcreate" perhaps? (It's a lot easier to write
non-vague messages for patches which only do one thing)

> Signed-off-by: Wei Liu 
> ---
>  tools/libxl/xl_cmdimpl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 971209c..d44eb4b 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -7228,7 +7228,7 @@ int main_cpupoolcreate(int argc, char **argv)
>  else
>  config_src="command line";
>  
> -if (strlen(extra_config)) {
> +if (extra_config && strlen(extra_config)) {
>  if (config_len > INT_MAX - (strlen(extra_config) + 2)) {
>  fprintf(stderr, "Failed to attach extra configration\n");

There's a typo in this line of context...

>  goto out;
> @@ -7365,6 +7365,7 @@ out_cfg:
>  out:
>  free(name);
>  free(config_data);
> +free(extra_config);
>  return rc;
>  }
>  



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 9/9] xl: fix main_config_update

[Ian Campbell]

On Fri, 2015-07-10 at 19:00 +0100, Wei Liu wrote:
> Don't dereference NULL.

Subject: xl: correctly handle null extra config in main_config_update

> 
> Signed-off-by: Wei Liu 
> ---
>  tools/libxl/xl_cmdimpl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index d44eb4b..631dbd1 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -5010,7 +5010,7 @@ int main_config_update(int argc, char **argv)
>  if (rc) { fprintf(stderr, "Failed to read config file: %s: %s\n",
> filename, strerror(errno));
>free(extra_config); return ERROR_FAIL; }
> -if (strlen(extra_config)) {
> +if (extra_config && strlen(extra_config)) {
>  if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) {
>  fprintf(stderr, "Failed to attach extra configration\n");
>  exit(1);



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [v7][PATCH 13/16] libxl: construct e820 map with RDM information for HVM guest

[Ian Campbell]

On Mon, 2015-07-13 at 17:47 +0800, Chen, Tiejun wrote:
> > This approach looks like it should work, and I think given the point in
> > the release it would be acceptable for 4.6.
> >
> > However long term I think it might make sense to try and reuse one of
> > the existing libxl__arch hooks, i.e.
> > libxl__arch_domain_init_hw_description or
> > libxl__arch_domain_finalise_hw_description. On ARM these are to do with
> > setting the Device Tree Blob, which included the memory map, so it is
> > somewhat morally equivalent to configuring the e820 on x86, I think.
> >
> > Those hooks are only called from libxl__build_pv today, but calling them
> > from libxl__build_hvm seems like it would be good too.
> 
> But seems this is raising some potential risks, isn't this? Although 
> libxl__arch_domain_init_hw_description() and 
> libxl__arch_domain_finalise_hw_description() are NOP to x86, they're 
> really working on ARM side. So if we call them inside 
> libxl__build_hvm(), any affects to ARM? I'm not very sure at this point 
> unless anyone can validate this change on ARM, or you really ensure my 
> concerns is unnecessary.

All ARM guests use the PV code path so there is no risk.

If there was some change to ARM to introduce an HVM style guest then it
would want those hooks called in this place too (and they would need
fixing as part of implementing such a thing).



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V2 1/1] libxl: set stub domain size based on VRAM size

[Ian Jackson]

Eric Shelton writes ("[PATCH V2 1/1] libxl: set stub domain size based on VRAM 
size"):
> Allocate additional memory to the stub domain for qemu-traditional if
> more than 4 MB is assigned to the video adapter to avoid out of memory
> condition for QEMU.

Acked-by: Ian Jackson 

This is IMO a bugfix so I am queueing it for 4.6.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [v7][PATCH 16/16] tools: parse to enable new rdm policy parameters

[Ian Campbell]

On Mon, 2015-07-13 at 17:55 +0800, Chen, Tiejun wrote:
> On 2015/7/13 17:40, Ian Campbell wrote:
> > On Mon, 2015-07-13 at 17:31 +0800, Chen, Tiejun wrote:
> >> I still can't understand what I'm missing here after compared to other
> >> contexts inside xlu_pci_parse_bdf().
> >
> > Perhaps comparing to the CODING_STYLE document would help?
> 
> Looks the whole xlu_pci_parse_bdf() doesn't follow that,
> 
>  if ( !strcmp(optkey, "msitranslate") ) {
>  pcidev->msitranslate = atoi(tok);
>  }else if ( !strcmp(optkey, "power_mgmt") ) {
>  pcidev->power_mgmt = atoi(tok);
>  }else if ( !strcmp(optkey, "permissive") ) {
>  pcidev->permissive = atoi(tok);
>  }else if ( !strcmp(optkey, "seize") ) {
>  pcidev->seize = atoi(tok);
>  }else if ( !strcmp(optkey, "rdm_policy") ) {
> 
> So I can do this as you're expecting now, but seems our change would 
> make the code style very inconsistent inside this function.

I think one could make an argument that the exception described in the
first section of tools/libxl/CODING_STYLE applies here for the
whitespace issues, but not for the long lines I think.

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [xen-unstable test] 59472: regressions - FAIL

[Jan Beulich]

>>> On 13.07.15 at 03:43,  wrote:
> flight 59472 xen-unstable real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/59472/ 
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install 
> fail REGR. vs. 58965

Considering (on italia1)

Jul 12 21:27:20.505103 (d1) No bootable device.
Jul 12 21:27:20.505128 (d1) Powering off in 30 seconds.

is this really a regression (rather than something that never fully
worked)? With flight 58917 also having (on chardonnay0)

Jun 27 11:08:08.025161 (d1) No bootable device.
Jun 27 11:08:08.033014 (d1) Powering off in 30 seconds.

is it possible that 58965's success (on elbling1) was because this only
works on a very limited subset of hosts? I didn't spot anything in the
logs of the failure cases that would help me understand the reason
for the failures...

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V2 1/1] libxl: set stub domain size based on VRAM size

[Ian Jackson]

Ian Jackson writes ("Re: [PATCH V2 1/1] libxl: set stub domain size based on 
VRAM size"):
> Eric Shelton writes ("[PATCH V2 1/1] libxl: set stub domain size based on 
> VRAM size"):
> > Allocate additional memory to the stub domain for qemu-traditional if
> > more than 4 MB is assigned to the video adapter to avoid out of memory
> > condition for QEMU.
> 
> Acked-by: Ian Jackson 
> 
> This is IMO a bugfix so I am queueing it for 4.6.

My build test failed.  It turns out that max() is no good because the
types of `4096' and `guest_config->b_info.video_memkb' are not the
same.

In a moment I am going to send a v3 which uses max_t and uint64_t
(which is the type of the memkb fields and also obviously correct).

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH for-4.6 v3] libxl: set stub domain size based on VRAM size

[Ian Jackson]

From: Eric Shelton 

Allocate additional memory to the stub domain for qemu-traditional if
more than 4 MB is assigned to the video adapter to avoid out of memory
condition for QEMU.

Signed-off-by: Eric Shelton 
Signed-off-by: Ian Jackson 
---
v3: Use max_t() instead
v2: Use max()
---
 tools/libxl/libxl_dm.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index ad434f0..f700f9a 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1095,7 +1095,8 @@ void libxl__spawn_stub_dm(libxl__egc *egc, 
libxl__stub_dm_spawn_state *sdss)
 libxl_domain_build_info_init_type(&dm_config->b_info, 
LIBXL_DOMAIN_TYPE_PV);
 
 dm_config->b_info.max_vcpus = 1;
-dm_config->b_info.max_memkb = 32 * 1024;
+dm_config->b_info.max_memkb = 28 * 1024 +
+max_t(uint64_t, guest_config->b_info.video_memkb, 4096);
 dm_config->b_info.target_memkb = dm_config->b_info.max_memkb;
 
 dm_config->b_info.u.pv.features = "";
-- 
1.7.10.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 for Xen 4.6 0/4] Enable per-VCPU parameter settings for RTDS scheduler

[Dario Faggioli]

On Sat, 2015-07-11 at 15:33 +0100, Wei Liu wrote:
> Hi Chong
> 
> This series is marked as "for 4.6", but we just hit feature freeze
> yesterday.
> 
Yeah, I wanted to reply myself about this, but Wei beat me... Good job
as release manager, I would say. :-)

> Given the status of this series (missing many acks), I am sorry to say
> this series will have to wait until next release.
> 
Indeed. The series is starting to look good, and, Chong, you're doing a
great work, especially by replying promptly to reviews, and reposting
new versions very quickly.

However, this series arrived a bit late in the dev cycle, and suffered
from some delay in reviewing (from me as well, sorry for that), but
(both) this things happen in (Open Source) software development, and we
can't do much about it.

Also, the original goal was to pull RTDS out of experimental, but, even
with this series in, we wouldn't get to there as:
 - not enough testing: it entered OSSTest not so long ago, which, e.g.,
   showed up it's failing on ARM!
 - not enough benchmarks/performance figures: I'd like to have the
   latency numbers, e.g., from cyclictest, we've spoke many times with
   Meng, give our official blessing at using it
 - the work Dagaen's doing is a rather fundamental restructuring, and it
   makes sense to do all the above (testing and performance evaluation)
   on top of the result of that for a bit, before declaring things
   stable and supported (or we risk disrupting that because of it, and
   since it's already ongoing, I'll really let him finish)

So, for the following reasons (coming from the above reasoning):
 - the series is good, but certainly still not ready;
 - having the series in, would not change much wrt RTDS in 4.6

I, as the maintainer of this feature, agree with Wei that we should work
toward merging this series really soon... at the beginning of 4.7
development cycle! :-D

> We will review this series in timely manner provided there are no other
> urgent matters for the release. Please keep up with your good work.
> 
Indeed. Thanks a log again to you, Meng, Dagaen, and everyone.

I'll review the series ASAP.

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V2 1/1] libxl: set stub domain size based on VRAM size

[Ian Campbell]

On Mon, 2015-07-13 at 11:22 +0100, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH V2 1/1] libxl: set stub domain size based on 
> VRAM size"):
> > Eric Shelton writes ("[PATCH V2 1/1] libxl: set stub domain size based on 
> > VRAM size"):
> > > Allocate additional memory to the stub domain for qemu-traditional if
> > > more than 4 MB is assigned to the video adapter to avoid out of memory
> > > condition for QEMU.
> > 
> > Acked-by: Ian Jackson 
> > 
> > This is IMO a bugfix so I am queueing it for 4.6.
> 
> My build test failed.  It turns out that max() is no good because the
> types of `4096' and `guest_config->b_info.video_memkb' are not the
> same.
> 
> In a moment I am going to send a v3 which uses max_t and uint64_t
> (which is the type of the memkb fields and also obviously correct).

Eric already sent a v3 in
<1436650242-1067-2-git-send-email-eshel...@pobox.com> which avoids the
use of max in a different way.

I think his approach looked fine.

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2] x86: avoid invalid phys_proc_id reference

[Chao Peng]

phys_proc_id is invalidated in remove_siblinginfo() which gets called
before cpu_smpboot_free(). This means calling cpu_to_socket(cpu) in
cpu_smpboot_free() is not possible to be correct.

This patch moves the invalidating of phys_proc_id from
remove_siblinginfo() to cpu_smpboot_free() so that cpu_to_socket(cpu)
can be used in cpu_smpboot_free().

The same is done for cpu_core_id/compute_unit_id and due to that
cpu_sibling_setup_map is private to the file so it's moved as well.

Reported-by: Dario Faggioli 
Suggested-by: Jan Beulich 
Signed-off-by: Chao Peng 
---
v2: use less intrusive solution.
---
 xen/arch/x86/smpboot.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 0f03364..8b292c0 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -666,6 +666,7 @@ void cpu_exit_clear(unsigned int cpu)
 static void cpu_smpboot_free(unsigned int cpu)
 {
 unsigned int order, socket = cpu_to_socket(cpu);
+struct cpuinfo_x86 *c = cpu_data;
 
 if ( cpumask_empty(socket_cpumask[socket]) )
 {
@@ -673,6 +674,11 @@ static void cpu_smpboot_free(unsigned int cpu)
 socket_cpumask[socket] = NULL;
 }
 
+c[cpu].phys_proc_id = XEN_INVALID_SOCKET_ID;
+c[cpu].cpu_core_id = XEN_INVALID_CORE_ID;
+c[cpu].compute_unit_id = INVALID_CUID;
+cpumask_clear_cpu(cpu, &cpu_sibling_setup_map);
+
 free_cpumask_var(per_cpu(cpu_sibling_mask, cpu));
 free_cpumask_var(per_cpu(cpu_core_mask, cpu));
 
@@ -882,7 +888,6 @@ static void
 remove_siblinginfo(int cpu)
 {
 int sibling;
-struct cpuinfo_x86 *c = cpu_data;
 
 cpumask_clear_cpu(cpu, socket_cpumask[cpu_to_socket(cpu)]);
 
@@ -891,17 +896,13 @@ remove_siblinginfo(int cpu)
 cpumask_clear_cpu(cpu, per_cpu(cpu_core_mask, sibling));
 /* Last thread sibling in this cpu core going down. */
 if ( cpumask_weight(per_cpu(cpu_sibling_mask, cpu)) == 1 )
-c[sibling].booted_cores--;
+cpu_data[sibling].booted_cores--;
 }

 for_each_cpu(sibling, per_cpu(cpu_sibling_mask, cpu))
 cpumask_clear_cpu(cpu, per_cpu(cpu_sibling_mask, sibling));
 cpumask_clear(per_cpu(cpu_sibling_mask, cpu));
 cpumask_clear(per_cpu(cpu_core_mask, cpu));
-c[cpu].phys_proc_id = XEN_INVALID_SOCKET_ID;
-c[cpu].cpu_core_id = XEN_INVALID_CORE_ID;
-c[cpu].compute_unit_id = INVALID_CUID;
-cpumask_clear_cpu(cpu, &cpu_sibling_setup_map);
 }
 
 void __cpu_disable(void)
-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Requesting for freeze exception for VT-d posted-interrupts

[Wei Liu]

On Mon, Jul 13, 2015 at 06:55:30AM +, Wu, Feng wrote:
> Hi maintainers,
> 
> We would like to request an extension for freeze exception for VT-d 
> posted-interrupts patch-set.
> 
> 1. clarify the state of patch series / feature.
> [v3 01/15] Vt-d Posted-interrupt (PI) design
> Reviewed-by: Kevin Tian 
> 
> [v3 02/15] Add helper macro for X86_FEATURE_CX16 feature detection
> Reviewed-by: Kevin Tian 
> Reviewed-by: Andrew Cooper 
> 
> [v3 04/15] iommu: Add iommu_intpost to control VT-d Posted-Interrupts feature
> Reviewed-by: Kevin Tian 
> 
> [v3 06/15] vmx: Extend struct pi_desc to support VT-d Posted-Interrupts
> Reviewed-by: Andrew Cooper 
> Acked-by: Kevin Tian 
> 
> [v3 07/15] vmx: Initialize VT-d Posted-Interrupts Descriptor
> Acked-by: Kevin Tian 
> 
> [v3 09/15] vt-d: Extend struct iremap_entry to support VT-d Posted-Interrupts
> Acked-by: Kevin Tian 
> 
> [v3 10/15] vt-d: Add API to update IRTE when VT-d PI is used
> Acked-by: Kevin Tian 
> 
> [v3 13/15] vmx: Properly handle notification event when vCPU is running
> Acked-by: Kevin Tian 
> 
> [v3 14/15] Update Posted-Interrupts Descriptor during vCPU scheduling
> Acked-by: Kevin Tian 
> 
> [v3 15/15] Add a command line parameter for VT-d posted-interrupts
> Reviewed-by: Kevin Tian 
> 
> 2. explain why it needs to be in this release (benefits).
> VT-d posted-interrupts is an important interrupt virtualization feature for
> device pass-through, the running guest can handle external interrupts
> in non-root mode, hence it can eliminate the VM-Exits caused by external
> interrupts. Please refer to the design doc:
> http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg03691.html
> 
> >From our experimental environment, after using VT-d posted-interrupts, we
> measured 25% improvement in transaction rate netperf TCP_RR benchmark
> and 28% reduction in host CPU utilization when using assigned devices.
> (10G NIC in my test).
> 
> 3. explain why it doesn't break things (risks).
> This feature only exists in Broadwell Server platform, it has no effect on the
> current hardware.
> 

You miss the part that how much common code it touches. There is still
risk of breaking VMX and VT-D even if PI is disabled.

> 4. CC relevant maintainers and release manager.
> Done
> 
> There are two main outstanding issues so far:
> 1. Jan's security concern. I have proposed some solutions but Jan still has
> some problems with my proposals. It would be great if Jan can give a clear
> proposal so that we can discuss and keep making progress.
> 2. Scheduler issue: there are conflicts among maintainers Jan/George/Dario.
> I would agree with Jan's suggestion below:
> 
> " Doing this in a central place is certainly the right approach, but
> adding an arch hook that needs to be called everywhere
> vcpu_runstate_change() wouldn't serve that purpose. Instead
> we'd need to replace all current vcpu_runstate_change() calls
> with calls to a new function calling both this and the to be added
> arch hook."
> 

Given the current time scale now, I think it would be very hard to get
these two concerns addressed within a week. Xen has always taken
security serious, I don't want to rush in a feature with possible flawed
design.

My answer to this request is no until these concerns are addressed.

> However, if different maintainers still hold different opinions, I would 
> appreciate
> it if maintainers can reach consensus among themselves so that we can keep
> making progress
> 

Yes, this is fore sure. This is what we need to do to work as a
community whether this feature is aimed for 4.6 or not.

Wei.

> Thanks,
> Feng

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] MAINTAINERS: support for xen-access and email change

[Ian Campbell]

On Fri, 2015-07-10 at 16:39 +0100, Jan Beulich wrote:
> >>> On 10.07.15 at 17:29,  wrote:
> > Add tools/tests/xen-acess to the supported list under VM EVENT/MEM ACCESS.
> > Also, changing my e-mail to the preferred one, as it is in many of the 
> > headers
> > already.
> > 
> > Signed-off-by: Tamas K Lengyel 
> 
> It looks slightly odd to me that you sign off with you "non-preferred"
> mail address then...

I don't think it is especially odd to want to receive patches via some
path which offers proper filtering etc, but to still be obliged to S-o-b
with the email associated with the entity which owns the rights to the
contribution (but which uses an inflexible mail arrangement of some
sort).

Acked-by: Ian Campbell 

Ian.



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH OSSTEST v2 01/13] toolstack: save / restore check

[Ian Campbell]

On Sun, 2015-07-12 at 17:20 +0100, Wei Liu wrote:
> Introduce _$TOOLSTACK_check_for_command function and use it to check
> save / restore functionality.

The _$TOOLSTACK_ prefix is not necessary IMHO, this is already within a
Perl module named $TOOLSTACK.pm (which you can think of as a class in
the OOP sense). (It seems like a Python-ism to me).

IOW I think just check_for_command would be fine. I don't think it needs
to be made private either, which would involve a bit more infrastructure
than is worthwhile IMHO.


> 
> Signed-off-by: Wei Liu 
> Cc: Ian Campbell 
> Cc: Ian Jackson 
> ---
> v2: introduce $TOOLSTACK_check_for_command function.
> ---
>  Osstest/Toolstack/libvirt.pm | 14 ++
>  Osstest/Toolstack/xend.pm|  3 +++
>  Osstest/Toolstack/xl.pm  | 16 +---
>  3 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
> index 51a10de..592cfa2 100644
> --- a/Osstest/Toolstack/libvirt.pm
> +++ b/Osstest/Toolstack/libvirt.pm
> @@ -77,6 +77,20 @@ sub migrate_check ($) {
>  die "Migration check is not yet supported on libvirt.";
>  }
>  
> +sub _libvirt_check_for_command($$) {
> +my ($self,$cmd) = @_;
> +my $ho = $self->{Host};
> +my $help = target_cmd_output_root($ho, "virsh help");
> +my $rc = ($help =~ m/^\s*$cmd/m) ? 0 : 1;
> +logm("rc=$rc");
> +return $rc;
> +}
> +
> +sub saverestore_check ($) {
> +my ($self) = @_;
> +return _libvirt_check_for_command($self, "save");
> +}
> +
>  sub migrate ($) {
>  my ($self,$gho,$dst,$timeout) = @_;
>  die "Migration is not yet supported on libvirt.";
> diff --git a/Osstest/Toolstack/xend.pm b/Osstest/Toolstack/xend.pm
> index 972b3b1..fd54ae1 100644
> --- a/Osstest/Toolstack/xend.pm
> +++ b/Osstest/Toolstack/xend.pm
> @@ -38,4 +38,7 @@ sub new {
>  # xend always supported migration
>  sub migrate_check ($) { return 0; }
>  
> +# xend always supported save / restore
> +sub saverestore_check ($) { return 0; }
> +
>  1;
> diff --git a/Osstest/Toolstack/xl.pm b/Osstest/Toolstack/xl.pm
> index 3c3d348..440d9d0 100644
> --- a/Osstest/Toolstack/xl.pm
> +++ b/Osstest/Toolstack/xl.pm
> @@ -61,15 +61,25 @@ sub shutdown_wait ($$$) {
>  target_cmd_root($ho,"$self->{_Command} shutdown -w${acpi_fallback} $gn", 
> $timeout);
>  }
>  
> -sub migrate_check ($) {
> -my ($self) = @_;
> +sub _xl_check_for_command($$) {
> +my ($self,$cmd) = @_;
>  my $ho = $self->{Host};
>  my $help = target_cmd_output_root($ho, $self->{_Command}." help");
> -my $rc = ($help =~ m/^\s*migrate/m) ? 0 : 1;
> +my $rc = ($help =~ m/^\s*$cmd/m) ? 0 : 1;
>  logm("rc=$rc");
>  return $rc;
>  }
>  
> +sub migrate_check ($) {
> +my ($self) = @_;
> +return _xl_check_for_command($self, "migrate");
> +}
> +
> +sub saverestore_check ($) {
> +my ($self) = @_;
> +return _xl_check_for_command($self, "save");
> +}
> +
>  sub migrate () {
>  my ($self,$gho,$dho,$timeout) = @_;
>  my $sho = $self->{Host};



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [qemu-mainline test] 59483: regressions - FAIL

[osstest service owner]

flight 59483 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/59483/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 11 guest-saverestore fail REGR. 
vs. 59059
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 11 guest-saverestore fail REGR. vs. 
59059
 test-amd64-i386-freebsd10-amd64 12 guest-saverestore  fail REGR. vs. 59059
 test-amd64-i386-xl-qemuu-ovmf-amd64 11 guest-saverestore  fail REGR. vs. 59059
 test-amd64-i386-freebsd10-i386 12 guest-saverestore   fail REGR. vs. 59059
 test-amd64-i386-xl-qemuu-debianhvm-amd64 11 guest-saverestore fail REGR. vs. 
59059
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 11 guest-saverestore fail REGR. 
vs. 59059
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 11 guest-saverestore fail REGR. vs. 
59059
 test-amd64-amd64-xl-qemuu-winxpsp3 11 guest-saverestore   fail REGR. vs. 59059
 test-amd64-amd64-xl-qemuu-win7-amd64 11 guest-saverestore fail REGR. vs. 59059
 test-amd64-i386-xl-qemuu-winxpsp3 11 guest-saverestorefail REGR. vs. 59059
 test-amd64-i386-xl-qemuu-win7-amd64 11 guest-saverestore fail in 59435 REGR. 
vs. 59059
 test-amd64-amd64-xl-qemuu-ovmf-amd64 11 guest-saverestore fail in 59465 REGR. 
vs. 59059

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt-xsm 7 host-ping-check-xen fail in 59465 pass in 59483
 test-amd64-i386-xl-qemuu-win7-amd64  9 windows-install  fail pass in 59435
 test-armhf-armhf-xl-rtds 11 guest-start fail pass in 59465
 test-amd64-amd64-xl-qemuu-ovmf-amd64  9 debian-hvm-install  fail pass in 59465

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 14 guest-start.2fail in 59465 REGR. vs. 59059
 test-amd64-amd64-libvirt 11 guest-start  fail   like 59059
 test-amd64-i386-libvirt  11 guest-start  fail   like 59059
 test-amd64-i386-libvirt-xsm  11 guest-start  fail   like 59059

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-rtds 12 migrate-support-check fail in 59465 never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 11 guest-start  fail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass

version targeted for testing:
 qemuu6169b60285fe1ff730d840a49527e721bfb30899
baseline version:
 qemuu35360642d043c2a5366e8a04a10e5545e7353bd5

Last test of basis59059  2015-07-05 10:39:20 Z8 days
Failing since 59109  2015-07-06 14:58:21 Z6 days8 attempts
Testing same since59387  2015-07-10 15:45:24 Z2 days4 attempts


People who touched revisions under test:
  Alberto Garcia 
  Alex Williamson 
  Alexander Graf 
  Alexey Kardashevskiy 
  Alvise Rigo 
  Andreas Färber 
  Andrew Jones 
  Artyom Tarasenko 
  Aurelien Jarno 
  Benjamin Herrenschmidt 
  Bharata B Rao 
  Brian Kress 
  Claudio Fontana 
  Cormac O'Brien 
  Cornelia Huck 
  Daniel P. Berrange 
  David Gibson 
  Denis V. Lunev 
  Dmitry Osipenko 
  Dr. David Alan Gilbert 
  Eduardo Habkost 
  Eric Auger 
  Fam Zheng 
  Gabriel Laupre 
  Gavin Shan 
  Gerd Hoffmann 
  Gonglei 
  Greg Kurz 
  Hannes Reinecke 
  Igor Mammedov 
  James Hogan 
  Jan Kiszka 
  Johannes Schlatow 
  John Snow 
  Juan Quintela 
  Justin Ossevoort 
  Kirk Allan 
  Laszlo Ersek 
  Laurent Vivier 
  Laurent Vivier 
  Li Zhijian 
  Marc-André Lureau 
  Markus Armbruster 
  Max Filippov 
  Michael Roth 
  Michael S. Tsirkin 
  Nikunj A Dadhania 
  Olga Krishtal 
  Paolo Bonzini 
  Paul Durrant 
  Paulo Alcantara 
  Paulo Alcantara 
  Peter Crosthwaite 
  Peter Crosthwaite 
  Peter Maydell 
  Richard W.M. Jones 
  Scott Feldman 
  Sergey Fedorov 
  Stefan Hajnoczi 
  Ting Wang 
  Vikram Sethi 
  Wen Congyang 
  Wenshuang Ma 
  马文霜 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  

Re: [Xen-devel] [PATCH V2 1/1] libxl: set stub domain size based on VRAM size

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH V2 1/1] libxl: set stub domain size based on 
VRAM size"):
> On Mon, 2015-07-13 at 11:22 +0100, Ian Jackson wrote:
> > In a moment I am going to send a v3 which uses max_t and uint64_t
> > (which is the type of the memkb fields and also obviously correct).
> 
> Eric already sent a v3 in
> <1436650242-1067-2-git-send-email-eshel...@pobox.com> which avoids the
> use of max in a different way.
> 
> I think his approach looked fine.

Well, it does also reduce the stubdom memory when the video memory is
<4Mby.  That sounds plausible to me but at the very least the commit
message needs changing, and maybe we should wait a bit to see if one
of the qemu experts objects.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH OSSTEST v2 03/13] osstest migrate support check catch -> variables

[Ian Campbell]

On Sun, 2015-07-12 at 17:20 +0100, Wei Liu wrote:
> @@ -300,7 +300,9 @@ proc run-job/test-pair {} {
>  }
>  
>  proc test-guest-migr {g} {
> -if {[catch { run-ts . = ts-migrate-support-check + host $g }]} return
> +set to_reap [spawn-ts . = ts-migrate-support-check + host $g]

Most other uses of spawn-ts use [eval spawn-ts ]. I think those
are just trying to expand a $args into multiple arguments to spawn-ts,
and hence that isn't needed here (because $g is a singleton argument
already). But TBH I don't know...



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 07/17] xen/arm: ITS: Add virtual ITS commands support

[Vijay Kilari]

On Mon, Jul 13, 2015 at 2:52 PM, Ian Campbell  wrote:
> On Sat, 2015-07-11 at 20:19 +0530, Vijay Kilari wrote:
>> >> +int vgic_its_process_cmd(struct vcpu *v, struct vgic_its *vits)
>> >> +{
>> >> +its_cmd_block virt_cmd;
>> >> +
>> >> +ASSERT(spin_is_locked(&vits->lock));
>> >> +
>> >> +do {
>> >> +if ( vgic_its_read_virt_cmd(v, vits, &virt_cmd) )
>> >> +goto err;
>> >> +if ( vgic_its_parse_its_command(v, vits, &virt_cmd) )
>> >> +goto err;
>> >> +vgic_its_update_read_ptr(v, vits);
>> >> +} while ( vits->cmd_write != vits->cmd_write_save );
>> >
>> > I can't find anywhere other than here where vits->cmd_write is touched.
>> > What am I missing?
>>
>>It is written by guest by GITS_CWRITER emulation in patch #9
>
> Ah, then please reverse the order so that the variable comes first and
> the target comes second.
>
> Also I think you need to find a better name that "cmd_write_save".
> Something which indicates the progress made perhaps? But why isn't this
> just cmd_read? Why the separate progress pointer?

I will check If I can use cmd_read.

BTW, I want to know if atomic_t supports 64-bit access?.
I have not made cmd_read as atomic_t.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH OSSTEST v2 04/13] toolstack: distinguish local and remote migration support

[Ian Campbell]

On Sun, 2015-07-12 at 17:20 +0100, Wei Liu wrote:
> @@ -23,5 +23,9 @@ use Osstest::TestSupport;
>  tsreadconfig();
>  
>  our $ho = selecthost($ARGV[0]);
> +# $ARGV[1] is guest name, $ARG[2] indicates whether it is checking for
> +# local migration or remote migration
> +# Mode should be either 1 ("local") or 0 ("remote")
> +our $mode = $ARGV[2];

I think:

our ($whhost, $gn, $mode) = @ARGV;
our $ho = selecthost($whhost);

would be preferable to the first two lines of the comment.


> -exit(toolstack($ho)->migrate_check());
> +exit(toolstack($ho)->migrate_check($mode));



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Xen 4.6 development window closes today and other information

[Wei Liu]

One thing that was missed in previous is how much time we have to apply
patch series that has been granted a freeze exception.

Given the fact that freeze exception would only be granted to patch
series that is very close to completion, I expect any patch series with
that status be applied within one week after it has been granted that
status.

That means the final cut off date for applying patch series with freeze
exception is July 24 (because we only grant freeze exception within this
week).

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH OSSTEST v2 05/13] sg-run-job: remove save/restore dependency on local migration support

[Ian Campbell]

On Sun, 2015-07-12 at 17:20 +0100, Wei Liu wrote:
> Since we've introduced different checks for save / restore and local
> migration, it's possible to run save / restore tests without running
> local migration tests.
> 
> Signed-off-by: Wei Liu 
> Cc: Ian Campbell 
> Cc: Ian Jackson 
> ---
>  sg-run-job | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/sg-run-job b/sg-run-job
> index 16fcfc1..172214e 100755
> --- a/sg-run-job
> +++ b/sg-run-job
> @@ -302,13 +302,20 @@ proc run-job/test-pair {} {
>  proc test-guest-migr {g} {
>  set to_reap [spawn-ts . = ts-migrate-support-check + host $g 1]
>  set can_migrate [reap-ts $to_reap]
> -if {!$can_migrate} return
> +set to_reap [spawn-ts . = ts-saverestore-support-check + host]

ts-migrate-support-check takes the guest name, even though it doesn't
actually need it today. I think this is justifiable because the decision
as to whether migration or s/r could be achieve _might_ be dependent on
the guest cfg.

Thus I think ts-saverrestore-support-check ought to take $g too for
consistency with that and with ts-guest-migrate too.

Or $g it could be removed from the migrate check, but that's not my
preference.

> +set can_saverestore [reap-ts $to_reap]
>  
>  foreach iteration {{} .2} {
> -run-ts . =$iteration ts-guest-saverestore + host $g
> -run-ts . =$iteration ts-guest-localmigrate + host $g
> +if {$can_saverestore} {
> +run-ts . =$iteration ts-guest-saverestore + host $g
> +}
> +if {$can_migrate} {
> +run-ts . =$iteration ts-guest-localmigrate + host $g
> +}
> +}
> +if {$can_migrate} {
> +run-ts . = ts-guest-localmigrate x10 + host $g
>  }
> -run-ts . = ts-guest-localmigrate x10 + host $g
>  }
>  
>  proc test-guest {g} {



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH OSSTEST v2 06/13] toolstack/libvirt: guest migrate, save and restore support

[Ian Campbell]

On Sun, 2015-07-12 at 17:20 +0100, Wei Liu wrote:

Perhaps the libvirt part of the check_for_command stuff ought to be
moved here? Otherwise we are claiming support before the code is
actually willing to try to do so.

> Signed-off-by: Wei Liu 
> Cc: Ian Campbell 
> Cc: Ian Jackson 
> Acked-by: Ian Campbell 
> ---
>  Osstest/Toolstack/libvirt.pm | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
> index ddf84df..3dc1856 100644
> --- a/Osstest/Toolstack/libvirt.pm
> +++ b/Osstest/Toolstack/libvirt.pm
> @@ -105,17 +105,22 @@ sub saverestore_check ($) {
>  
>  sub migrate ($) {
>  my ($self,$gho,$dst,$timeout) = @_;
> -die "Migration is not yet supported on libvirt.";
> +my $ho = $self->{Host};
> +my $gn = $gho->{Name};
> +target_cmd_root($ho, "virsh migrate $gn $dst", $timeout);
>  }
>  
>  sub save () {
>  my ($self,$gho,$f,$timeout) = @_;
> -die "Save is not yet supported on libvirt.";
> +my $ho = $self->{Host};
> +my $gn = $gho->{Name};
> +target_cmd_root($ho, "virsh save $gn $f", $timeout);
>  }
>  
>  sub restore () {
>  my ($self,$gho,$f,$timeout) = @_;
> -die "Restore is not yet supported on libvirt.";
> +my $ho = $self->{Host};
> +target_cmd_root($ho, "virsh restore $f", $timeout);
>  }
>  
>  1;



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel