[Xen-devel] [PATCH v2 for Xen 4.6 2/6] tools/libxl: fix socket display error for CMT

[Chao Peng]

When displaying the CMT information for all the sockets, we assume socket
number is continuous. This is not true in the hotplug case. For instance,
when the 3rd socket is plugged out on a 4-socket system, the available
sockets numbers are 1,2,4 but current we will display the CMT
information for socket 1,2,3.

The fix is getting the socket bitmap for all the sockets on the system
first and then displaying CMT information for_each_set_bit in that bitmap.

Signed-off-by: Chao Peng 
Acked-by: Wei Liu 
---
v2:
* add libxl_bitmap_init().
---
 tools/libxl/xl_cmdimpl.c | 43 +++
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2706759..f01d245 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8192,7 +8192,7 @@ static int psr_cmt_get_mem_bandwidth(uint32_t domid,
 
 static void psr_cmt_print_domain_info(libxl_dominfo *dominfo,
   libxl_psr_cmt_type type,
-  uint32_t nr_sockets)
+  libxl_bitmap *socketmap)
 {
 char *domain_name;
 uint32_t socketid;
@@ -8205,7 +8205,7 @@ static void psr_cmt_print_domain_info(libxl_dominfo 
*dominfo,
 printf("%-40s %5d", domain_name, dominfo->domid);
 free(domain_name);
 
-for (socketid = 0; socketid < nr_sockets; socketid++) {
+libxl_for_each_set_bit(socketid, *socketmap) {
 switch (type) {
 case LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY:
 if (!libxl_psr_cmt_get_sample(ctx, dominfo->domid, type, socketid,
@@ -8228,9 +8228,9 @@ static void psr_cmt_print_domain_info(libxl_dominfo 
*dominfo,
 
 static int psr_cmt_show(libxl_psr_cmt_type type, uint32_t domid)
 {
-uint32_t i, socketid, nr_sockets, total_rmid;
+uint32_t i, socketid, total_rmid;
 uint32_t l3_cache_size;
-libxl_physinfo info;
+libxl_bitmap socketmap;
 int rc, nr_domains;
 
 if (!libxl_psr_cmt_enabled(ctx)) {
@@ -8244,41 +8244,39 @@ static int psr_cmt_show(libxl_psr_cmt_type type, 
uint32_t domid)
 return -1;
 }
 
-libxl_physinfo_init();
-rc = libxl_get_physinfo(ctx, );
+libxl_bitmap_init();
+libxl_socket_bitmap_alloc(ctx, , 0);
+rc = libxl_get_online_socketmap(ctx, );
 if (rc < 0) {
-fprintf(stderr, "Failed getting physinfo, rc: %d\n", rc);
-libxl_physinfo_dispose();
-return -1;
+fprintf(stderr, "Failed getting available sockets, rc: %d\n", rc);
+goto out;
 }
-nr_sockets = info.nr_cpus / info.threads_per_core / info.cores_per_socket;
-libxl_physinfo_dispose();
 
 rc = libxl_psr_cmt_get_total_rmid(ctx, _rmid);
 if (rc < 0) {
 fprintf(stderr, "Failed to get max RMID value\n");
-return -1;
+goto out;
 }
 
 printf("Total RMID: %d\n", total_rmid);
 
 /* Header */
 printf("%-40s %5s", "Name", "ID");
-for (socketid = 0; socketid < nr_sockets; socketid++)
+libxl_for_each_set_bit(socketid, socketmap)
 printf("%14s %d", "Socket", socketid);
 printf("\n");
 
 if (type == LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY) {
 /* Total L3 cache size */
 printf("%-46s", "Total L3 Cache Size");
-for (socketid = 0; socketid < nr_sockets; socketid++) {
+libxl_for_each_set_bit(socketid, socketmap) {
 rc = libxl_psr_cmt_get_l3_cache_size(ctx, socketid,
  _cache_size);
 if (rc < 0) {
 fprintf(stderr,
 "Failed to get system l3 cache size for 
socket:%d\n",
 socketid);
-return -1;
+goto out;
 }
 printf("%13u KB", l3_cache_size);
 }
@@ -8292,9 +8290,10 @@ static int psr_cmt_show(libxl_psr_cmt_type type, 
uint32_t domid)
 libxl_dominfo_init();
 if (libxl_domain_info(ctx, , domid)) {
 fprintf(stderr, "Failed to get domain info for %d\n", domid);
-return -1;
+rc = -1;
+goto out;
 }
-psr_cmt_print_domain_info(, type, nr_sockets);
+psr_cmt_print_domain_info(, type, );
 libxl_dominfo_dispose();
 }
 else
@@ -8302,13 +8301,17 @@ static int psr_cmt_show(libxl_psr_cmt_type type, 
uint32_t domid)
 libxl_dominfo *list;
 if (!(list = libxl_list_domain(ctx, _domains))) {
 fprintf(stderr, "Failed to get domain info for domain list.\n");
-return -1;
+rc = -1;
+goto out;
 }
 for (i = 0; i < nr_domains; i++)
-psr_cmt_print_domain_info(list + i, type, nr_sockets);
+psr_cmt_print_domain_info(list + i, type, );
 libxl_dominfo_list_free(list, nr_domains);
 }
-

Re: [Xen-devel] [PATCH 1/3] x86/p2m: tighten conditions of IOMMU mapping updates

[Jan Beulich]

>>> On 28.09.15 at 18:32,  wrote:
> On 21/09/15 15:02, Jan Beulich wrote:
>> In the EPT case permission changes should also result in updates or
>> TLB flushes.
>> 
>> In the NPT case the old MFN does not depend on the new entry being
>> valid (but solely on the old one), and the need to update or TLB-flush
>> again also depends on permission changes.
>> 
>> In the shadow mode case, iommu_hap_pt_share should be ignored.
>> 
>> Furthermore in the NPT/shadow case old intermediate page tables must
>> be freed only after IOMMU side updates/flushes have got carried out.
>> 
>> Signed-off-by: Jan Beulich 
> 
> So first of all, having all these changes in the same patch almost
> certainly slowed down the review process a bit.

Okay, I'll try to split things up ...

>> --- a/xen/arch/x86/mm/p2m-ept.c
>> +++ b/xen/arch/x86/mm/p2m-ept.c
>> @@ -668,6 +668,7 @@ ept_set_entry(struct p2m_domain *p2m, un
>>  uint8_t ipat = 0;
>>  int need_modify_vtd_table = 1;
>>  int vtd_pte_present = 0;
>> +unsigned int iommu_flags = p2m_get_iommu_flags(p2mt);
>>  enum { sync_off, sync_on, sync_check } needs_sync = sync_check;
>>  ept_entry_t old_entry = { .epte = 0 };
>>  ept_entry_t new_entry = { .epte = 0 };
>> @@ -798,8 +799,9 @@ ept_set_entry(struct p2m_domain *p2m, un
>>  new_entry.mfn = mfn_x(mfn);
>>  
>>  /* Safe to read-then-write because we hold the p2m lock */
>> -if ( ept_entry->mfn == new_entry.mfn )
>> - need_modify_vtd_table = 0;
>> +if ( ept_entry->mfn == new_entry.mfn &&
>> + p2m_get_iommu_flags(ept_entry->sa_p2mt) == iommu_flags )
>> +need_modify_vtd_table = 0;
>>  
>>  ept_p2m_type_to_flags(p2m, _entry, p2mt, p2ma);
>>  }
>> @@ -830,11 +832,9 @@ out:
>>  iommu_pte_flush(d, gfn, _entry->epte, order, 
>> vtd_pte_present);
>>  else
>>  {
>> -unsigned int flags = p2m_get_iommu_flags(p2mt);
>> -
>> -if ( flags != 0 )
>> +if ( iommu_flags )
>>  for ( i = 0; i < (1 << order); i++ )
>> -iommu_map_page(d, gfn + i, mfn_x(mfn) + i, flags);
>> +iommu_map_page(d, gfn + i, mfn_x(mfn) + i, 
> iommu_flags);
>>  else
>>  for ( i = 0; i < (1 << order); i++ )
>>  iommu_unmap_page(d, gfn + i);
> 
> EPT changes:
> 
> Reviewed-by: George Dunlap 

... and commit the EPT side.

>> --- a/xen/arch/x86/mm/p2m-pt.c
>> +++ b/xen/arch/x86/mm/p2m-pt.c
>> @@ -488,12 +488,13 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
>>  void *table;
>>  unsigned long i, gfn_remainder = gfn;
>>  l1_pgentry_t *p2m_entry;
>> -l1_pgentry_t entry_content;
>> +l1_pgentry_t entry_content, old_entry = l1e_empty();
>>  l2_pgentry_t l2e_content;
>>  l3_pgentry_t l3e_content;
>>  int rc;
>>  unsigned int iommu_pte_flags = p2m_get_iommu_flags(p2mt);
>> -unsigned long old_mfn = 0;
>> +unsigned int old_flags = 0;
>> +unsigned long old_mfn = INVALID_MFN;
> 
> I think this could use at least a comment, and perhaps some better
> variable naming here explaining what setting or not setting each of
> these different variables means.
> 
> Trying to sort out what the effect of setting old_flags to ~0 is
> particularly convoluted.
> 
> Inferring from the code:
> 
> 1) Setting old_entry() to some value will cause those values to be
> unmapped.  This will only be set for 1G and 2M entries, if the existing
> entry is both present and not set as a superpage.  This is pretty
> straightforward and looks correct.
> 
> 2) old_mfn and old_flags are primarily used to control whether an IOMMU
> flush happens.
> 
> 3) old_mfn
>  - old_mfn begins at INVALID_MFN, and is set only if the entry itself is
> leaf entry.
>  - a flush will happen if old_mfn != mfn
>  - The effect of this will be to force a flush if replacing a leaf with
> a leaf but a different mfn, or if replacing an intermediate table with a
> leaf *if* the leaf is not INVALID_MFN
> 
> 4) old_flags
>  - old_flags will be the actual flags if replacing a leaf with a leaf,
> or ~0 if replacing an intermediate table with a leaf.
>  - a flush will happen if
> p2m_iommu_get_flags(p2m_flags_to_type(old_flags)) != iommu_pte_flags.
>  - p2m_flags_to_type
>- returns p2m_invalid if flags == 0.  This should never happen here
>- returns the type bits from the flags otherwires.
>- So in the case of old_flags being the actual flags, you get the
> actual type of the old entry
>- in the case of old_flags being ~0, you get 0x7f, which is undefined
>  - p2m_iommu_get_flags() will return 0 for unknown types.
>  - so the effect of the above will be to force a flush if replacing a
> leaf with a leaf of different iommu permisions; or, to force a flush if
> replacing an intermediate table with any permissions with a leaf that
> has at least some iommu 

Re: [Xen-devel] [PATCH 1/3] x86/p2m: tighten conditions of IOMMU mapping updates

[Jan Beulich]

>>> On 21.09.15 at 16:02,  wrote:
> In the EPT case permission changes should also result in updates or
> TLB flushes.
> 
> In the NPT case the old MFN does not depend on the new entry being
> valid (but solely on the old one), and the need to update or TLB-flush
> again also depends on permission changes.
> 
> In the shadow mode case, iommu_hap_pt_share should be ignored.
> 
> Furthermore in the NPT/shadow case old intermediate page tables must
> be freed only after IOMMU side updates/flushes have got carried out.
> 
> Signed-off-by: Jan Beulich 
> ---
> In addition to the fixes here it looks to me as if both EPT and
> NPT/shadow code lack invalidation of IOMMU side paging structure
> caches, i.e. further changes may be needed. Am I overlooking something?

Okay, I think I finally figured it myself (with the help of the partial
patch presented yesterday): There is no need for more flushing
in the current model, where we only ever split large pages or fully
replace sub-hierarchies with large pages (accompanied by a suitable
flush already). More flushing would only be needed if we were to
add code to re-establish large pages if an intermediate page table
re-gained a state where this would be possible (quite desirable
in a situation where log-dirty mode got temporarily enabled on a
domain, but I'm not sure how common an operation that would be).
When splitting large pages, all the hardware can have cached are
- leaf entries the size of the page being split
- leaf entries of smaller size sub-pages (in case large pages don't
  get entered into the TLB)
- intermediate entries from above the leaf entry being split (which
  don't get changed)
Leaf entries already get flushed correctly, and since the involved
intermediate entries don't get changed, not flush is needed there.

Jan


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

Re: [Xen-devel] [PATCH V3 2/2] xen: Introduce VM_EVENT_FLAG_SET_REGISTERS

[Jan Beulich]

>>> On 28.09.15 at 20:46,  wrote:
>> > +if ( rsp.flags & VM_EVENT_FLAG_SET_REGISTERS )
>> >> +vm_event_set_registers(v, );
>> >> +
>> >>  if ( rsp.flags & VM_EVENT_FLAG_TOGGLE_SINGLESTEP )
>> >>  vm_event_toggle_singlestep(d, v);
>> >
>> > What meaning has setting both flags and EFLAGS.TF in the new
>> > register values?
>>
>> That's a good question. I'm not sure how that would affect an attached
>> debugger type scenario.
>>
>> I'm also unsure of what a good solution to this issue would be. I could
>> make the flags exclusive, but that would prevent, for example, setting
>> EAX and singlestepping, which should not be a problem. I could also
>> remove the eflags assignment from vm_event_set_registers() (maybe
>> replace it with a comment).
>>
>> Tamas, do you need eflags set? What do you think? Again, I'm happy with
>> just setting EIP, what scenarios are you interested in?
>>
>>
> Being able to set registers this way and enabling MTF in one-shot I think
> offers good flexibility and performance for vm_event applications. I don't
> see any reason why these options should be exclusive. Furthermore, I don't
> see why we would treat EFLAGS.TF any different then the others. If the
> vm_event application decides to flip both the in-guest TF and the external
> MTF, why prevent that? While I don't see an immediate use-case for this,  I
> also don't see why it would be problematic either.

Okay. Sounds like an ack then - if so, could you formalize that?

Jan


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

Re: [Xen-devel] [Patch RFC 00/13] VT-d Asynchronous Device-TLB Flush for ATS Device

[Jan Beulich]

>>> On 29.09.15 at 04:53,  wrote:
 Monday, September 28, 2015 2:47 PM, wrote:
>> >>> On 28.09.15 at 05:08,  wrote:
>>  Thursday, September 24, 2015 12:27 AM, Tim Deegan wrote:
> 
>> It would be a guest kernel bug, but all _we_ care about is that such a guest 
> kernel
>> bug won't affect the hypervisor or other guests.
> 
> It won't affect the hypervisor or other guest domains.
> As the required Device-TLB flushes are not applied, the hypercall is not 
> completed. The being freed page is still owned by this buggy
> Guest, not released back to xen or reallocated for other guests.

Seems like you misunderstood the purpose of my reply: I wasn't
claiming that what you patch set currently does would constitute an
issue. I was simply stating a general rule to consider when thinking
about which solutions are viable and which aren't.

> For Tim's suggestion --"to make the IOMMU table take typed refcounts to
> anything it points to, and only drop those refcounts when the flush 
> completes."
> 
> From IOMMU point of view, if it can walk through IOMMU table to get these 
> pages and take typed refcounts. 
> These pages are maybe owned by hardware_domain, dummy, HVM guest .etc. could 
> I narrow it down to HVM guest? --- It is not for anything it points to, but 
> just 
> for HVM guest related. this will simplify the design.

I don't follow. Why would you want to walk page tables? And why
would a HVM guest have pages other than those owned by itself or
granted access to by another guest mapped in its IOMMU page
tables? In any event - the ref-counting would need to happen as
you _create_ the mappings, not at some later point.

> from HVM guest point of view, once the ATS device is assigned, we can: 
> *pause the HVM guest domain.
> *scan domain's xenpage_list, page_list and arch.relmem_list to get these 
> pages, which will be took typed refcounts ( PGT_dev_tlb_page -- a new type).
> *unpause the HVM guest domain.
> 
> (we can ignore domain's xenpage_list) as:
> ((
>Actually, the previous pages are maybe mapped from Xen heap for guest 
> domains in decrease_reservation() / xenmem_add_to_physmap_one()
>/ p2m_add_foreign(), but they are not mapped to IOMMU table. The below 4 
> functions will map xen heap page for guest domains:
>   * share page for xen Oprofile.
>   * vLAPIC mapping.
>   * grant table shared page.
>   * domain share_info page.
> ))

Neither of which really has a need to be in the IOMMU page tables
afaics.

>  Just for check, do typed refcounts refer to the following?
> 
> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -183,6 +183,7 @@ struct page_info
>  #define PGT_seg_desc_page PG_mask(5, 4)  /* using this page in a GDT/LDT?  */
>  #define PGT_writable_page PG_mask(7, 4)  /* has writable mappings? */
>  #define PGT_shared_page   PG_mask(8, 4)  /* CoW sharable page  */
> +#define PGT_dev_tlb_page  PG_mask(9, 4)  /* Maybe in Device-TLB mapping?   */
>  #define PGT_type_mask PG_mask(15, 4) /* Bits 28-31 or 60-63.   */
> 
> * I define a new typed refcounts PGT_dev_tlb_page.

Why? I.e. why won't a base ref for r/o pages and a writable type-ref
for r/w ones suffice, just like we do everywhere else?

>> Once you do that, I
>> don't think there'll be a reason to pause the guest for the duration of the 
> flush.
>> And really (as pointed out before) pausing the guest would get us _far_ away
>> from how real hardware behaves.
>> 
> 
> Once I do that, I think the guest should be still paused, if the Device-TLB 
> flush is not completed.
> 
> As mentioned in previous email, for example:
> Call do_memory_op HYPERCALL to free a pageX (gfn1 <---> mfn1). The gfn1 is 
> the 
> freed portion of GPA.
> assume that there is a mapping(gfn1<---> mfn1) in Device-TLB. If the 
> Device-TLB 
> flush is not completed and return to guest mode,
> the guest may call do_memory_op HYPERCALL to allocate a new pageY(mfn2) to 
> gfn1..
> then:
> the EPT mapping is (gfn1--mfn2), the Device-TLB mapping is (gfn1<--->mfn1) .
> 
> If the Device-TLB flush is not completed, DMA associated with gfn1 may still 
> write some data with pageX(gfn1 <---> mfn1), but pageX will be 
> Released to xen when the Device-TLB flush is completed. It is maybe not 
> correct for guest to read data from gfn1 after DMA(now the page associated 
> with gfn1 is pageY ).
> 
> Right?

No. The extra ref taken will prevent the page from getting freed. And
as long as the flush is in process, DMA to/from the page is going to
produce undefined results (affecting only the guest). But note that
there may be reasons for an external to the guest entity invoking the
operation which ultimately led to the flush to do this on a paused guest
only. But that's not of concern to the hypervisor side implementation.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xen.org

[Xen-devel] [linux-3.18 baseline-only test] 38087: tolerable FAIL

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 38087 linux-3.18 real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/38087/

Failures :-/ but no regressions.

Tests which did not succeed,
including tests which could not be run:
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass
 test-armhf-armhf-xl-qcow2 9 debian-di-installfail   never pass
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 test-armhf-armhf-xl-raw   9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-rumpuserxen-amd64 15 
rumpuserxen-demo-xenstorels/xenstorels.repeat fail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  16 guest-start/debian.repeatfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  16 guest-start/debian.repeatfail   never pass
 test-amd64-i386-freebsd10-amd64  9 freebsd-install fail never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-midway   13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-midway   12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-amd64-i386-freebsd10-i386  9 freebsd-install  fail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-raw  11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-vhd  11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qcow2 11 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemut-winxpsp3  9 windows-install  fail never pass
 test-amd64-amd64-xl-qemuu-winxpsp3  9 windows-install  fail never pass

version targeted for testing:
 linuxd048c068d00da7d4cfa5ea7651933b99026958cf

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   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops  

Re: [Xen-devel] [PATCH v6 24/29] xen/x86: allow HVM guests to use hypercalls to bring up vCPUs

[Jan Beulich]

>>> On 28.09.15 at 18:09,  wrote:
> El 21/09/15 a les 17.44, Jan Beulich ha escrit:
> On 04.09.15 at 14:09,  wrote:
>>> Allow the usage of the VCPUOP_initialise, VCPUOP_up, VCPUOP_down and
>>> VCPUOP_is_up hypercalls from HVM guests.
>>>
>>> This patch introduces a new structure (vcpu_hvm_context) that should be used
>>> in conjuction with the VCPUOP_initialise hypercall in order to initialize
>>> vCPUs for HVM guests.
>>>
>>> Signed-off-by: Roger Pau Monné 
>>> Signed-off-by: Andrew Cooper 
>> 
>> So this bi-modal thing doesn't look too bad, but a concern I have is
>> with the now different contexts used by XEN_DOMCTL_setvcpucontext
>> and VCPUOP_initialise.
> 
> Yes, that's far from ideal. I was going to say that
> XEN_DOMCTL_{set/get}vcpucontext should return EOPNOTSUPP when executed
> against HVM guests, but that's going to break current toolstack code
> that relies on this in order to perform suspend/resume of HVM guests
> (and gdbsx would probably also be affected).
> 
> If you feel that would be a better solution I could fix current tools
> code in order to use XEN_DOMCTL_{get/set}hvmcontext instead of
> XEN_DOMCTL_{set/get}vcpucontext when dealing with HVM guests.

That might be a follow-up thing, subject to the tools maintainers
agreeing.

>>> +if ( !paging_mode_hap(v->domain) )
>>> +v->arch.guest_table = pagetable_null();
>> 
>> A comment with the reason for this would be nice. I can't immediately
>> see why this is here.
> 
> guest_table contains uninitialized data at this point, which makes
> pagetable_is_null return false. Other places where guest_table is set
> check if previous guest_table is null or not, and if it's not null Xen
> will try to free it, causing a bug. hvm_vcpu_reset_state does something
> similar when setting the initial vCPU state.

Truly uninitialized is not possible, as struct vcpu starts out zeroed.
Hence if anything the field may hold left over data, in which case
the store would better go where the reference becomes dangling.

>>> +hvm_update_guest_cr(v, 0);
>>> +hvm_update_guest_cr(v, 4);
>>> +
>>> +if ( (ctx->mode == VCPU_HVM_MODE_32B) ||
>>> + (ctx->mode == VCPU_HVM_MODE_64B) )
>>> +{
>>> +hvm_update_guest_cr(v, 3);
>>> +hvm_update_guest_efer(v);
>>> +}
>>> +
>>> +if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) )
>>> +{
>>> +/* Shadow-mode CR3 change. Check PDBR and update refcounts. */
>>> +struct page_info *page = get_page_from_gfn(v->domain,
>>> + v->arch.hvm_vcpu.guest_cr[3] >> 
>>> PAGE_SHIFT,
>>> + NULL, P2M_ALLOC);
>> 
>> P2M_ALLOC but not P2M_UNSHARE?
> 
> This is a copy of what's done in hvm_set_cr3 when shadow mode is enabled.

As said on IRC - sadly we have to mem-sharing maintainer to ask. I
wonder whether the past or current x86/mm maintainer would know
- Tim, George?

>>> +/* Sync AP's TSC with BSP's. */
>>> +v->arch.hvm_vcpu.cache_tsc_offset =
>>> +v->domain->vcpu[0]->arch.hvm_vcpu.cache_tsc_offset;
>>> +hvm_funcs.set_tsc_offset(v, v->arch.hvm_vcpu.cache_tsc_offset,
>>> + v->domain->arch.hvm_domain.sync_tsc);
>>> +
>>> +v->arch.hvm_vcpu.msr_tsc_adjust = 0;
>> 
>> The need for this one I also can't see right away - another comment
>> perhaps?
> 
> AFAICT we need to initialize this to a sane value, like it's done in
> hvm_vcpu_reset_state.

As said above - struct vcpu starts out zeroed, so unless stale data
is being left here, "initialize" would at least not be the right term.

>>> +#ifndef __XEN_PUBLIC_HVM_HVM_VCPU_H__
>>> +#define __XEN_PUBLIC_HVM_HVM_VCPU_H__
>>> +
>>> +#include "../xen.h"
>>> +
>>> +struct vcpu_hvm_x86_16 {
>>> +uint16_t ax;
>>> +uint16_t cx;
>>> +uint16_t dx;
>>> +uint16_t bx;
>>> +uint16_t sp;
>>> +uint16_t bp;
>>> +uint16_t si;
>>> +uint16_t di;
>>> +uint16_t ip;
>>> +uint16_t flags;
>>> +
>>> +uint32_t cr0;
>>> +uint32_t cr4;
>>> +
>>> +uint32_t cs_base;
>>> +uint32_t ds_base;
>>> +uint32_t ss_base;
>>> +uint32_t tr_base;
>>> +uint32_t cs_limit;
>>> +uint32_t ds_limit;
>>> +uint32_t ss_limit;
>>> +uint32_t tr_limit;
>>> +uint16_t cs_ar;
>>> +uint16_t ds_ar;
>>> +uint16_t ss_ar;
>>> +uint16_t tr_ar;
>>> +};
>> 
>> I doubt this is very useful: While one ought to be able to start a
>> guest in 16-bit mode, its GPRs still are supposed to be 32 bits wide.
>> The mode used really would depend on the cs_ar setting. (Having
>> the structure just for a 16-bit IP would seem insane.)
> 
> So, would you prefer to go just with the 64-bit structure and the mode
> is simply set by the value of the control registers / segment selectors?

No, you certainly want a 32- and a 64-bit layout, for the benefit of
32- and 64-bit guests.

>>> +uint32_t 

Re: [Xen-devel] [PATCH for Xen 4.6 3/5] tools/libxl: return socket id from libxl_psr_cat_get_l3_info

[Dario Faggioli]

On Tue, 2015-09-29 at 11:05 +0800, Chao Peng wrote:
> On Mon, Sep 28, 2015 at 04:46:17PM +0100, Wei Liu wrote:
> > On Mon, Sep 28, 2015 at 05:35:56PM +0200, Dario Faggioli wrote:

> > > But since now you're building the full bitmap, we can use
> > > libxl_bitmap_count_set(), for that.
> > > 
> > > This may not be a bit deal, but if I'm not wrong, it saves us an
> > > hypercall (the PHYSINFO that libxl__count_physical_socket()
> > > issues).
> > > 
> 
> I can pass 0 to libxl_socket_bitmap_alloc() but it will call
> hypercall
> internally. We need the size for the socketmap anyway before we
> allocating it.
> 
Yes, looking better, the number of hypercall we need seems the same in
both cases. Furthermore, as Wei said, this is an internal detail /
optimization that we can always look into in future, so nevermind, do
as you like best.

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R 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

[Xen-devel] [PATCH v2 for Xen 4.6 6/6] docs/man: resort sections

[Chao Peng]

Section 'IGNORED FOR COMPATIBILITY WITH XM' separates 'CACHE MONITORING
TECHNOLOGY' and 'CACHE ALLOCATION TECHNOLOGY' but they really should be
put together.

Signed-off-by: Chao Peng 
---
Current incorrect output can be seen at:
http://xenbits.xen.org/docs/unstable/man/xl.1.html 
---
 docs/man/xl.pod.1 | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index f22c3f3..d0cd612 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1509,18 +1509,6 @@ monitor types are:
 
 =back
 
-=head1 IGNORED FOR COMPATIBILITY WITH XM
-
-xl is mostly command-line compatible with the old xm utility used with
-the old Python xend.  For compatibility, the following options are
-ignored:
-
-=over 4
-
-=item B
-
-=back
-
 =head2 CACHE ALLOCATION TECHNOLOGY
 
 Intel Broadwell and later server platforms offer capabilities to configure and
@@ -1553,6 +1541,18 @@ Show CAT settings for a certain domain or all domains.
 
 =back
 
+=head1 IGNORED FOR COMPATIBILITY WITH XM
+
+xl is mostly command-line compatible with the old xm utility used with
+the old Python xend.  For compatibility, the following options are
+ignored:
+
+=over 4
+
+=item B
+
+=back
+
 =head1 TO BE DOCUMENTED
 
 We need better documentation for:
-- 
1.9.1


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

[Xen-devel] [PATCH v2 for Xen 4.6 0/6] Several PSR fixes in libxl

[Chao Peng]

The patch series basically contain several PSR related fixes in libxl.
patch1-3: fix the socket display error in certain hotplug case.
patch4:   fix a minor range check.
patch5:   improve the PSR document.
patch6:   improve xl man page.

Detailed problem and fix please see commit message.

Change history
v2:
* Address comments from Wei/Dario.
* Add patch6.

Chao Peng (6):
  tools/libxl: introduce libxl_get_online_socketmap
  tools/libxl: fix socket display error for CMT
  tools/libxl: return socket id from libxl_psr_cat_get_l3_info
  tools/libxl: fix range check in main_psr_cat_cbm_set
  docs: make xl-psr.markdown more precise
  docs/man: resort sections

 docs/man/xl.pod.1   | 24 ++---
 docs/misc/xl-psr.markdown   |  8 ++---
 tools/libxl/libxl.h |  7 ++--
 tools/libxl/libxl_psr.c | 23 +---
 tools/libxl/libxl_types.idl |  1 +
 tools/libxl/libxl_utils.c   | 22 
 tools/libxl/libxl_utils.h   |  2 ++
 tools/libxl/xl_cmdimpl.c| 86 +++--
 8 files changed, 107 insertions(+), 66 deletions(-)

-- 
1.9.1


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

[Xen-devel] [PATCH v2 for Xen 4.6 5/6] docs: make xl-psr.markdown more precise

[Chao Peng]

Drop the chapter number as it can be confusing when it gets changed in
the referred document.

Signed-off-by: Chao Peng 
Reviewed-by: Dario Faggioli 
Acked-by: Wei Liu 
---
v2:
* minor commit message adjustment.
---
 docs/misc/xl-psr.markdown | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
index 3545912..737f0f7 100644
--- a/docs/misc/xl-psr.markdown
+++ b/docs/misc/xl-psr.markdown
@@ -14,7 +14,7 @@ tracks cache utilization of memory accesses according to the 
RMID and reports
 monitored data via a counter register.
 
 For more detailed information please refer to Intel SDM chapter
-"17.14 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
+"Platform Shared Resource Monitoring: Cache Monitoring Technology".
 
 In Xen's implementation, each domain in the system can be assigned a RMID
 independently, while RMID=0 is reserved for monitoring domains that don't
@@ -52,7 +52,7 @@ event type to monitor system total/local memory bandwidth. 
The same RMID can
 be used to monitor both cache usage and memory bandwidth at the same time.
 
 For more detailed information please refer to Intel SDM chapter
-"17.14 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
+"Overview of Cache Monitoring Technology and Memory Bandwidth Monitoring".
 
 In Xen's implementation, MBM shares the same set of underlying monitoring
 service with CMT and can be used to monitor memory bandwidth on a per domain
@@ -92,7 +92,7 @@ For example, assuming a system with 8 portions and 3 domains:
access to one quarter each.
 
 For more detailed information please refer to Intel SDM chapter
-"17.15 - Platform Shared Resource Control: Cache Allocation Technology".
+"Platform Shared Resource Control: Cache Allocation Technology".
 
 In Xen's implementation, CBM can be configured with libxl/xl interfaces but
 COS is maintained in hypervisor only. The cache partition granularity is per
@@ -130,4 +130,4 @@ Per domain CBM settings can be shown by:
 ## Reference
 
 [1] Intel SDM
-(http://www.intel.com/content/www/us/en/processors/architectures-software-developer-manuals.html).
+(http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-system-programming-manual-325384.pdf).
-- 
1.9.1


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

[Xen-devel] [PATCH v2 for Xen 4.6 3/6] tools/libxl: return socket id from libxl_psr_cat_get_l3_info

[Chao Peng]

The entries returned from libxl_psr_cat_get_l3_info are assumed
to be socket-continuous. But this is not true in the hotplug case.

This patch gets the socket bitmap for all the sockets on the system
first and stores the socket id in the structure libxl_psr_cat_info in
libxl_psr_cat_get_l3_info. The xl or similar consumers then can display
socket information correctly.

Signed-off-by: Chao Peng 
---
v2:
* add libxl_bitmap_init();
* rename target_id to id.
* fix the iteration code in psr_cat_hwinfo().
---
 tools/libxl/libxl_psr.c | 23 ++-
 tools/libxl/libxl_types.idl |  1 +
 tools/libxl/xl_cmdimpl.c| 41 -
 3 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 3378239..30740a1 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -339,30 +339,43 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, 
libxl_psr_cat_info **info,
 {
 GC_INIT(ctx);
 int rc;
-int i, nr_sockets;
+int i = 0, socket, nr_sockets;
+libxl_bitmap socketmap;
 libxl_psr_cat_info *ptr;
 
+libxl_bitmap_init();
+
 rc = libxl__count_physical_sockets(gc, _sockets);
 if (rc) {
 LOGE(ERROR, "failed to get system socket count");
 goto out;
 }
 
+libxl_socket_bitmap_alloc(ctx, , nr_sockets);
+rc = libxl_get_online_socketmap(ctx, );
+if (rc < 0) {
+LOGE(ERROR, "failed to get available sockets");
+goto out;
+}
+
 ptr = libxl__malloc(NOGC, nr_sockets * sizeof(libxl_psr_cat_info));
 
-for (i = 0; i < nr_sockets; i++) {
-if (xc_psr_cat_get_l3_info(ctx->xch, i, [i].cos_max,
-[i].cbm_len)) {
+libxl_for_each_set_bit(socket, socketmap) {
+ptr[i].id = socket;
+if (xc_psr_cat_get_l3_info(ctx->xch, socket, [i].cos_max,
+ [i].cbm_len)) {
 libxl__psr_cat_log_err_msg(gc, errno);
 rc = ERROR_FAIL;
 free(ptr);
 goto out;
 }
+i++;
 }
 
 *info = ptr;
-*nr = nr_sockets;
+*nr = i;
 out:
+libxl_bitmap_dispose();
 GC_FREE;
 return rc;
 }
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 9f6ec00..dc84864 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -792,6 +792,7 @@ libxl_psr_cbm_type = Enumeration("psr_cbm_type", [
 ])
 
 libxl_psr_cat_info = Struct("psr_cat_info", [
+("id", uint32),
 ("cos_max", uint32),
 ("cbm_len", uint32),
 ])
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index f01d245..9947dba 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8384,35 +8384,35 @@ int main_psr_cmt_show(int argc, char **argv)
 static int psr_cat_hwinfo(void)
 {
 int rc;
-int socketid, nr_sockets;
+int i, nr;
 uint32_t l3_cache_size;
 libxl_psr_cat_info *info;
 
 printf("Cache Allocation Technology (CAT):\n");
 
-rc = libxl_psr_cat_get_l3_info(ctx, , _sockets);
+rc = libxl_psr_cat_get_l3_info(ctx, , );
 if (rc) {
 fprintf(stderr, "Failed to get cat info\n");
 return rc;
 }
 
-for (socketid = 0; socketid < nr_sockets; socketid++) {
-rc = libxl_psr_cmt_get_l3_cache_size(ctx, socketid, _cache_size);
+for (i = 0; i < nr; i++) {
+rc = libxl_psr_cmt_get_l3_cache_size(ctx, info[i].id, _cache_size);
 if (rc) {
 fprintf(stderr, "Failed to get l3 cache size for socket:%d\n",
-socketid);
+info[i].id);
 goto out;
 }
-printf("%-16s: %u\n", "Socket ID", socketid);
+printf("%-16s: %u\n", "Socket ID", info[i].id);
 printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
-printf("%-16s: %u\n", "Maximum COS", info->cos_max);
-printf("%-16s: %u\n", "CBM length", info->cbm_len);
+printf("%-16s: %u\n", "Maximum COS", info[i].cos_max);
+printf("%-16s: %u\n", "CBM length", info[i].cbm_len);
 printf("%-16s: %#llx\n", "Default CBM",
-   (1ull << info->cbm_len) - 1);
+   (1ull << info[i].cbm_len) - 1);
 }
 
 out:
-libxl_psr_cat_info_list_free(info, nr_sockets);
+libxl_psr_cat_info_list_free(info, nr);
 return rc;
 }
 
@@ -8454,47 +8454,46 @@ static int psr_cat_print_domain_cbm(uint32_t domid, 
uint32_t socketid)
 return 0;
 }
 
-static int psr_cat_print_socket(uint32_t domid, uint32_t socketid,
-libxl_psr_cat_info *info)
+static int psr_cat_print_socket(uint32_t domid, libxl_psr_cat_info *info)
 {
 int rc;
 uint32_t l3_cache_size;
 
-rc = libxl_psr_cmt_get_l3_cache_size(ctx, socketid, _cache_size);
+rc = libxl_psr_cmt_get_l3_cache_size(ctx, info->id, _cache_size);
 if (rc) {
 fprintf(stderr, 

[Xen-devel] [PATCH v2 for Xen 4.6 1/6] tools/libxl: introduce libxl_get_online_socketmap

[Chao Peng]

It sets the bit on the given bitmap if the corresponding socket is
available and clears the bit when the corresponding socket is not
available.

Signed-off-by: Chao Peng 
---
v2:
* rename libxl_socket_bitmap_fill => libxl_get_online_socketmap.
* fix blanklines.

NOTE:LIBXL_HAVE_SOCKET_BITMAP_ALLOC is changed as we are still in 4.6 release 
cycle.
---
 tools/libxl/libxl.h   |  7 ---
 tools/libxl/libxl_utils.c | 22 ++
 tools/libxl/libxl_utils.h |  2 ++
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5f9047c..fa5aedd 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -807,11 +807,12 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, 
libxl_mac *src);
 #define LIBXL_HAVE_PCITOPOLOGY 1
 
 /*
- * LIBXL_HAVE_SOCKET_BITMAP_ALLOC
+ * LIBXL_HAVE_SOCKET_BITMAP
  *
- * If this is defined, then libxl_socket_bitmap_alloc exists.
+ * If this is defined, then libxl_socket_bitmap_alloc and
+ * libxl_get_online_socketmap exist.
  */
-#define LIBXL_HAVE_SOCKET_BITMAP_ALLOC 1
+#define LIBXL_HAVE_SOCKET_BITMAP 1
 
 /*
  * LIBXL_HAVE_SRM_V2
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index bfc9699..408ec85 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -886,6 +886,28 @@ int libxl_socket_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap 
*socketmap,
 
 }
 
+int libxl_get_online_socketmap(libxl_ctx *ctx, libxl_bitmap *socketmap)
+{
+libxl_cputopology *tinfo = NULL;
+int nr_cpus = 0, i, rc = 0;
+
+tinfo = libxl_get_cpu_topology(ctx, _cpus);
+if (tinfo == NULL) {
+rc = ERROR_FAIL;
+goto out;
+}
+
+libxl_bitmap_set_none(socketmap);
+for (i = 0; i < nr_cpus; i++)
+if (tinfo[i].socket != XEN_INVALID_SOCKET_ID
+&& !libxl_bitmap_test(socketmap, tinfo[i].socket))
+libxl_bitmap_set(socketmap, tinfo[i].socket);
+
+ out:
+libxl_cputopology_list_free(tinfo, nr_cpus);
+return rc;
+}
+
 int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
 const libxl_bitmap *nodemap,
 libxl_bitmap *cpumap)
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 1e5ca8a..339ebdf 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -143,6 +143,8 @@ int libxl_node_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap 
*nodemap,
 int max_nodes);
 int libxl_socket_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *socketmap,
   int max_sockets);
+/* Fill socketmap with the CPU topology information on the system. */
+int libxl_get_online_socketmap(libxl_ctx *ctx, libxl_bitmap *socketmap);
 
 /* Populate cpumap with the cpus spanned by the nodes in nodemap */
 int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
-- 
1.9.1


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

Re: [Xen-devel] [PATCH v2] PVH Dom0 RMRR IOMMU mapping regression fix

[Wei Liu]

On Tue, Sep 29, 2015 at 06:00:38AM -0600, Jan Beulich wrote:
> >>> On 25.09.15 at 22:59,  wrote:
> > From: Elena Ufimtseva 
> > 
> > This patch addresses a regression introduced by commit 
> > 5ae03990c120a7b3067a52d9784c9aa72c0705a6 in new set_identity_p2m_entry.
> > RMRRs are not being mapped in IOMMU for PVH Dom0. This causes pages faults 
> > and
> > some long 'hang-like' delays during Dom0 PVH boot and device assignments.
> > 
> > During construct_dom0, in PVH path p2m is being constructed and identity 
> > mapped
> > in IOMMU. The p2m type is p2m_mmio_direct and p2m access p2m_rwx.
> > New code used to map RMRRs invoked from rmrr_identity_mapping
> > checks if p2m entry exists with same type and access and if yes, skips iommu
> > mapping. Since there are p2m entries for pvh dom0 iomem, RMRRs are not being
> > mapped in IOMMU.
> > 
> > As was mentioned in the earlier discussion, the PVH Dom0 construction code
> > should be modified to properly map RMRR regions in IOMMU. Since change will 
> > be
> > too invasive, this solution is a temporary fix at this time before better
> > solution is in. Also as Jan mentioned, there is no need in having 'x' 
> > permissions
> > for p2m entry of a mmio region, thus changed here.
> > 
> > You comments and suggestions are welcome!
> > Thank you.
> > 
> > Signed-off-by: Elena Ufimtseva 
> 
> Wei,
> 
> how about this one for 4.6 then? On one hand I recall you validly
> stating that PVH is still unsupported, but otoh the change is well
> isolated against affecting anything but PVH, i.e. it is very low risk.
> 

I'm fine with this going in.

Release-acked-by: Wei Liu 

> Jan

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

Re: [Xen-devel] [PATCH] x86/p2m-pt: delay freeing of intermediate page tables

[Wei Liu]

On Tue, Sep 29, 2015 at 04:51:46AM -0600, Jan Beulich wrote:
> Old intermediate page tables must be freed only after IOMMU side
> updates/flushes have got carried out.
> 
> Signed-off-by: Jan Beulich 

Release-acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH 3/4 RFC] x86/p2m: use large pages for MMIO mappings

[Jan Beulich]

>>> On 29.09.15 at 14:16,  wrote:
> On 29/09/15 12:44, Jan Beulich wrote:
> On 29.09.15 at 13:33,  wrote:
>>> On 15/09/15 08:34, Jan Beulich wrote:
 RFC reasons:
 - ARM side unimplemented (and hence libxc for now made cope with both
   models), the main issue (besides my inability to test any change
   there) being the many internal uses of map_mmio_regions())
>>>
>>> map_mmio_regions is used in ARM to map all the device memory in a guest.
>>> We expect this function to map everything at once when called during
>>> DOM0 build and/or when a guest is created (used to map the interrupt
>>> controller).
>>>
>>> I would rather prefer to avoid introducing specific helpers with
>>> slightly different behavior (i.e one is only mapping N page, the other
>>> everything).
>>>
>>> What about extending map_mmio_regions to take a parameter telling if we
>>> want to limit the number of mapping in a single invocation?
>> 
>> Sure an option, albeit something that would be sufficient to be
>> done in ARM specific code, albeit the only user using variable
>> length is map_range_to_domain(). All the others, using fixed
>> lengths up to 32 pages, would implicitly get everything done at
>> once as long as the threshold is >= 32.
> 
> While this is the case today, we have different patch series coming up
> using variable lenght in different place within the ARM code (vGIC,
> ACPI...).

Okay.

> It won't be possible to use map_range_to_domain because it's very
> specific to build DOM0.

Sure; I didn't even think of suggesting that.

> So, I would extend map_mmio_region like that
> 
> map_mmio_regions(struct domain *d,
>unsigned long start_gfn,
>unsigned long nr,
>unsigned long mfn,
>unsigned long limit);
> 
> The limit parameter would be 0 if there is no limit otherwise the
> maximum of iteration.

Again, make map_mmio_regions() a wrapper around an ARM-specific
function with the extra argument. No need to alter common or x86
code.

Jan


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

[Xen-devel] [xen-4.6-testing test] 62461: tolerable trouble: broken/fail/pass - PUSHED

[osstest service owner]

flight 62461 xen-4.6-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/62461/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt  3 host-install(3)   broken pass in 62328
 test-amd64-amd64-xl-qemut-debianhvm-amd64 19 guest-start/debianhvm.repeat fail 
in 62328 pass in 62461
 test-amd64-i386-xl-qemut-debianhvm-amd64 19 guest-start/debianhvm.repeat fail 
in 62328 pass in 62461
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 16 
guest-start/debianhvm.repeat fail pass in 62328

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail REGR. vs. 
62015
 test-armhf-armhf-xl-rtds 11 guest-start   fail REGR. vs. 62015
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 13 guest-localmigrate 
fail blocked in 62015

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 guest-saverestore fail in 62328 never pass
 test-armhf-armhf-libvirt 12 migrate-support-check fail in 62328 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
 test-armhf-armhf-xl-qcow2 9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-armhf-armhf-xl-raw   9 debian-di-installfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-i386-libvirt-qcow2 11 migrate-support-checkfail  never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-libvirt-raw  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-vhd  11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass

version targeted for testing:
 xen  7472ced461b4c4480a6dee2753a266f98e791456
baseline version:
 xen  70d63e48077f8fee8eda6d8d95eeda52a34d9077

Last test of basis62015  2015-09-14 22:22:30 Z   14 days
Failing since 62089  2015-09-17 06:48:20 Z   12 days5 attempts
Testing same since62328  2015-09-24 12:43:04 Z5 days2 attempts


People who touched revisions under test:
  Andrew Cooper 
  Chunyan Liu 
  Ian Campbell 
  Ian Jackson 
  Jan Beulich 
  Julien Grall 
  Kevin Tian 
  Roger Pau Monne 
  Roger Pau 

Re: [Xen-devel] [PATCH 3/4 RFC] x86/p2m: use large pages for MMIO mappings

[Julien Grall]

On 29/09/15 13:46, Jan Beulich wrote:
 On 29.09.15 at 14:16,  wrote:
>> On 29/09/15 12:44, Jan Beulich wrote:
>> On 29.09.15 at 13:33,  wrote:
 On 15/09/15 08:34, Jan Beulich wrote:
> RFC reasons:
> - ARM side unimplemented (and hence libxc for now made cope with both
>   models), the main issue (besides my inability to test any change
>   there) being the many internal uses of map_mmio_regions())

 map_mmio_regions is used in ARM to map all the device memory in a guest.
 We expect this function to map everything at once when called during
 DOM0 build and/or when a guest is created (used to map the interrupt
 controller).

 I would rather prefer to avoid introducing specific helpers with
 slightly different behavior (i.e one is only mapping N page, the other
 everything).

 What about extending map_mmio_regions to take a parameter telling if we
 want to limit the number of mapping in a single invocation?
>>>
>>> Sure an option, albeit something that would be sufficient to be
>>> done in ARM specific code, albeit the only user using variable
>>> length is map_range_to_domain(). All the others, using fixed
>>> lengths up to 32 pages, would implicitly get everything done at
>>> once as long as the threshold is >= 32.
>>
>> While this is the case today, we have different patch series coming up
>> using variable lenght in different place within the ARM code (vGIC,
>> ACPI...).
> 
> Okay.
> 
>> It won't be possible to use map_range_to_domain because it's very
>> specific to build DOM0.
> 
> Sure; I didn't even think of suggesting that.
> 
>> So, I would extend map_mmio_region like that
>>
>> map_mmio_regions(struct domain *d,
>>   unsigned long start_gfn,
>>   unsigned long nr,
>>   unsigned long mfn,
>>   unsigned long limit);
>>
>> The limit parameter would be 0 if there is no limit otherwise the
>> maximum of iteration.
> 
> Again, make map_mmio_regions() a wrapper around an ARM-specific
> function with the extra argument. No need to alter common or x86
> code.

TBH, extending the mapp_mmio_region is the best solution.

The name map_mmio_region is very generic and there is no reason we can't
use it in the hypervisor. Adding yet another wrapper will confuse people
and it will be hard for both the reviewer and the developer to know
which one to use.

Regards,

-- 
Julien Grall

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

[Xen-devel] [xen-unstable-smoke test] 62530: tolerable all pass - PUSHED

[osstest service owner]

flight 62530 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/62530/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  61598449ae28fe89036a699fd524b73b9ca454ae
baseline version:
 xen  3a1238d7cb73bc50f9ca1abede2b7cc59ec97bcd

Last test of basis62505  2015-09-28 19:52:10 Z0 days
Testing same since62530  2015-09-29 10:52:25 Z0 days1 attempts


People who touched revisions under test:
  Chao Peng 
  Jan Beulich 
  Kevin Tian 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

+ branch=xen-unstable-smoke
+ revision=61598449ae28fe89036a699fd524b73b9ca454ae
+ . cri-lock-repos
++ . cri-common
+++ . cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 
61598449ae28fe89036a699fd524b73b9ca454ae
+ branch=xen-unstable-smoke
+ revision=61598449ae28fe89036a699fd524b73b9ca454ae
+ . cri-lock-repos
++ . cri-common
+++ . cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . cri-common
++ . cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-unstable
+ : tested/2.6.39.x
+ . ap-common
++ xenbranch_forqemu=xen-unstable
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/staging/qemu-xen-unstable.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git
+++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src 
'[fetch=try]'
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local 'options=[fetch=try]'
 getconfig GitCacheProxy
 perl -e '
use Osstest;
readglobalconfig();
print $c{"GitCacheProxy"} or die $!;
'
+++ local 

Re: [Xen-devel] [PATCH] x86: detect CMOS aliasing on ports other then 0x70/0x71

[Andrew Cooper]

On 25/09/15 12:00, Jan Beulich wrote:
 On 25.09.15 at 11:55,  wrote:
>> It is safe for the SMM handler to use CMOS if it returns the index 
>> register back to how it found it.  Furthermore, I am willing to bet that 
>> there real SMM handlers out there which do do this.
> So what options do you see then here? Don't do anything (drop the
> patch) seems the only one I can see not getting in conflict with SMI.
> Yet using the patch as is would not make anything less safe, it would
> just have the potential of not always making things as safe as they
> could be.

Given some orthogonal interaction with cmos-rtc-probe recently, I am
fairly sure that we can remove all real RTC interaction from Xen.

Dom0 is required to provide fine-grained time via XENPF_settime{32,64}
which gives an absolute time to seed the other domain wallclocks with. 
The RTC was assumed always to be in UTC, which allows full date and time
information to be derived.

It would certainly simplify things to leave all of the RTC in dom0's hand.

~Andrew

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

Re: [Xen-devel] [PATCH v1 8/8] xen/arm: vgic-v3: Support 32-bit access for 64-bit registers

[Ian Campbell]

On Fri, 2015-09-25 at 15:51 +0100, Julien Grall wrote:
> Based on 8.1.3 (IHI 0069A), unless stated otherwise, the 64-bit registers
> supports both 32-bit and 64-bits access.


> 
> All the registers we properly emulate (i.e not RAZ/WI) supports 32-bit
> access.
> 
> For RAZ/WI, it's also seems to be the case but I'm not 100% sure. Anyway,
> emulating 32-bit access for them doesn't hurt. Note that we would need
> some extra care when they will be implemented (for instance
> GICR_PROPBASER).
> 
> Signed-off-by: Julien Grall 

Acked-by: Ian Campbell 

> ---
> This is technically fixing boot of FreeBSD ARM64 guest with GICv3.
> 
> AFAICT, Linux is not using 32-bit access in the GICv3 code expect
> for the ITS (which we don't support yet).
> 
> So this patch is a good candiate for Xen 4.6. Although this patch is
> heavily depend on previous patches. It may be possible to shuffle
> and move the "opmitization" patches towards the end. I haven't yet
> done that because I feel this series makes more sense in the current
> order.

I think if we let it bake in unstable for a bit to gain confidence we could
consider backporting the whole lot to either 4.6.1 or .2.


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

[Xen-devel] [OSSTEST PATCH 1/7] Debian installs: Nobble /etc/network/if-up.d/openssh-server

[Ian Jackson]

(See the comment in the new file for the explanation.)

This change affects all our Debian installs (both hosts and guests)
which are done with preseeding, because preseed_base() arranges to
install overlay/.

Signed-off-by: Ian Jackson 
---
 overlay/etc/network/if-up.d/openssh-server |   32 
 1 file changed, 32 insertions(+)
 create mode 100755 overlay/etc/network/if-up.d/openssh-server

diff --git a/overlay/etc/network/if-up.d/openssh-server 
b/overlay/etc/network/if-up.d/openssh-server
new file mode 100755
index 000..9fe2faf
--- /dev/null
+++ b/overlay/etc/network/if-up.d/openssh-server
@@ -0,0 +1,32 @@
+#!/bin/sh
+exit 0
+
+# In a default Debian install, this script reloads (or, in some
+# versions of Debian, restarts) sshd as new network interfaces come
+# up.  This is in case you have specific listen addresses specified in
+# the config.
+#
+# But the default config listens on 0.0.0.0 and ::.  So sshd is active
+# as soon as an interface is up, and does not need to be restarted or
+# reloaded.
+#
+# This restarting or reloading is harmful because it causes ssh to
+# stop listening briefly.  We can see the following race:
+#
+#  target sshd   target dhcp client osstest controller
+#
+#   startsstarts
+#   binds to ANY  obtains lease
+# configures eth0
+#connects to :22 with nc
+#   accepts conn nc succeeds
+#decides target sshd is up
+# runs ifup hook
+# ifup hook reloads
+#
+#   gets SIGHUP
+#   closes listen socket
+#   rereads config   runs ssh root@target
+#ssh gets ECONNREFUSED
+#   opens new listen socket
+#declares test fail
-- 
1.7.10.4


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

[Xen-devel] [OSSTEST PATCH 3/7] TestSupport: Honour $stdin fh argument to cmd, tcmd and tcmdex

[Ian Jackson]

These are internal functions, so the change is entirely within
TestSupport.  All the call sites are adjusted to pass undef so there
is no functional change.

Signed-off-by: Ian Jackson 
---
 Osstest/TestSupport.pm |   27 ---
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 7b9d815..91829a0 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -323,9 +323,13 @@ sub target_adjust_timeout ($$) {
 #-- running commands eg on targets --
 
 sub cmd {
-my ($timeout,$stdout,@cmd) = @_;
+my ($timeout,$stdin,$stdout,@cmd) = @_;
 my $child= fork;  die $! unless defined $child;
 if (!$child) {
+if (defined $stdin) {
+open STDIN, '<&', $stdin
+or die "STDIN $stdin $cmd[0] $!";
+}
 if (defined $stdout) {
 open STDOUT, '>&', $stdout
 or die "STDOUT $stdout $cmd[0] $!";
@@ -395,19 +399,20 @@ sub sshopts () {
 }
 
 sub tcmdex {
-my ($timeout,$stdout,$cmd,$optsref,@args) = @_;
+my ($timeout,$stdin,$stdout,$cmd,$optsref,@args) = @_;
 logm("executing $cmd ... @args");
 # We use timeout(1) as a backstop, in case $cmd doesn't die.  We
 # need $cmd to die because we won't release the resources we own
 # until all of our children are dead.
-my $r= cmd($timeout,$stdout, 'timeout',$timeout+30, $cmd,@$optsref,@args);
+my $r= cmd($timeout,$stdin,$stdout,
+  'timeout',$timeout+30, $cmd,@$optsref,@args);
 $r and die "status $r";
 }
 
 sub tgetfileex {
 my ($ruser, $ho,$timeout, $rsrc,$ldst) = @_;
 unlink $ldst or $!== or die "$ldst $!";
-tcmdex($timeout,undef,
+tcmdex($timeout,undef,undef,
'scp', sshopts(),
sshuho($ruser,$ho).":$rsrc", $ldst);
 } 
@@ -424,12 +429,12 @@ sub tputfileex {
 my ($ruser, $ho,$timeout, $lsrc,$rdst, $rsync) = @_;
 my @args= ($lsrc, sshuho($ruser,$ho).":$rdst");
 if (!defined $rsync) {
-tcmdex($timeout,undef,
+tcmdex($timeout,undef,undef,
'scp', sshopts(),
@args);
 } else {
 unshift @args, $rsync if length $rsync;
-tcmdex($timeout,undef,
+tcmdex($timeout,undef,undef,
'rsync', [ '-e', 'ssh '.join(' ',@{ sshopts() }) ],
@args);
 }
@@ -625,19 +630,19 @@ sub target_await_down ($$) {
 }
 
 sub tcmd { # $tcmd will be put between '' but not escaped
-my ($stdout,$user,$ho,$tcmd,$timeout,$extrasshopts) = @_;
+my ($stdin,$stdout,$user,$ho,$tcmd,$timeout,$extrasshopts) = @_;
 $timeout=30 if !defined $timeout;
 target_adjust_timeout($ho,\$timeout);
-tcmdex($timeout,$stdout,
+tcmdex($timeout,$stdin,$stdout,
'ssh', sshopts(), @{ $extrasshopts || [] },
sshuho($user,$ho), $tcmd);
 }
-sub target_cmd ($$;$$) { tcmd(undef,'osstest',@_); }
-sub target_cmd_root ($$;$$) { tcmd(undef,'root',@_); }
+sub target_cmd ($$;$$) { tcmd(undef,undef,'osstest',@_); }
+sub target_cmd_root ($$;$$) { tcmd(undef,undef,'root',@_); }
 
 sub tcmdout {
 my $stdout= IO::File::new_tmpfile();
-tcmd($stdout,@_);
+tcmd(undef,$stdout,@_);
 $stdout->seek(0,0) or die "$stdout $!";
 my $r;
 { local ($/) = undef;
-- 
1.7.10.4


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

[Xen-devel] [OSSTEST PATCH 5/7] Debian preseed: Break out debian_overlays

[Ian Jackson]

We are going to want to handle the overlays elswhere too, so factor
out the iteration over them.

Signed-off-by: Ian Jackson 
---
 Osstest/Debian.pm |   14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index a158f34..47d1767 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -34,6 +34,7 @@ BEGIN {
 $VERSION = 1.00;
 @ISA = qw(Exporter);
 @EXPORT  = qw(debian_boot_setup
+  debian_overlays
   %preseed_cmds
   preseed_base
   preseed_create
@@ -775,14 +776,23 @@ echo COMPRESS=/usr/sbin/osstest-initramfs-gzip >> \\
 END
 }
 
+sub debian_overlays ($) {
+my ($func) = @_;
+$func->($c{OverlayLocal}, 'overlay-local.tar');
+$func->('overlay', 'overlay.tar');
+}
+
 sub preseed_base (;@) {
 my ($ho,$suite,$sfx,$extra_packages,%xopts) = @_;
 
 $xopts{ExtraPreseed} ||= '';
 
 preseed_ssh($ho, $sfx);
-preseed_hook_overlay($ho, $sfx, $c{OverlayLocal}, 'overlay-local.tar');
-preseed_hook_overlay($ho, $sfx, 'overlay', 'overlay.tar');
+
+debian_overlays(sub {
+   my ($srcdir, $tfilename) = @_;
+   preseed_hook_overlay($ho, $sfx, $srcdir, $tfilename);
+});
 
 my $preseed = <<"END";
 d-i debian-installer/locale string en_GB
-- 
1.7.10.4


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

[Xen-devel] [OSSTEST PATCH 2/7] TestSupport::open_unique_stashfile: Provide a RDWR filehandle

[Ian Jackson]

The caller can then rewind and reread it if they feel like.

Signed-off-by: Ian Jackson 
---
 Osstest/TestSupport.pm |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 2b67e32..7b9d815 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -1005,7 +1005,7 @@ sub open_unique_stashfile ($) {
 my $dh;
 for (;;) {
 my $df= $$leafref;
-$dh= new IO::File "$stash/$df", O_WRONLY|O_EXCL|O_CREAT;
+$dh= new IO::File "$stash/$df", O_RDWR|O_EXCL|O_CREAT;
 last if $dh;
 die "$df $!" unless $!==
 next_unique_name $leafref;
-- 
1.7.10.4


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

Re: [Xen-devel] [PATCH 1/3] xen/arm: io: Extend write/read handler to pass private data

[Ian Campbell]

On Mon, 2015-09-28 at 21:31 +0100, Julien Grall wrote:
> Some handlers may require to use private data in order to get quickly
> information related to the region emulated.
> 
> Signed-off-by: Julien Grall 
> 
> ---
> 
> Cc: shameerali.kolothum.th...@huawei.com
> 
> This will be necessary in the follow-up in order to fix bug in the
> GICR emulation.
> ---
>  xen/arch/arm/io.c  | 15 +++
>  xen/arch/arm/vgic-v2.c |  8 +---
>  xen/arch/arm/vgic-v3.c | 17 +++--
>  xen/arch/arm/vuart.c   | 11 ++-
>  xen/include/asm-arm/mmio.h |  7 ---
>  5 files changed, 37 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
> index 8e55d49..85797f1 100644
> --- a/xen/arch/arm/io.c
> +++ b/xen/arch/arm/io.c
> @@ -37,18 +37,24 @@ int handle_mmio(mmio_info_t *info)
>  if ( (info->gpa >= mmio_handler->addr) &&
>   (info->gpa < (mmio_handler->addr + mmio_handler->size)) )
>  {
> -return info->dabt.write ?
> -mmio_handler->mmio_handler_ops->write_handler(v, info) :
> -mmio_handler->mmio_handler_ops->read_handler(v, info);
> +goto found;

Rather than goto use break instead.

>  }
>  }
>  
>  return 0;

And make this "if ( i == io_handlers->num_entries ) return 0;"

The continue to handle the op as you have done.

Other than that looks good.

Ian.


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

Re: [Xen-devel] [PATCH 2/3] xen/arm: vgic-v3: Correctly retrieve the vCPU associated to a re-distributor

[Ian Campbell]

On Mon, 2015-09-28 at 21:31 +0100, Julien Grall wrote:
> When the guest is accessing the re-distributor, Xen retrieves the base
> of the re-distributor using a mask based on the stride.
> 
> Although, when the stride contains multiple set, the corresponding mask

 ^bits?

(Also, drop the "Although, ").

> will be computed incorrectly [1] and therefore giving invalid vCPU and
> offset:
> 
> (XEN) d0v0: vGICR: unknown gpa read address 8d130008
> (XEN) traps.c:2447:d0v1 HSR=0x93c08006 pc=0xffc00032362c
> gva=0xff8b0008 gpa=0x008d130008
> 
> For instance if the region of re-distributor is starting at 0x8d10
> and the stride is 0x3, an access to the address 0x8d130008 should
> be valid and use the re-distributor of vCPU1 with an offset of 0x8.
> Although, Xen is returning the vCPU0 and an offset of 0x20008.
> 
> I didn't find a way to replace the current computation of the mask with
> a valid. The only solution I have found is to pass the region in private

^one

> data of the handler. So we can directly get the offset from the
> beginning of the region and find the corresponding vCPU/offset in the
> re-distributor.
> 
> This is also make the code simpler and avoid fast/slow path.
> 
> [1] http://lists.xen.org/archives/html/xen-devel/2015-09/msg03372.html
> 
> Signed-off-by: Julien Grall 
> 
> ---
> Cc: shameerali.kolothum.th...@huawei.com
> Cc: Wei Liu 
> 
> This is technically a good candidate for Xen 4.6. Without it DOM0
> may not be able to bring secondary CPU on platform using a stride
> with multiple bit set [1]. Although, we don't support such platform
> right now. So I would rather defer this change to Xen 4.6.1 and
> avoid possible downside/bug in this patch.

Agreed.

Other than the text:

Acked-by: Ian Campbell 
> 
> Shamaeerali, I've only tested quickly on the foundation model. Can
> you give a try on your platform to check if it fixes DOM0 boot?
> 
> [1] 
> http://lists.xen.org/archives/html/xen-devel/2015-09/msg03372.html
> ---
>  xen/arch/arm/vgic-v3.c | 58 +---
> --
>  1 file changed, 20 insertions(+), 38 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 6a4feb2..0a14184 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -593,55 +593,34 @@ write_ignore_32:
>  return 1;
>  }
>  
> -static inline struct vcpu *get_vcpu_from_rdist(paddr_t gpa,
> -   struct vcpu *v,
> -   uint32_t *offset)
> +static struct vcpu *get_vcpu_from_rdist(struct domain *d,
> +const struct vgic_rdist_region *region,
> +paddr_t gpa, uint32_t *offset)
>  {
> -struct domain *d = v->domain;
> +struct vcpu *v;
>  uint32_t stride = d->arch.vgic.rdist_stride;
> -paddr_t base;
> -int i, vcpu_id;
> -struct vgic_rdist_region *region;
> -
> -*offset = gpa & (stride - 1);
> -base = gpa & ~((paddr_t)stride - 1);
> -
> -/* Fast path: the VCPU is trying to access its re-distributor */
> -if ( likely(v->arch.vgic.rdist_base == base) )
> -return v;
> -
> -/* Slow path: the VCPU is trying to access another re-distributor */
> -
> -/*
> - * Find the region where the re-distributor lives. For this purpose,
> - * we look one region ahead as only MMIO range for redistributors
> - * traps here.
> - * Note: The region has been ordered during the GIC initialization
> - */
> -for ( i = 1; i < d->arch.vgic.nr_regions; i++ )
> -{
> -if ( base < d->arch.vgic.rdist_regions[i].base )
> -break;
> -}
> -
> -region = >arch.vgic.rdist_regions[i - 1];
> -
> -vcpu_id = region->first_cpu + ((base - region->base) / stride);
> +unsigned int vcpu_id;
>  
> +vcpu_id = region->first_cpu + ((gpa - region->base) / stride);
>  if ( unlikely(vcpu_id >= d->max_vcpus) )
>  return NULL;
>  
> -return d->vcpu[vcpu_id];
> +v = d->vcpu[vcpu_id];
> +
> +*offset = gpa - v->arch.vgic.rdist_base;
> +
> +return v;
>  }
>  
>  static int vgic_v3_rdistr_mmio_read(struct vcpu *v, mmio_info_t *info,
>  void *priv)
>  {
>  uint32_t offset;
> +const struct vgic_rdist_region *region = priv;
>  
>  perfc_incr(vgicr_reads);
>  
> -v = get_vcpu_from_rdist(info->gpa, v, );
> +v = get_vcpu_from_rdist(v->domain, region, info->gpa, );
>  if ( unlikely(!v) )
>  return 0;
>  
> @@ -661,10 +640,11 @@ static int vgic_v3_rdistr_mmio_write(struct vcpu
> *v, mmio_info_t *info,
>   void *priv)
>  {
>  uint32_t offset;
> +const struct vgic_rdist_region *region = priv;
>  
>  perfc_incr(vgicr_writes);
>  
> -v = get_vcpu_from_rdist(info->gpa, v, );
> 

Re: [Xen-devel] [PATCH 13/13] tools/libxl: Add 'vtsc_khz' option to set guest TSC rate

[Andrew Cooper]

On 29/09/15 14:53, Haozhong Zhang wrote:
> On Tue, Sep 29, 2015 at 11:28:38AM +0100, Ian Campbell wrote:
>> On Tue, 2015-09-29 at 11:24 +0100, Andrew Cooper wrote:
>>> On 29/09/15 11:13, Haozhong Zhang wrote:
 On Tue, Sep 29, 2015 at 11:04:14AM +0100, Ian Campbell wrote:
> On Mon, 2015-09-28 at 15:13 +0800, Haozhong Zhang wrote:
>> This patch adds an option 'vtsc_khz' to allow users to set vcpu's
>> TSC
>> rate in KHz. In the case that tsc_mode = 'default', the default
>> value of
>> 'vtsc_khz' option is the host TSC rate which is used when
>> 'vtsc_khz'
>> option is set to 0 or does not appear in the configuration. In all
>> other
>> cases of tsc_mode, 'vtsc_khz' option is just ignored.
>>
>> Another purpose of adding this option is to keep vcpu's TSC rate
>> across
>> guest reboot. In existing code, a new domain is created from the
>> configuration of the previous domain which was just rebooted.
>> vcpu's TSC
>> rate is not stored in the configuration and the host TSC rate is
>> the
>> used as vcpu's TSC rate. This works fine unless the previous domain
>> was
>> migrated from another host machine with a different host TSC rate
>> than
>> the current one.
> I understand why this is necessary over a migration, but why is it
> important to be able to retain the TSC rate across a reboot? What is
> the
> usecase there?
>
 No usecase so far. Is 'making a virtual machine more like a physical
 machine' reasonable here? (I suppose a physical machine retains TSC
 rate across reboot)
>>> There are situations such as altering firmware settings which can cause
>>> the TSC rate to differ across reboot.  I don't see any reason to try and
>>> maintain it across VM reboots.
>> Right. If it happens to come for free as a side effect of making it work
>> for migration then fine.
>>
>> But it seems to me that tsc rate could/should be in the hypervisors save
>> blob and require no interaction with the toolstack once it is latched when
>> the domain is built.
>>
>> Ian.
>>
> Seemingly I don't need vtsc_khz at all if retaining tsc rate across
> reboot is unnecessary (or even wrong). The migration of tsc rate is
> already done through write_tsc_info() and handle_tsc_info() w/o this
> patch. I'll check if "xl save/restore" also go through this path. If
> so, I think vtsc_khz can be removed.

I can confirm from my rewrite of migration that tsc info is explicitly
saved and restored in both PV and HVM migration.

~Andrew

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

Re: [Xen-devel] Make ready for 4.6! Xen Project Document Day is Wednesday, Sept 30

[Russ Pavlicek]

Remember: Document Day is tomorrow (Wednesday) and we need to make the
Wiki 4.6-compatible.  Join us in #xendocs!

On Thu, Sep 24, 2015 at 3:19 PM, Russ Pavlicek
 wrote:
> Our next Xen Project Document Day is this Wednesday, September 30!
>
> OUR THEME OF THE MONTH: "Ready for 4.6"
>
> This month, we prepare for the release of Xen Project 4.6 early next
> month. We need to make sure that users of the new release can find the
> documentation they need to make it all work. So, this month, we need
> to:
>
> - Add new documentation for new features
> - Modify existing documentation for anything which is changing in the
> newest release, and
> - Deprecate old documentation, letting people know that certain
> information is applies only to older releases
>
> Check out the current documentation, and anything which doesn't map to
> 4.6 (or 4.5, for that matter) commands or best practices will need
> improvement.
>
> More detailed information can be found in the TODO document (below).
> And, as always, feel free to add any other documentation which you
> believe to be necessary.
>
> All the information you need to participate in Document Day is here:
>
> http://wiki.xenproject.org/wiki/Xen_Document_Days
>
> Also take a look at the current TODO list to see other items which
> need attention:
>
> http://wiki.xenproject.org/wiki/Xen_Document_Days/TODO
>
> Please think about how you can help out.  If you haven't requested
> to be made a Wiki editor, save time and do it now so you are ready to
> go on Document Day.  Just fill out the form below:
>
> http://xenproject.org/component/content/article/100-misc/145-request-to-be-made-a-wiki-editor.html
>
> We hope to see you Wednesday in #xendocs!

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

Re: [Xen-devel] [PATCH OSSTEST 2/6] Debian: Search for kernel in /boot as well as / when making u-boot script

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST 2/6] Debian: Search for kernel in /boot as 
well as / when making u-boot script"):
> The vmlinuz and initrd.img symlinks appear to have moved to /boot when
> installing Jessie on armhf systems compared to Wheezy.

How inconvenient.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [OSSTEST PATCH 1/7] Debian installs: Nobble /etc/network/if-up.d/openssh-server

[Ian Campbell]

On Tue, 2015-09-29 at 14:37 +0100, Ian Jackson wrote:
> (See the comment in the new file for the explanation.)
> 
> This change affects all our Debian installs (both hosts and guests)
> which are done with preseeding, because preseed_base() arranges to
> install overlay/.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [OSSTEST PATCH 2/7] TestSupport::open_unique_stashfile: Provide a RDWR filehandle

[Ian Campbell]

On Tue, 2015-09-29 at 14:37 +0100, Ian Jackson wrote:
> The caller can then rewind and reread it if they feel like.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [PATCH OSSTEST 0/6] Fixes for switching to Jessie as base OS for test hosts

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [PATCH OSSTEST 0/6] Fixes for switching 
to Jessie as base OS for test hosts"):
> On Tue, 2015-09-29 at 10:44 +0100, Ian Campbell wrote:
> >  * Switch to apt-cacher-ng on the cache host, since a bug in Jessie's apt
> >apparently interacts badly with apt-cacher's handling of Ranges in http
> >requests[0]. I've deployed apt-cacher-ng on an alternative port on the
> >production cache and tested that it works with Jessie and Wheezy.
> 
> I was assuming we'd want to switch out apt-cacher for apt-cacher-ng on port
> cache:3142. Instead you suggested just changing the config to use the
> alternative port 3143.

Ah here we are.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [V5 3/4] x86/xsaves: enable xsaves/xrstors for hvm guest

[Jan Beulich]

>>> On 21.09.15 at 13:34,  wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4550,6 +4550,23 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
> unsigned int *ebx,
>  *ebx = _eax + _ebx;
>  }
>  }
> +if ( count == 1 )
> +{
> +if ( cpu_has_xsaves )

Doesn't this also depend on the respective VMX capability (which
of course you shouldn't check directly here)?

> +{
> +*ebx = XSTATE_AREA_MIN_SIZE;
> +if ( v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss )
> +for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ )
> +{
> +if ( !((v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss)
> +   & (1ULL << sub_leaf)) )

Indentation.

> +continue;

Please invert the condition and drop the continue.

> @@ -4781,6 +4804,13 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t 
> msr_content,
> return X86EMUL_EXCEPTION;
>  break;
>  
> +case MSR_IA32_XSS:
> +  /* No XSS features currently supported for guests. */

Hard tab. Also - is the patch of much use then?

> @@ -1238,7 +1239,8 @@ static int construct_vmcs(struct vcpu *v)
>  __vmwrite(HOST_PAT, host_pat);
>  __vmwrite(GUEST_PAT, guest_pat);
>  }
> -
> +if ( cpu_has_vmx_xsaves )
> +__vmwrite(XSS_EXIT_BITMAP, 0);
>  vmx_vmcs_exit(v);

Instead of removing a blank line I'd rather see you add another one
before the call to vmx_vmcs_exit().

> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -14,8 +14,8 @@
>  #include 
>  #include 
>  
> -static bool_t __read_mostly cpu_has_xsaveopt;
> -static bool_t __read_mostly cpu_has_xsavec;
> +bool_t __read_mostly cpu_has_xsaveopt;
> +bool_t __read_mostly cpu_has_xsavec;

Why? (But iirc this will go away anyway once re-based on Andrew's
changes.)

> --- a/xen/include/asm-x86/hvm/vmx/vmcs.h
> +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
> @@ -225,6 +225,7 @@ extern u32 vmx_vmentry_control;
>  #define SECONDARY_EXEC_ENABLE_VMCS_SHADOWING0x4000
>  #define SECONDARY_EXEC_ENABLE_PML   0x0002
>  #define SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS   0x0004
> +#define SECONDARY_EXEC_XSAVES   0x0010
>  extern u32 vmx_secondary_exec_control;
>  
>  #define VMX_EPT_EXEC_ONLY_SUPPORTED 0x0001
> @@ -240,6 +241,8 @@ extern u32 vmx_secondary_exec_control;
>  
>  #define VMX_MISC_VMWRITE_ALL0x2000
>  
> +#define VMX_XSS_EXIT_BITMAP 0

What use is this addition without it having any user? (And without
any user judging whether this is a meaningful definition is also
impossible.)

Jan


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

Re: [Xen-devel] [PATCH 3/4 RFC] x86/p2m: use large pages for MMIO mappings

[Julien Grall]

On 29/09/15 12:44, Jan Beulich wrote:
 On 29.09.15 at 13:33,  wrote:
>> On 15/09/15 08:34, Jan Beulich wrote:
>>> RFC reasons:
>>> - ARM side unimplemented (and hence libxc for now made cope with both
>>>   models), the main issue (besides my inability to test any change
>>>   there) being the many internal uses of map_mmio_regions())
>>
>> map_mmio_regions is used in ARM to map all the device memory in a guest.
>> We expect this function to map everything at once when called during
>> DOM0 build and/or when a guest is created (used to map the interrupt
>> controller).
>>
>> I would rather prefer to avoid introducing specific helpers with
>> slightly different behavior (i.e one is only mapping N page, the other
>> everything).
>>
>> What about extending map_mmio_regions to take a parameter telling if we
>> want to limit the number of mapping in a single invocation?
> 
> Sure an option, albeit something that would be sufficient to be
> done in ARM specific code, albeit the only user using variable
> length is map_range_to_domain(). All the others, using fixed
> lengths up to 32 pages, would implicitly get everything done at
> once as long as the threshold is >= 32.

While this is the case today, we have different patch series coming up
using variable lenght in different place within the ARM code (vGIC,
ACPI...).

It won't be possible to use map_range_to_domain because it's very
specific to build DOM0.

So, I would extend map_mmio_region like that

map_mmio_regions(struct domain *d,
 unsigned long start_gfn,
 unsigned long nr,
 unsigned long mfn,
 unsigned long limit);

The limit parameter would be 0 if there is no limit otherwise the
maximum of iteration.

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v2] x86/PoD: shorten certain operations on higher order ranges

[Andrew Cooper]

On 28/09/15 15:30, Jan Beulich wrote:
> Now that p2m->get_entry() always returns a valid order, utilize this
> to accelerate some of the operations in PoD code. (There are two uses
> of p2m->get_entry() left which don't easily lend themselves to this
> optimization.)
>
> Also adjust a few types as needed and remove stale comments from
> p2m_pod_cache_add() (to avoid duplicating them yet another time).
>
> Signed-off-by: Jan Beulich 
> ---
> v2: Add a code comment in p2m_pod_decrease_reservation().
>
> --- a/xen/arch/x86/mm/p2m-pod.c
> +++ b/xen/arch/x86/mm/p2m-pod.c
> @@ -119,20 +119,23 @@ p2m_pod_cache_add(struct p2m_domain *p2m
>  
>  unlock_page_alloc(p2m);
>  
> -/* Then add the first one to the appropriate populate-on-demand list */
> -switch(order)
> +/* Then add to the appropriate populate-on-demand list. */
> +switch ( order )
>  {
> +case PAGE_ORDER_1G:
> +for ( i = 0; i < (1UL << PAGE_ORDER_1G); i += 1UL << PAGE_ORDER_2M )
> +page_list_add_tail(page + i, >pod.super);
> +break;
>  case PAGE_ORDER_2M:
> -page_list_add_tail(page, >pod.super); /* lock: page_alloc */
> -p2m->pod.count += 1 << order;
> +page_list_add_tail(page, >pod.super);
>  break;
>  case PAGE_ORDER_4K:
> -page_list_add_tail(page, >pod.single); /* lock: page_alloc */
> -p2m->pod.count += 1;
> +page_list_add_tail(page, >pod.single);
>  break;
>  default:
>  BUG();
>  }
> +p2m->pod.count += 1 << order;

1UL

Otherwise, Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH 3/4 RFC] x86/p2m: use large pages for MMIO mappings

[Jan Beulich]

>>> On 29.09.15 at 14:52,  wrote:
> On 29/09/15 13:46, Jan Beulich wrote:
>> Again, make map_mmio_regions() a wrapper around an ARM-specific
>> function with the extra argument. No need to alter common or x86
>> code.
> 
> TBH, extending the mapp_mmio_region is the best solution.
> 
> The name map_mmio_region is very generic and there is no reason we can't
> use it in the hypervisor. Adding yet another wrapper will confuse people
> and it will be hard for both the reviewer and the developer to know
> which one to use.

I disagree - the function was originally meant to exclusively support
the respective domctl. The fact that ARM gained (many) more uses
should not impact common code or x86.

Jan


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

Re: [Xen-devel] [PATCH v1 6/8] xen/arm: vgic: Optimize the way to store the target vCPU in the rank

[Ian Campbell]

On Fri, 2015-09-25 at 15:51 +0100, Julien Grall wrote:
> Xen is currently directly storing the value of register GICD_ITARGETSR
> (for GICv2) and GICD_IROUTER (for GICv3) in the rank. This makes the
> emulation of the registers access very simple but makes the code to get
> the target vCPU for a given IRQ more complex.
> 
> While the target vCPU of an IRQ is retrieved everytime an IRQ is
> injected to the guest, the access to the register occurs less often.
> 
> So the data structure should be optimized for the most common case
> rather than the inverse.
> 
> This patch introduce the usage of an array to store the target vCPU for
> every interrupt in the rank. This will make the code to get the target
> very quick. The emulation code will now have to generate the
> GICD_ITARGETSR
> and GICD_IROUTER register for read access and split it to store in a
> convenient way.
> 
> Note that with these changes, any read to those registers will list only
> the target vCPU used by Xen. This is fine because the GIC spec doesn't
> require to return exactly the value written and it can be seen as if we
> decide to implement the register read-only.

I think this is probably OK, but skirting round what the spec actually says
a fair bit.

> Finally take the opportunity to fix coding style in section we are
> modifying.
> 
> Signed-off-by: Julien Grall 
> 
> ---
> The real reason is I'd like to drop vgic_byte_* helpers in favor of
> more
> generic access helpers. Although it would make the code to retrieve
> the
> priority more complex. So reworking the code to get the target vCPU
> was the best solution.
> 
> Changes in v2:
> - Patch added
> ---
>  xen/arch/arm/vgic-v2.c | 172 ++-
> --
>  xen/arch/arm/vgic-v3.c | 121 +--
>  xen/arch/arm/vgic.c|  28 ++--
>  xen/include/asm-arm/vgic.h |  13 ++--
>  4 files changed, 197 insertions(+), 137 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index 23d1982..b6db64f 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -50,6 +50,90 @@ void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase,
> paddr_t vbase)
>  vgic_v2_hw.vbase = vbase;
>  }
>  
> +#define NR_TARGET_PER_REG 4U
> +#define NR_BIT_PER_TARGET 8U
> +
> +/*
> + * Generate the associated ITARGETSR register based on the offset from
> + * ITARGETSR0. Only one vCPU will be listed for a given IRQ.
> + *
> + * Note the offset will be round down to match a real HW register.

"rounded down".

Although I'm not 100% sure what that comment is trying to say. From the
code I think you mean aligned to the appropriate 4 byte boundary?

> + */
> +static uint32_t vgic_generate_itargetsr(struct vgic_irq_rank *rank,

For all these why not s/generate/read/?

Or since you use "store" for the other half "fetch".

("generate" is a bit of an implementation detail, the caller doesn't care
where or how the result is achieved)

> +unsigned int offset)
> +{
> +uint32_t reg = 0;
> +unsigned int i;
> +
> +ASSERT(spin_is_locked(>lock));
> +
> +offset &= INTERRUPT_RANK_MASK;
> +offset &= ~(NR_TARGET_PER_REG - 1);
> +
> +for ( i = 0; i < NR_TARGET_PER_REG; i++, offset++ )
> +reg |= (1 << rank->vcpu[offset]) << (i * NR_BIT_PER_TARGET);
> +
> +return reg;
> +}
> +
> +/*
> + * Store a ITARGETSR register in a convenient way and migrate the IRQ
> + * if necessary.
> + * Note the offset will be round down to match a real HW register.

As above.

> + */
> +static void vgic_store_itargetsr(struct domain *d, struct vgic_irq_rank
> *rank,
> + unsigned int offset, uint32_t
> itargetsr)
> +{
> +unsigned int i, rank_idx;
> +
> +ASSERT(spin_is_locked(>lock));
> +
> +rank_idx = offset / NR_INTERRUPT_PER_RANK;
> +
> +offset &= INTERRUPT_RANK_MASK;
> +offset &= ~(NR_TARGET_PER_REG - 1);
> +
> +for ( i = 0; i < NR_TARGET_PER_REG; i++, offset++ )
> +{
> +unsigned int new_target, old_target;
> +
> +/*
> + * SPI are using the 1-N model (see 1.4.3 in ARM IHI 0048B).
> + * While the interrupt could be set pending to all the vCPUs in
> + * target list, it's not guarantee by the spec.
> + * For simplicity, always route the IRQ to the first interrupt
> + * in the target list
> + */

I don't see anything which is handling the PPI and SGI special case (which
is that they are r/o).

Likewise on read they are supposed to always reflect the value of the CPU
doing the read.

Are both of those handled elsewhere in some way I'm missing?

[...]
> + * Note the offset will be round down to match a real HW register

As earlier.

> + * Note the offset will be round down to match a real HW register.

Again.

> + */
> +static void vgic_store_irouter(struct domain *d, struct vgic_irq_rank
> *rank,
> +

Re: [Xen-devel] [PATCH 3/4 RFC] x86/p2m: use large pages for MMIO mappings

[Jan Beulich]

>>> On 29.09.15 at 15:06,  wrote:
> On 29/09/15 14:00, Jan Beulich wrote:
> On 29.09.15 at 14:52,  wrote:
>>> On 29/09/15 13:46, Jan Beulich wrote:
 Again, make map_mmio_regions() a wrapper around an ARM-specific
 function with the extra argument. No need to alter common or x86
 code.
>>>
>>> TBH, extending the mapp_mmio_region is the best solution.
>>>
>>> The name map_mmio_region is very generic and there is no reason we can't
>>> use it in the hypervisor. Adding yet another wrapper will confuse people
>>> and it will be hard for both the reviewer and the developer to know
>>> which one to use.
>> 
>> I disagree - the function was originally meant to exclusively support
>> the respective domctl. The fact that ARM gained (many) more uses
>> should not impact common code or x86.
> 
> The expectation you described is neither documented nor explicit from
> the name...

>From history only, agreed.

> As the interface is not set in stone, we could decide to extend the
> usage of the function to make a coherent interface and not adding new
> wrapper because we don't want to touch x86...

One more thing - what meaning would you expect the new parameter
to carry? Number of frames mapped? Number of iterations done? In
the former case, the goal aimed at here won't be achievable. In the
latter case, would you mean to pass -1UL for it for the ARM callers
wanting the operation to run to completion?

Jan


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

Re: [Xen-devel] [PATCH v6 24/29] xen/x86: allow HVM guests to use hypercalls to bring up vCPUs

[Roger Pau Monné]

El 29/09/15 a les 12.48, Jan Beulich ha escrit:
 On 29.09.15 at 12:37,  wrote:
>> On 29/09/15 11:33, Jan Beulich wrote:
>> On 29.09.15 at 12:25,  wrote:
 On 29/09/15 11:07, Jan Beulich wrote:
 On 29.09.15 at 12:00,  wrote:
>> Therefore, we are back to the question of whether to provide all segment
>> registers, or specify a flat layout without specific selector values.  I
>> would prefer the former to the latter.
> If we don't go the CS+SS only route, then yes, I'd too prefer
> completing the set (I would probably agree with not adding FS
> and GS, and even recommend against it in the 64-bit variant,
> but I do insist on ES in that case).
 I would still err on the CS/SS/DS/ES side given a straight choice.  It
 offers more flexibility for rarer usecases.
>>> Okay, all four of them then for 32-bit, and just CS and SS for 64-bit?
>>
>> Is SS needed for 64bit?  It is expected to be NUL just like DS and ES.
> 
> Indeed, we should be able to get away without it. And for CS all
> we'd need would be a selector.

Ok thanks, so we seem to have a consensus. Before posting a new 
revision, does the following vcpu_hvm_context look fine to both of you:

struct vcpu_hvm_x86_32 {
uint32_t eax;
uint32_t ecx;
uint32_t edx;
uint32_t ebx;
uint32_t esp;
uint32_t ebp;
uint32_t esi;
uint32_t edi;
uint32_t eip;
uint32_t eflags;

uint32_t cr0;
uint32_t cr3;
uint32_t cr4;

/*
 * EFER should only be used to set the NXE bit (if required)
 * when starting a vCPU in 32bit mode with paging enabled.
 */
uint64_t efer;

uint32_t cs_base;
uint32_t ds_base;
uint32_t ss_base;
uint32_t es_base;
uint32_t tr_base;
uint32_t cs_limit;
uint32_t ds_limit;
uint32_t ss_limit;
uint32_t es_limit;
uint32_t tr_limit;
uint16_t cs_ar;
uint16_t ds_ar;
uint16_t ss_ar;
uint16_t es_ar;
uint16_t tr_ar;
};

struct vcpu_hvm_x86_64 {
uint64_t rax;
uint64_t rcx;
uint64_t rdx;
uint64_t rbx;
uint64_t rsp;
uint64_t rbp;
uint64_t rsi;
uint64_t rdi;
uint64_t rip;
uint64_t rflags;

uint64_t cr0;
uint64_t cr3;
uint64_t cr4;
uint64_t efer;

/*
 * Setting the CS attributes field is allowed in order to
 * start in compatibility mode.
 */
uint16_t cs_ar;
};

struct vcpu_hvm_context {
#define VCPU_HVM_MODE_32B 0  /* 32bit fields of the structure will be used. */
#define VCPU_HVM_MODE_64B 1  /* 64bit fields of the structure will be used. */
uint32_t mode;

/* CPU registers. */
union {
struct vcpu_hvm_x86_32 x86_32;
struct vcpu_hvm_x86_64 x86_64;
} cpu_regs;
};

Thanks, Roger.


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

Re: [Xen-devel] [OSSTEST PATCH 3/7] TestSupport: Honour $stdin fh argument to cmd, tcmd and tcmdex

[Ian Campbell]

On Tue, 2015-09-29 at 14:37 +0100, Ian Jackson wrote:
> These are internal functions, so the change is entirely within
> TestSupport.  All the call sites are adjusted to pass undef so there
> is no functional change.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [OSSTEST PATCH 4/7] TestSupport: Provide target_cmd_inputfh_root

[Ian Campbell]

On Tue, 2015-09-29 at 14:37 +0100, Ian Jackson wrote:
> No caller yet.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 

> ---
>  Osstest/TestSupport.pm |6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
> index 91829a0..3fc8e15 100644
> --- a/Osstest/TestSupport.pm
> +++ b/Osstest/TestSupport.pm
> @@ -50,6 +50,7 @@ BEGIN {
>  
>target_cmd_root target_cmd target_cmd_build
>target_cmd_output_root target_cmd_output
> +  target_cmd_inputfh_root
>target_getfile target_getfile_root
>target_putfile target_putfile_root
>target_putfilecontents_stash
> @@ -655,6 +656,11 @@ sub tcmdout {
>  sub target_cmd_output ($$;$) { tcmdout('osstest',@_); }
>  sub target_cmd_output_root ($$;$) { tcmdout('root',@_); }
>  
> +sub target_cmd_inputfh_root ($$$;$$) {
> +my ($tho,$stdinfh,$tcmd,@rest) = @_;
> +tcmd($stdinfh,undef,'root',$tho,$tcmd,@rest);
> +}
> +
>  sub poll_loop ($$$&) {
>  my ($maxwait, $interval, $what, $code) = @_;
>  # $code should return undef when all is well

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

Re: [Xen-devel] [PATCH 1/2] VT-d: use proper error codes in iommu_enable_x2apic_IR()

[Andrew Cooper]

On 29/09/15 12:44, Jan Beulich wrote:
> ... allowing to suppress a confusing messeage combination: When
> ACPI_DMAR_X2APIC_OPT_OUT is set, so far we first logged a message
> that IR could not be enabled (hence not using x2APIC), followed by
> one indicating successful initialization of IR (if no other problems
> prevented that).
>
> Also adjust the return type of iommu_supports_eim() and fix some
> broken indentation in the function.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH 13/13] tools/libxl: Add 'vtsc_khz' option to set guest TSC rate

[Haozhong Zhang]

On Tue, Sep 29, 2015 at 11:07:17AM +0100, Ian Campbell wrote:
> On Tue, 2015-09-29 at 08:40 +0800, Haozhong Zhang wrote:
> > > > @@ -1462,6 +1462,28 @@ static void parse_config_data(const char 
> > > > *config_source,
> > > >  }
> > > >  }
> > > >  
> > > > +/* "vtsc_khz" option works only if "tsc_mode" option is
> > > > + * "default". In this case, if "vtsc_khz" option is set to 0, we
> > > > + * will reset it to the host TSC rate. In all other cases, we just
> > > > + * ignore any given value and always set it to 0.
> > > > + */
> > > > +if (!xlu_cfg_get_long(config, "vtsc_khz", , 0))
> > > > +b_info->vtsc_khz = l;
> > > > +if (b_info->tsc_mode == LIBXL_TSC_MODE_DEFAULT) {
> > > > +if (b_info->vtsc_khz == 0) {
> > > > +libxl_physinfo physinfo;
> > > > +if (!libxl_get_physinfo(ctx, ))
> > > > +b_info->vtsc_khz = physinfo.cpu_khz;
> > > > +else
> > > > +fprintf(stderr, "WARNING: cannot get host TSC 
> > > > rate.\n");
> > > > +}
> > > 
> > > And this hunk (the decision making bit) should be in libxl, not xl.
> > > 
> > > Consider there are other toolstack that uses libxl, say libvirt.
> > > 
> > 
> > Good to know this.
> > 
> > I'm going to move it to libxl__arch_domain_create() where
> > b_info->vtsc_khz is used.
> 
> libxl__domain_build_info_setdefault would be more usual, I think.
>

Yes, this is a better place.

> Rather than calling get_physinfo in order to give vtsc_khz a specific value
> instead of zero can we not leave it as zero and just not call
>  xc_domain_set_tsc_info() in that case and let the hypervisor default to
> using the host rate?
>

Alternatively, I can leave vtsc_khz zero if it's not set in xl.cfg
and, if xc_domain_set_tsc_info() passes a zero vtsc_khz to hypervisor,
the latter will just use the host tsc rate (which was the original logic),

> Then the check in libxl just becomes "is vtsc_khz non-zero and is tsc_mode
> not DEFAULT".
>

... and use this check in libxl__domain_build_info_setdefault().

> Don't forget to switch from fprintf to the proper log macros.
>

Of course. Thanks for reminding!

- Haozhong

> Ian.

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

Re: [Xen-devel] Xen4.7 unable to get domain type for domid

[Andrew Cooper]

On 29/09/15 13:54, Wei Liu wrote:
> On Mon, Sep 28, 2015 at 10:24:15PM -0400, soapcn wrote:
> [...]
>> domainbuilder: detail: shared_info_x86_64: called
>> domainbuilder: detail: vcpu_x86_64: called
>> domainbuilder: detail: vcpu_x86_64: cr3: pfn 0x6c0d mfn 0x12c40d
>> domainbuilder: detail: launch_vm: called, ctxt=0x7f2c9ae6d004
>> domainbuilder: detail: xc_dom_release: called
>> libxl: error: libxl_dom.c:37:libxl__domain_type: unable to get domain type
>> for domid=5
>> xl: unable to exec console client: No such file or directory
>> libxl: error: libxl_exec.c:118:libxl_report_child_exitstatus: console child
>> [3755] exited with error status 1
>>
> Couldn't think of a reason why this would fail. Is this a freshly
> installed Xen? If not, is the previous installation purged?

It possibly means the domain has crashed very early after start.  Does
starting the domain paused (xl create -p) cause it to be constructed
correctly?

~Andrew

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

Re: [Xen-devel] [PATCH 3/4 RFC] x86/p2m: use large pages for MMIO mappings

[Julien Grall]

On 29/09/15 14:00, Jan Beulich wrote:
 On 29.09.15 at 14:52,  wrote:
>> On 29/09/15 13:46, Jan Beulich wrote:
>>> Again, make map_mmio_regions() a wrapper around an ARM-specific
>>> function with the extra argument. No need to alter common or x86
>>> code.
>>
>> TBH, extending the mapp_mmio_region is the best solution.
>>
>> The name map_mmio_region is very generic and there is no reason we can't
>> use it in the hypervisor. Adding yet another wrapper will confuse people
>> and it will be hard for both the reviewer and the developer to know
>> which one to use.
> 
> I disagree - the function was originally meant to exclusively support
> the respective domctl. The fact that ARM gained (many) more uses
> should not impact common code or x86.

The expectation you described is neither documented nor explicit from
the name...

As the interface is not set in stone, we could decide to extend the
usage of the function to make a coherent interface and not adding new
wrapper because we don't want to touch x86...

Anyway, I'm not a maintainer so I will let Ian and Stefano decide what's
best.

Regards,

-- 
Julien Grall

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

[Xen-devel] MSI X99A SLI Krait Edition Motherboard Supports Intel VT-d

[Teo En Ming]

Dear Xen Developers,

I can confirm that the UEFI BIOS in MSI X99A SLI Krait Edition
Motherboard (Intel X99 Express Chipset, LGA2011-V3 Socket) supports
Intel Virtualization Technology for Directed I/O (VT-d).

In addition, Intel 5th Generation Core i7 5820K Processor (15MB Cache,
3.3 GHz, 6 cores, 12 threads) supports Intel VT-d.

Intel VT-d is extremely useful for Xen VGA Passthrough.

Yours sincerely,

Subtle Denial of Medical Treatment by the Singapore Government for Mr.
Teo En Ming (Zhang Enming)
Link: 
https://www.scribd.com/doc/258700156/Subtle-Denial-of-Medical-Treatment-by-the-Singapore-Government-for-Mr-Teo-En-Ming-Zhang-Enming

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

Re: [Xen-devel] [PATCH v1 7/8] xen/arm: vgic: Introduce helpers to read/write/clear/set vGIC register ...

[Julien Grall]

On 29/09/15 14:23, Ian Campbell wrote:
> On Fri, 2015-09-25 at 15:51 +0100, Julien Grall wrote:
>> and use them in the vGIC emulation.
>>
>> The GIC registers may support different access sizes. Rather than open
>> coding the access for every registers, provide a set of helpers to access
>> them.
>>
>> The caller will have to call vgic_regN_* where N is the size of the
>> emulated registers.
> 
> These helpers end up as e.g. vgic_regN_read/write, but they don't really
> read or write anything, they just extract the required bits into a register
> or update the bits into a variable.
> 
> Can we think of a better name? encode/decode?

Encode/decode would be the best. I will replace read by decode and write
by encode.

> 
> This might be more apparent (maybe without the need to rename) if the
> helpers took info->dabt.size instead of info.

We also need info to get the MMIO adddress. I would prefer to keep info
and avoid having another parameter here.

>> [...]
>> +static inline void vgic_reg_clearbit(unsigned long *reg, register_t bits,
> 
> Please call these ones clearbits/setbits to avoid giving the impression
> that they take a bit offset and just set/clear that one.

Will do.


-- 
Julien Grall

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

Re: [Xen-devel] [PATCH 13/13] tools/libxl: Add 'vtsc_khz' option to set guest TSC rate

[Haozhong Zhang]

On Tue, Sep 29, 2015 at 11:28:38AM +0100, Ian Campbell wrote:
> On Tue, 2015-09-29 at 11:24 +0100, Andrew Cooper wrote:
> > On 29/09/15 11:13, Haozhong Zhang wrote:
> > > On Tue, Sep 29, 2015 at 11:04:14AM +0100, Ian Campbell wrote:
> > > > On Mon, 2015-09-28 at 15:13 +0800, Haozhong Zhang wrote:
> > > > > This patch adds an option 'vtsc_khz' to allow users to set vcpu's
> > > > > TSC
> > > > > rate in KHz. In the case that tsc_mode = 'default', the default
> > > > > value of
> > > > > 'vtsc_khz' option is the host TSC rate which is used when
> > > > > 'vtsc_khz'
> > > > > option is set to 0 or does not appear in the configuration. In all
> > > > > other
> > > > > cases of tsc_mode, 'vtsc_khz' option is just ignored.
> > > > > 
> > > > > Another purpose of adding this option is to keep vcpu's TSC rate
> > > > > across
> > > > > guest reboot. In existing code, a new domain is created from the
> > > > > configuration of the previous domain which was just rebooted.
> > > > > vcpu's TSC
> > > > > rate is not stored in the configuration and the host TSC rate is
> > > > > the
> > > > > used as vcpu's TSC rate. This works fine unless the previous domain
> > > > > was
> > > > > migrated from another host machine with a different host TSC rate
> > > > > than
> > > > > the current one.
> > > > I understand why this is necessary over a migration, but why is it
> > > > important to be able to retain the TSC rate across a reboot? What is
> > > > the
> > > > usecase there?
> > > > 
> > > No usecase so far. Is 'making a virtual machine more like a physical
> > > machine' reasonable here? (I suppose a physical machine retains TSC
> > > rate across reboot)
> > 
> > There are situations such as altering firmware settings which can cause
> > the TSC rate to differ across reboot.  I don't see any reason to try and
> > maintain it across VM reboots.
> 
> Right. If it happens to come for free as a side effect of making it work
> for migration then fine.
> 
> But it seems to me that tsc rate could/should be in the hypervisors save
> blob and require no interaction with the toolstack once it is latched when
> the domain is built.
> 
> Ian.
>

Seemingly I don't need vtsc_khz at all if retaining tsc rate across
reboot is unnecessary (or even wrong). The migration of tsc rate is
already done through write_tsc_info() and handle_tsc_info() w/o this
patch. I'll check if "xl save/restore" also go through this path. If
so, I think vtsc_khz can be removed.

- Haozhong

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

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

Re: [Xen-devel] [PATCH 3/3] xen/arm: io: Shorten the name of the fields and clean up

[Ian Campbell]

On Mon, 2015-09-28 at 21:31 +0100, Julien Grall wrote:
> The field names in the IO emulation are really long and use repeatedly
> the term handler which make some line cumbersome to read:
> 
> mmio_handler->mmio_handler_ops->write_handler
> 
> Also take the opportunity to do some clean up:
> - Avoid "handler" vs "handle" in register_mmio_handler
> - Use a local variable to initialize handler in
> register_mmio_handler
> - Add a comment explaining the dsb(ish) in register_mmio_handler
> - Rename the structure io_handler into vmmio because the io_handler
> is in fine handling multiple handlers and the name a the fields was
> io_handlers. Also rename the field io_handlers to vmmmio

Stray extra "m" in vmmmio.

> - Rename the field mmio_handler_ops to ops because we are in the
> structure mmio_handler to not need to repeat it
> - Rename the field mmio_handlers to handlers because we are in the
> vmmio structure
> - Make it clear that register_mmio_ops is taking an ops and not an
> handle
> - Clean up local variable to helpe to understand the code

Stray "e" in the last line.
> 
> Signed-off-by: Julien Grall 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [PATCH v7 2/2] xen/arm: support gzip compressed kernels

[Ian Campbell]

On Wed, 2015-09-23 at 19:31 +0100, Stefano Stabellini wrote:
> [/...]
> +
> +/* 
> + * Need to free pages after output_size here because they won't be
> + * freed by discard_initial_modules
> + */
> +i = (output_size + PAGE_SIZE - 1) >> PAGE_SHIFT;

You have access to DIV_ROUND_UP here.

The rest looks good to me.

I'd have preferred to avoid exporting dt_unreserved_regions, but nevermind.

Ian.


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

Re: [Xen-devel] [PATCH 2/3] xen/arm: vgic-v3: Correctly retrieve the vCPU associated to a re-distributor

[Shameerali Kolothum Thodi]

> -Original Message-
> From: Julien Grall [mailto:julien.gr...@citrix.com]
> Sent: 28 September 2015 21:32
> To: xen-de...@lists.xenproject.org
> Cc: ian.campb...@citrix.com; stefano.stabell...@eu.citrix.com; Julien
> Grall; Shameerali Kolothum Thodi; Wei Liu
> Subject: [PATCH 2/3] xen/arm: vgic-v3: Correctly retrieve the vCPU
> associated to a re-distributor
> 
> When the guest is accessing the re-distributor, Xen retrieves the base
> of the re-distributor using a mask based on the stride.
>

 
> with multiple bit set [1]. Although, we don't support such platform
> right now. So I would rather defer this change to Xen 4.6.1 and
> avoid possible downside/bug in this patch.
> 
> Shamaeerali, I've only tested quickly on the foundation model. Can
> you give a try on your platform to check if it fixes DOM0 boot?


I tried and it fixes the issue on our platform :). 
(I applied only the first two patches in the series).

> [1] http://lists.xen.org/archives/html/xen-devel/2015-
> 09/msg03372.html

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

Re: [Xen-devel] [PATCH 2/2] VT-d: section placement and type adjustments

[Andrew Cooper]

On 29/09/15 12:45, Jan Beulich wrote:
> With x2APIC requiring iommu_supports_eim() to return true, we can
> adjust a few conditonals such that both it and
> platform_supports_x2apic() can be marked __init. For the latter as
> well as for platform_supports_intremap() also change the return types
> to bool_t.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH] x86: detect CMOS aliasing on ports other then 0x70/0x71

[Jan Beulich]

>>> On 29.09.15 at 15:25,  wrote:
> On 25/09/15 12:00, Jan Beulich wrote:
> On 25.09.15 at 11:55,  wrote:
>>> It is safe for the SMM handler to use CMOS if it returns the index 
>>> register back to how it found it.  Furthermore, I am willing to bet that 
>>> there real SMM handlers out there which do do this.
>> So what options do you see then here? Don't do anything (drop the
>> patch) seems the only one I can see not getting in conflict with SMI.
>> Yet using the patch as is would not make anything less safe, it would
>> just have the potential of not always making things as safe as they
>> could be.
> 
> Given some orthogonal interaction with cmos-rtc-probe recently, I am
> fairly sure that we can remove all real RTC interaction from Xen.
> 
> Dom0 is required to provide fine-grained time via XENPF_settime{32,64}
> which gives an absolute time to seed the other domain wallclocks with. 

Is it? If so, I suppose we should enforce this in some way, e.g. by
failing domain creation until done. Right now I guess we expect that
to happen, but don't really require it.

> The RTC was assumed always to be in UTC, which allows full date and time
> information to be derived.
> 
> It would certainly simplify things to leave all of the RTC in dom0's hand.

Yes, it would.

Jan


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

Re: [Xen-devel] [PATCH OSSTEST 5/6] TestSupport: Remove now unused target_guest_lv_name.

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST 5/6] TestSupport: Remove now unused 
target_guest_lv_name."):
> Signed-off-by: Ian Campbell 

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH OSSTEST 6/6] Switch to Debian 8.0 (jessie) as OS for test hosts

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST 6/6] Switch to Debian 8.0 (jessie) as OS 
for test hosts"):
> mg-debian-installer-update-all has been run on the production instance
> and TftpDiVersion is also updated to match.
> 
> The resulting binaries have also been copied to the Cambridge
> instance, so update Cambridge config too.

Jolly good.

Subject to the prerequisites,

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH] Add missing license and copyright statements to public interface headers.

[Andrew Cooper]

On 29/09/15 13:03, Mike Belopuhov wrote:
> On Mon, Sep 28, 2015 at 11:24 +0200, Mike Belopuhov wrote:
>> On Fri, Sep 25, 2015 at 01:12 -0600, Jan Beulich wrote:
>> On 22.09.15 at 16:02,  wrote:
 --- xen/include/public/arch-x86/pmu.h
 +++ xen/include/public/arch-x86/pmu.h
>>> I fixed this up for you this time, but in the future please make sure
>>> you send patches in conventional format (applicable with patch's -p1
>>> or whatever tool's equivalent).
>>>
>>> Thanks, Jan
>>>
>> Thanks, Jan.  I'll keep that in mind.
> Hi Guys,
>
> Are we waiting for others to Ack the diff or is there anything
> I should do?
>
> With best regards,
> Mike

This has been committed (staging and staging-4.6)

http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=8d6ff9ef259e296e6aee32ad8840cc5081280e0d

In both cases, they have not yet passed the automatic test gate, which
is why they have not appeared in master and stable-4.6

~Andrew

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

Re: [Xen-devel] [PATCH] x86/EPT: defer enabling of A/D maintenance until PML get enabled

[Andrew Cooper]

On 28/09/15 15:42, Jan Beulich wrote:
> There's no point in enabling the extra feature for every domain when
> we're not meaning to use it (yet). Just setting the flag should be
> sufficient - the domain is required to be paused for PML enabling
> anyway, i.e. hardware will pick up the new setting the next time
> each vCPU of the guest gets scheduled.
>
> Signed-off-by: Jan Beulich 
> Cc: Kai Huang 

Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH v2] x86/PoD: shorten certain operations on higher order ranges

[George Dunlap]

On 28/09/15 15:30, Jan Beulich wrote:
> Now that p2m->get_entry() always returns a valid order, utilize this
> to accelerate some of the operations in PoD code. (There are two uses
> of p2m->get_entry() left which don't easily lend themselves to this
> optimization.)
> 
> Also adjust a few types as needed and remove stale comments from
> p2m_pod_cache_add() (to avoid duplicating them yet another time).
> 
> Signed-off-by: Jan Beulich 
> ---
> v2: Add a code comment in p2m_pod_decrease_reservation().
> 
> --- a/xen/arch/x86/mm/p2m-pod.c
> +++ b/xen/arch/x86/mm/p2m-pod.c
> @@ -119,20 +119,23 @@ p2m_pod_cache_add(struct p2m_domain *p2m
>  
>  unlock_page_alloc(p2m);
>  
> -/* Then add the first one to the appropriate populate-on-demand list */
> -switch(order)
> +/* Then add to the appropriate populate-on-demand list. */
> +switch ( order )
>  {
> +case PAGE_ORDER_1G:
> +for ( i = 0; i < (1UL << PAGE_ORDER_1G); i += 1UL << PAGE_ORDER_2M )
> +page_list_add_tail(page + i, >pod.super);
> +break;
>  case PAGE_ORDER_2M:
> -page_list_add_tail(page, >pod.super); /* lock: page_alloc */
> -p2m->pod.count += 1 << order;
> +page_list_add_tail(page, >pod.super);
>  break;
>  case PAGE_ORDER_4K:
> -page_list_add_tail(page, >pod.single); /* lock: page_alloc */
> -p2m->pod.count += 1;
> +page_list_add_tail(page, >pod.single);
>  break;
>  default:
>  BUG();
>  }
> +p2m->pod.count += 1 << order;
>  
>  return 0;
>  }
> @@ -502,11 +505,10 @@ p2m_pod_decrease_reservation(struct doma
>   unsigned int order)
>  {
>  int ret=0;
> -int i;
> +unsigned long i, n;
>  struct p2m_domain *p2m = p2m_get_hostp2m(d);
> -
> -int steal_for_cache;
> -int pod, nonpod, ram;
> +bool_t steal_for_cache;
> +long pod, nonpod, ram;
>  
>  gfn_lock(p2m, gpfn, order);
>  pod_lock(p2m);
> @@ -525,21 +527,21 @@ recount:
>  /* Figure out if we need to steal some freed memory for our cache */
>  steal_for_cache =  ( p2m->pod.entry_count > p2m->pod.count );
>  
> -/* FIXME: Add contiguous; query for PSE entries? */
> -for ( i=0; i<(1< +for ( i = 0; i < (1UL << order); i += n )
>  {
>  p2m_access_t a;
>  p2m_type_t t;
> +unsigned int cur_order;
>  
> -(void)p2m->get_entry(p2m, gpfn + i, , , 0, NULL, NULL);
> -
> +p2m->get_entry(p2m, gpfn + i, , , 0, _order, NULL);
> +n = 1UL << min(order, cur_order);
>  if ( t == p2m_populate_on_demand )
> -pod++;
> +pod += n;
>  else
>  {
> -nonpod++;
> +nonpod += n;
>  if ( p2m_is_ram(t) )
> -ram++;
> +ram += n;
>  }
>  }
>  
> @@ -574,41 +576,53 @@ recount:
>   * + There are PoD entries to handle, or
>   * + There is ram left, and we want to steal it
>   */
> -for ( i=0;
> -  i<(1<0 || (steal_for_cache && ram > 0));
> -  i++)
> +for ( i = 0;
> +  i < (1UL << order) && (pod > 0 || (steal_for_cache && ram > 0));
> +  i += n )
>  {
>  mfn_t mfn;
>  p2m_type_t t;
>  p2m_access_t a;
> +unsigned int cur_order;
>  
> -mfn = p2m->get_entry(p2m, gpfn + i, , , 0, NULL, NULL);
> +mfn = p2m->get_entry(p2m, gpfn + i, , , 0, _order, NULL);
> +if ( order < cur_order )
> +cur_order = order;
> +n = 1UL << cur_order;
>  if ( t == p2m_populate_on_demand )
>  {
> -p2m_set_entry(p2m, gpfn + i, _mfn(INVALID_MFN), 0, p2m_invalid,
> -  p2m->default_access);
> -p2m->pod.entry_count--;
> +p2m_set_entry(p2m, gpfn + i, _mfn(INVALID_MFN), cur_order,
> +  p2m_invalid, p2m->default_access);
> +p2m->pod.entry_count -= n;
>  BUG_ON(p2m->pod.entry_count < 0);
> -pod--;
> +pod -= n;
>  }
>  else if ( steal_for_cache && p2m_is_ram(t) )
>  {
> +/*
> + * If we need less than 1 << cur_order, we may end up stealing
> + * more memory here than we actually need. This will be rectified
> + * below, however; and stealing too much and then freeing what we
> + * need may allow us to free smaller pages from the cache, and
> + * avoid breaking up superpages.
> + */
>  struct page_info *page;
> +unsigned int j;
>  
>  ASSERT(mfn_valid(mfn));
>  
>  page = mfn_to_page(mfn);
>  
> -p2m_set_entry(p2m, gpfn + i, _mfn(INVALID_MFN), 0, p2m_invalid,
> -  p2m->default_access);
> -set_gpfn_from_mfn(mfn_x(mfn), INVALID_M2P_ENTRY);
> -
> -  

Re: [Xen-devel] [PATCH v2] x86/PoD: shorten certain operations on higher order ranges

[Jan Beulich]

>>> On 29.09.15 at 14:20,  wrote:
> On 28/09/15 15:30, Jan Beulich wrote:
>> --- a/xen/arch/x86/mm/p2m-pod.c
>> +++ b/xen/arch/x86/mm/p2m-pod.c
>> @@ -119,20 +119,23 @@ p2m_pod_cache_add(struct p2m_domain *p2m
>>  
>>  unlock_page_alloc(p2m);
>>  
>> -/* Then add the first one to the appropriate populate-on-demand list */
>> -switch(order)
>> +/* Then add to the appropriate populate-on-demand list. */
>> +switch ( order )
>>  {
>> +case PAGE_ORDER_1G:
>> +for ( i = 0; i < (1UL << PAGE_ORDER_1G); i += 1UL << PAGE_ORDER_2M )
>> +page_list_add_tail(page + i, >pod.super);
>> +break;
>>  case PAGE_ORDER_2M:
>> -page_list_add_tail(page, >pod.super); /* lock: page_alloc */
>> -p2m->pod.count += 1 << order;
>> +page_list_add_tail(page, >pod.super);
>>  break;
>>  case PAGE_ORDER_4K:
>> -page_list_add_tail(page, >pod.single); /* lock: page_alloc */
>> -p2m->pod.count += 1;
>> +page_list_add_tail(page, >pod.single);
>>  break;
>>  default:
>>  BUG();
>>  }
>> +p2m->pod.count += 1 << order;
> 
> 1UL

Not really - the field is a "long" one, so at best 1L or 1U. And then
all the valid order values are visible right above, for none of them
it makes a difference, and there are ample similar uses scattered
around the file (yes, bad examples are no excuse, but in cases
where the suffix doesn't really matter I think it is better to omit it).

> Otherwise, Reviewed-by: Andrew Cooper 

Let me know regrading this one,
Jan


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

Re: [Xen-devel] [PATCH v1 3/8] xen/arm: Support sign-extension for every read access

[Julien Grall]

On 29/09/15 14:13, Ian Campbell wrote:
> On Tue, 2015-09-29 at 12:13 +0100, Julien Grall wrote:
>>> Or is it, couldn't we be updating a byte in the middle of the word?
>>
>> *r is a register, the byte/half-word/word... are always living in the
>> lowest bit of the register.
> 
> So just to check:
> 
> ldrs* do support e.g odd offsets, but given a 32 bit MMIO region which
> reads 0xAABBCCDD then when reading from offset 1 at this point *r contains
> 0x00CC already (and not 0xC00) and sign extending the lowest byte
> is therefore correct.
> 
> Right?

Correct. For more details see the pseudo-code to implement the different
ldrs* instruction (for instance A8.8.86 in ARM DDI 0406C.b).

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH 12/13] vmx: Add a call-back to apply TSC scaling ratio to hardware

[Haozhong Zhang]

On Tue, Sep 29, 2015 at 11:25:56AM +0100, Andrew Cooper wrote:
> On 29/09/15 11:02, Haozhong Zhang wrote:
> > On Tue, Sep 29, 2015 at 10:33:14AM +0100, Andrew Cooper wrote:
> >> On 29/09/15 02:07, Haozhong Zhang wrote:
> >>> On Mon, Sep 28, 2015 at 12:02:08PM -0400, Boris Ostrovsky wrote:
>  On 09/28/2015 03:13 AM, Haozhong Zhang wrote:
> > This patch adds a new call-back setup_tsc_scaling in struct
> > hvm_function_table to apply the TSC scaling ratio to hardware. For VMX,
> > it writes the TSC scaling ratio to VMCS field TSC_MULTIPLIER.
> >
> > Signed-off-by: Haozhong Zhang 
> > ---
> >  xen/arch/x86/hvm/hvm.c| 1 +
> >  xen/arch/x86/hvm/svm/svm.c| 5 +
> >  xen/arch/x86/hvm/vmx/vmx.c| 8 
> >  xen/include/asm-x86/hvm/hvm.h | 3 +++
> >  4 files changed, 17 insertions(+)
> >
> > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> > index 3522d20..2d8a148 100644
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -376,6 +376,7 @@ void hvm_setup_tsc_scaling(struct vcpu *v)
> >  }
> >  v->arch.tsc_scaling_ratio = ratio;
> > +hvm_funcs.setup_tsc_scaling(v);
> >  }
> >  void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
> > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> > index 73bc863..d890c1f 100644
> > --- a/xen/arch/x86/hvm/svm/svm.c
> > +++ b/xen/arch/x86/hvm/svm/svm.c
> > @@ -2236,6 +2236,10 @@ static void svm_invlpg_intercept(unsigned long 
> > vaddr)
> >  svm_asid_g_invlpg(curr, vaddr);
> >  }
> > +static void svm_setup_tsc_scaling(struct vcpu *v)
> > +{
> > +}
> > +
>  Should this be wrmsrl(MSR_AMD64_TSC_RATIO, v->arch.tsc_scaling_ratio) ?
> 
>  -boris
> 
> >>> MSR_AMD64_TSC_RATIO is set in svm_ctxt_switch_to() before entering guest.
> >>>
> >>> For VMX, the ratio is set to a VMCS field TSC_MULTIPLIER and it's not
> >>> necessary to set it every time entering guest. Therefore, I introduce
> >>> the call-back setup_tsc_scaling() to do this. For SVM, as the ratio is
> >>> set every time entering guest, I leave the SVM version of 
> >>> setup_tsc_scaling()
> >>> empty.
> >> VT-x has a per-VMCS scale, while SVM has a per-core MSR to adjust the
> >> scale.  These do require different modification algorithms.
> >>
> > Yes, this is what I mean.
> >
> >> However, if there is any chance that any part of the system can update
> >> the ratio while an SVM VCPU is in context (which appears to be the
> >> case), then MSR_AMD64_TSC_RATIO needs updating synchronously, or it will
> >> be deferred until the next full context switch which could be an
> >> arbitrary time into the future.  This appears to be a latent bug in the
> >> SVM side.
> >>
> > In my patch, tsc ratio is set only when
> >  1. a domain is created (by arch_domain_create()),
> >  2. a vcpu's state is reset (by hvm_vcpu_reset_state()),
> >  3. a vcpu's context is restored (by hvm_load_cpu_ctxt()), or
> >  4. through the hypercall XEN_DOMCTL_settscinfo.
> >
> > (Correct me if I'm wrong below)
> >
> > For the first 3 cases, vcpu is definitely not in context, so it's safe
> > to set tsc ratio without any latent bug. For the last case,
> > arch_do_domctl() pauses the domain before updating tsc ratio, so it's
> > also safe.
> 
> That logic appears to be correct, which would suggest that there isn't
> actually a latent bug.
> 
> In such a case, we would typically make the hvm_funcs pointer optional,
> and omit an empty stub on the SVM side.
>

Yes, I'll add the following check in hvm_setup_tsc_scaling():

  if ( !hvm_funcs.setup_tsc_scaling )
  return;

- Haozhong

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

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

Re: [Xen-devel] [PATCH OSSTEST 3/3] standalone: Rotate logs rather than clobbering them

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST 3/3] standalone: Rotate logs rather than 
clobbering them"):
> Keep 300, for no better reason than cr-for-branches does.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH OSSTEST 1/6] Debian: Uninstall flash-kernel when creating our own boot.scr

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST 1/6] Debian: Uninstall flash-kernel when 
creating our own boot.scr"):
> flash-kernel will run from various kernel postinst hooks and overwrite
> our own boot scripts. While this might be tollerable for the initial
> installation we don't want to risk it occuring after we have created
> our own boot.scr to boot xen.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH OSSTEST 0/6] Fixes for switching to Jessie as base OS for test hosts

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST 0/6] Fixes for switching to Jessie as base 
OS for test hosts"):
>  * Switch to apt-cacher-ng on the cache host, since a bug in Jessie's apt
>apparently interacts badly with apt-cacher's handling of Ranges in http
>requests[0]. I've deployed apt-cacher-ng on an alternative port on the
>production cache and tested that it works with Jessie and Wheezy.

Did we not decide to do this with a change to production-config ?

Ian.

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

Re: [Xen-devel] [OSSTEST PATCH 6/7] ts-debian-fixup: Put "/mnt" in a Perl variable

[Ian Campbell]

On Tue, 2015-09-29 at 14:37 +0100, Ian Jackson wrote:
> No functional change now, but this will allow us to change the
> mountpoint.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [PATCH 2/3] xen/arm: vgic-v3: Correctly retrieve the vCPU associated to a re-distributor

[Julien Grall]

On 29/09/15 13:24, Shameerali Kolothum Thodi wrote:
> 
> 
>> -Original Message-
>> From: Julien Grall [mailto:julien.gr...@citrix.com]
>> Sent: 28 September 2015 21:32
>> To: xen-de...@lists.xenproject.org
>> Cc: ian.campb...@citrix.com; stefano.stabell...@eu.citrix.com; Julien
>> Grall; Shameerali Kolothum Thodi; Wei Liu
>> Subject: [PATCH 2/3] xen/arm: vgic-v3: Correctly retrieve the vCPU
>> associated to a re-distributor
>>
>> When the guest is accessing the re-distributor, Xen retrieves the base
>> of the re-distributor using a mask based on the stride.
>>
> 
>  
>> with multiple bit set [1]. Although, we don't support such platform
>> right now. So I would rather defer this change to Xen 4.6.1 and
>> avoid possible downside/bug in this patch.
>>
>> Shamaeerali, I've only tested quickly on the foundation model. Can
>> you give a try on your platform to check if it fixes DOM0 boot?
> 
> 
> I tried and it fixes the issue on our platform :). 

Great, thank you for testing! Can I add your Tested-by?

> (I applied only the first two patches in the series).

The last one is only a clean up so it's fine.

>> [1] http://lists.xen.org/archives/html/xen-devel/2015-
>> 09/msg03372.html
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 


-- 
Julien Grall

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

Re: [Xen-devel] Xen4.7 unable to get domain type for domid

[Wei Liu]

On Mon, Sep 28, 2015 at 10:24:15PM -0400, soapcn wrote:
[...]
> domainbuilder: detail: shared_info_x86_64: called
> domainbuilder: detail: vcpu_x86_64: called
> domainbuilder: detail: vcpu_x86_64: cr3: pfn 0x6c0d mfn 0x12c40d
> domainbuilder: detail: launch_vm: called, ctxt=0x7f2c9ae6d004
> domainbuilder: detail: xc_dom_release: called
> libxl: error: libxl_dom.c:37:libxl__domain_type: unable to get domain type
> for domid=5
> xl: unable to exec console client: No such file or directory
> libxl: error: libxl_exec.c:118:libxl_report_child_exitstatus: console child
> [3755] exited with error status 1
> 

Couldn't think of a reason why this would fail. Is this a freshly
installed Xen? If not, is the previous installation purged?

Wei.

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

Re: [Xen-devel] [PATCH v2] x86/PoD: shorten certain operations on higher order ranges

[Andrew Cooper]

On 29/09/15 13:57, Jan Beulich wrote:
 On 29.09.15 at 14:20,  wrote:
>> On 28/09/15 15:30, Jan Beulich wrote:
>>> --- a/xen/arch/x86/mm/p2m-pod.c
>>> +++ b/xen/arch/x86/mm/p2m-pod.c
>>> @@ -119,20 +119,23 @@ p2m_pod_cache_add(struct p2m_domain *p2m
>>>  
>>>  unlock_page_alloc(p2m);
>>>  
>>> -/* Then add the first one to the appropriate populate-on-demand list */
>>> -switch(order)
>>> +/* Then add to the appropriate populate-on-demand list. */
>>> +switch ( order )
>>>  {
>>> +case PAGE_ORDER_1G:
>>> +for ( i = 0; i < (1UL << PAGE_ORDER_1G); i += 1UL << PAGE_ORDER_2M 
>>> )
>>> +page_list_add_tail(page + i, >pod.super);
>>> +break;
>>>  case PAGE_ORDER_2M:
>>> -page_list_add_tail(page, >pod.super); /* lock: page_alloc */
>>> -p2m->pod.count += 1 << order;
>>> +page_list_add_tail(page, >pod.super);
>>>  break;
>>>  case PAGE_ORDER_4K:
>>> -page_list_add_tail(page, >pod.single); /* lock: page_alloc */
>>> -p2m->pod.count += 1;
>>> +page_list_add_tail(page, >pod.single);
>>>  break;
>>>  default:
>>>  BUG();
>>>  }
>>> +p2m->pod.count += 1 << order;
>> 1UL
> Not really - the field is a "long" one, so at best 1L or 1U. And then
> all the valid order values are visible right above, for none of them
> it makes a difference, and there are ample similar uses scattered
> around the file (yes, bad examples are no excuse, but in cases
> where the suffix doesn't really matter I think it is better to omit it).
>
>> Otherwise, Reviewed-by: Andrew Cooper 
> Let me know regrading this one,

For sanity sake, I would suggest going with 1L as one less place to go
wrong when we gain 512GB superpages.

~Andrew

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

Re: [Xen-devel] [PATCH] xen/pvhvm: add soft reset on kexec/kdump support

[Boris Ostrovsky]

On 09/29/2015 07:39 AM, Vitaly Kuznetsov wrote:

Boris Ostrovsky  writes:


On 09/25/2015 03:35 PM, Konrad Rzeszutek Wilk wrote:

On Fri, Sep 25, 2015 at 03:19:57PM -0400, Boris Ostrovsky wrote:

On 09/25/2015 03:01 PM, Konrad Rzeszutek Wilk wrote:

On Fri, Sep 25, 2015 at 01:17:40PM -0400, Boris Ostrovsky wrote:

On 09/25/2015 12:07 PM, Vitaly Kuznetsov wrote:

Also, I am not sure I see how this new op will be used in the
hypervisor --- currently AFAICS it is only processed under
is_hardware_domain(). Are there other patches that will support HVM
guests?

Please see my Xen series:
http://lists.xenproject.org/archives/html/xen-devel/2015-09/msg00547.html
(last 'full' submission).

All patches from my 'toolstack-assisted approach to PVHVM guest kexec'
are already merged to xen.git (first 10 are already in 'master' and the
last one is in 'staging').


OK, so I was looking at the right tree. Then I don't understand how
SHUTDOWN_soft_reset would be reached for a non-privileged domain. The only
path that I see is

  domain_shutdown()
  {
  ...
  if ( is_hardware_domain(d) )
  hwdom_shutdown(reason);
  ...
  }

Is there another path to handle this op?

Yes:
   e1bd9812966de9a16f30a58e7162b80bd6af361b libxc: support 
XEN_DOMCTL_soft_reset operation
and
   c57e6ebd8c3e490e353e68d96abec1dad01e72f5 (lib)xl: soft reset support


That's toolstack issuing hypercalls from dom0.

I am asking about (non-privileged) guest itself calling SCHEDOP_shutdown.

The hypervisor ends up calling:
__domain_finalise_shutdown which sends an VIRQ_DOM_EXC to dom0 which
makes the toolstack do all of those operations.

OK. But the I don't see anyone checking that 'reason' (or
'shutdown_code') is SHUTDOWN_soft_reset. In other words, the guest can
do 'xen_reboot(23)' or 'xen_reboot(154)'. Or, it seems, even
'xen_reboot(SHUTDOWN_reboot)'? The only value it shouldn't be is
SHUTDOWN_suspend.

Xen hypervisor doesn't analyzie the reason, it is being analyzed by the
toolstack (XEN_DOMCTL_getdomaininfo returns this info encoded in flags
with XEN_DOMINF_shutdownshift shift).


Thanks.

My problem was that I didn't realize that toolstack generates macros for 
shutdown reasons from libxl_types.idl and I couldn't see who looks for 
SHUTDOWN_soft_reset.


-boris


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

Re: [Xen-devel] [PATCH v1 3/8] xen/arm: Support sign-extension for every read access

[Ian Campbell]

On Tue, 2015-09-29 at 12:13 +0100, Julien Grall wrote:
> > Or is it, couldn't we be updating a byte in the middle of the word?
> 
> *r is a register, the byte/half-word/word... are always living in the
> lowest bit of the register.

So just to check:

ldrs* do support e.g odd offsets, but given a 32 bit MMIO region which
reads 0xAABBCCDD then when reading from offset 1 at this point *r contains
0x00CC already (and not 0xC00) and sign extending the lowest byte
is therefore correct.

Right?


> 
> > Probably figuring out the correct assertion is more hassle than it is
> > worth..
> 
> I would rather skip it.
> 
> [1] http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf
> 

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

Re: [Xen-devel] [PATCH v1 7/8] xen/arm: vgic: Introduce helpers to read/write/clear/set vGIC register ...

[Ian Campbell]

On Fri, 2015-09-25 at 15:51 +0100, Julien Grall wrote:
> and use them in the vGIC emulation.
> 
> The GIC registers may support different access sizes. Rather than open
> coding the access for every registers, provide a set of helpers to access
> them.
> 
> The caller will have to call vgic_regN_* where N is the size of the
> emulated registers.

These helpers end up as e.g. vgic_regN_read/write, but they don't really
read or write anything, they just extract the required bits into a register
or update the bits into a variable.

Can we think of a better name? encode/decode?

This might be more apparent (maybe without the need to rename) if the
helpers took info->dabt.size instead of info.

> [...]
> +static inline void vgic_reg_clearbit(unsigned long *reg, register_t bits,

Please call these ones clearbits/setbits to avoid giving the impression
that they take a bit offset and just set/clear that one.

Ian.


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

[Xen-devel] [OSSTEST PATCH 6/7] ts-debian-fixup: Put "/mnt" in a Perl variable

[Ian Jackson]

No functional change now, but this will allow us to change the
mountpoint.

Signed-off-by: Ian Jackson 
---
 ts-debian-fixup |   14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/ts-debian-fixup b/ts-debian-fixup
index beae049..6d24587 100755
--- a/ts-debian-fixup
+++ b/ts-debian-fixup
@@ -50,15 +50,17 @@ sub ether () {
 store_runvar("$gho->{Guest}_tcpcheckport", 22);
 }
 
+our $mountpoint = '/mnt';
+
 sub access () {
 guest_umount_lv($ho, $gho);
 target_cmd_root($ho, <{Vg}/$gho->{Lv} /mnt
+mount /dev/$gho->{Vg}/$gho->{Lv} $mountpoint
 perl -i~ -pe "s/^root:[^:]+:/root::/" /etc/shadow
-mkdir -p /mnt/root/.ssh /mnt/etc/ssh
-cp -a /root/.ssh/* /mnt/root/.ssh/.
-cp -a /etc/ssh/ssh_host_*key* /mnt/etc/ssh/.
+mkdir -p $mountpoint/root/.ssh $mountpoint/etc/ssh
+cp -a /root/.ssh/* $mountpoint/root/.ssh/.
+cp -a /etc/ssh/ssh_host_*key* $mountpoint/etc/ssh/.
 END
 }
 
@@ -66,7 +68,7 @@ our $extra;
 
 sub console () {
 my $console=
-target_kernkind_console_inittab($ho,$gho,"/mnt");
+target_kernkind_console_inittab($ho,$gho,"$mountpoint");
 return unless length $console;
 
 my $xextra= "console=$console earlyprintk=xen";
@@ -97,7 +99,7 @@ sub filesystems () {
 set -ex
 perl -i~ -pe "
 s,^(/dev/)sda(d+),\\\${1}$rootdev\\\$2,;
-" /mnt/etc/fstab
+" $mountpoint/etc/fstab
 END
 }
 
-- 
1.7.10.4


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

Re: [Xen-devel] [PATCH v1 6/8] xen/arm: vgic: Optimize the way to store the target vCPU in the rank

[Julien Grall]

On 29/09/15 14:07, Ian Campbell wrote:
> On Fri, 2015-09-25 at 15:51 +0100, Julien Grall wrote:
>> Xen is currently directly storing the value of register GICD_ITARGETSR
>> (for GICv2) and GICD_IROUTER (for GICv3) in the rank. This makes the
>> emulation of the registers access very simple but makes the code to get
>> the target vCPU for a given IRQ more complex.
>>
>> While the target vCPU of an IRQ is retrieved everytime an IRQ is
>> injected to the guest, the access to the register occurs less often.
>>
>> So the data structure should be optimized for the most common case
>> rather than the inverse.
>>
>> This patch introduce the usage of an array to store the target vCPU for
>> every interrupt in the rank. This will make the code to get the target
>> very quick. The emulation code will now have to generate the
>> GICD_ITARGETSR
>> and GICD_IROUTER register for read access and split it to store in a
>> convenient way.
>>
>> Note that with these changes, any read to those registers will list only
>> the target vCPU used by Xen. This is fine because the GIC spec doesn't
>> require to return exactly the value written and it can be seen as if we
>> decide to implement the register read-only.
> 
> I think this is probably OK, but skirting round what the spec actually says
> a fair bit.

Well, nothing in the spec clearly explain the behavior of a read access
on the register. An implementation could decide to make some bits RO or
even not store everything.

FWIW, KVM is using the same trick.

>> Finally take the opportunity to fix coding style in section we are
>> modifying.
>>
>> Signed-off-by: Julien Grall 

[...]

>> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
>> index 23d1982..b6db64f 100644
>> --- a/xen/arch/arm/vgic-v2.c
>> +++ b/xen/arch/arm/vgic-v2.c
>> @@ -50,6 +50,90 @@ void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase,
>> paddr_t vbase)
>>  vgic_v2_hw.vbase = vbase;
>>  }
>>  
>> +#define NR_TARGET_PER_REG 4U
>> +#define NR_BIT_PER_TARGET 8U
>> +
>> +/*
>> + * Generate the associated ITARGETSR register based on the offset from
>> + * ITARGETSR0. Only one vCPU will be listed for a given IRQ.
>> + *
>> + * Note the offset will be round down to match a real HW register.
> 
> "rounded down".
> 
> Although I'm not 100% sure what that comment is trying to say. From the
> code I think you mean aligned to the appropriate 4 byte boundary?

Yes. What about: "Note the offset will be aligned to the appropriate 4
byte boundary"

> 
>> + */
>> +static uint32_t vgic_generate_itargetsr(struct vgic_irq_rank *rank,
> 
> For all these why not s/generate/read/?
> 
> Or since you use "store" for the other half "fetch".
> 
> ("generate" is a bit of an implementation detail, the caller doesn't care
> where or how the result is achieved)

I will rename to vgic_fetch_itargetsr(...).


[...]

>> + */
>> +static void vgic_store_itargetsr(struct domain *d, struct vgic_irq_rank
>> *rank,
>> + unsigned int offset, uint32_t
>> itargetsr)
>> +{
>> +unsigned int i, rank_idx;
>> +
>> +ASSERT(spin_is_locked(>lock));
>> +
>> +rank_idx = offset / NR_INTERRUPT_PER_RANK;
>> +
>> +offset &= INTERRUPT_RANK_MASK;
>> +offset &= ~(NR_TARGET_PER_REG - 1);
>> +
>> +for ( i = 0; i < NR_TARGET_PER_REG; i++, offset++ )
>> +{
>> +unsigned int new_target, old_target;
>> +
>> +/*
>> + * SPI are using the 1-N model (see 1.4.3 in ARM IHI 0048B).
>> + * While the interrupt could be set pending to all the vCPUs in
>> + * target list, it's not guarantee by the spec.
>> + * For simplicity, always route the IRQ to the first interrupt
>> + * in the target list
>> + */
> 
> I don't see anything which is handling the PPI and SGI special case (which
> is that they are r/o).
> 
> Likewise on read they are supposed to always reflect the value of the CPU
> doing the read.
> 
> Are both of those handled elsewhere in some way I'm missing?

A write to those registers is ignored and handled by a separate case
(GICD_ITARGETSR ... GICD_ITARGETSR + 7).

[...]


>>  static struct vcpu *vgic_v3_get_target_vcpu(struct vcpu *v, unsigned int
>> irq)
>>  {
>> -struct vcpu *v_target;
>>  struct vgic_irq_rank *rank = vgic_rank_irq(v, irq);
>>  
>>  ASSERT(spin_is_locked(>lock));
>>  
>> -v_target = vgic_v3_irouter_to_vcpu(v->domain, rank->v3.irouter[irq %
>> 32]);
>> -
>> -ASSERT(v_target != NULL);
>> -
>> -return v_target;
>> +return v->domain->vcpu[rank->vcpu[irq & INTERRUPT_RANK_MASK]];
> 
> 
> Looks like there isn't much call for this to be specific to a given
> revision of the gic any more?

Correct, although I keep them as it is because I wasn't sure how this
would fit with the ITS support.

Though we could add LPIs specific helpers if necessary. So I will add a
patch to drop the callbacks get_target_vcpu and get_irq_priority.

> 
> Are there sufficient checks for the range of 

[Xen-devel] [OSSTEST PATCH 0/7] Fix Debian HVM ssh ECONNREFUSED race

[Ian Jackson]

This race seems actually to exist in principle in all of our hosts and
guests.

The first patch nobbles it for all installs based on preseeding, by
overwriting the unnecessary and problematic hook script in our
overlay.

The remaining patches arrange to install the overlay in
ts-debian-fixup, which is called for installs done with
xen-create-image rather than d-i.


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

[Xen-devel] [OSSTEST PATCH 7/7] ts-debian-fixup: Install the overlays

[Ian Jackson]

We want debootstrap-installed guests to get these overlays too.

Signed-off-by: Ian Jackson 
---
 ts-debian-fixup |   14 ++
 1 file changed, 14 insertions(+)

diff --git a/ts-debian-fixup b/ts-debian-fixup
index 6d24587..3261185 100755
--- a/ts-debian-fixup
+++ b/ts-debian-fixup
@@ -19,6 +19,7 @@ use strict qw(vars);
 use DBI;
 use Osstest;
 use Osstest::TestSupport;
+use Osstest::Debian;
 
 tsreadconfig();
 
@@ -64,6 +65,18 @@ sub access () {
 END
 }
 
+sub overlay ($$) {
+my ($srcdir, $tfilename) = @_;
+
+my $leaf = "$gho->{Guest}-$tfilename";
+my $fh = open_unique_stashfile(\$leaf);
+contents_make_cpio($fh,'ustar',$srcdir);
+seek $fh,0,0 or die "$leaf: $!";
+target_cmd_inputfh_root($ho, $fh, <

[Xen-devel] [OSSTEST PATCH 4/7] TestSupport: Provide target_cmd_inputfh_root

[Ian Jackson]

No caller yet.

Signed-off-by: Ian Jackson 
---
 Osstest/TestSupport.pm |6 ++
 1 file changed, 6 insertions(+)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 91829a0..3fc8e15 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -50,6 +50,7 @@ BEGIN {
 
   target_cmd_root target_cmd target_cmd_build
   target_cmd_output_root target_cmd_output
+  target_cmd_inputfh_root
   target_getfile target_getfile_root
   target_putfile target_putfile_root
   target_putfilecontents_stash
@@ -655,6 +656,11 @@ sub tcmdout {
 sub target_cmd_output ($$;$) { tcmdout('osstest',@_); }
 sub target_cmd_output_root ($$;$) { tcmdout('root',@_); }
 
+sub target_cmd_inputfh_root ($$$;$$) {
+my ($tho,$stdinfh,$tcmd,@rest) = @_;
+tcmd($stdinfh,undef,'root',$tho,$tcmd,@rest);
+}
+
 sub poll_loop ($$$&) {
 my ($maxwait, $interval, $what, $code) = @_;
 # $code should return undef when all is well
-- 
1.7.10.4


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

Re: [Xen-devel] [PATCH] Add missing license and copyright statements to public interface headers.

[Mike Belopuhov]

On Tue, Sep 29, 2015 at 13:23 +0100, Andrew Cooper wrote:
> On 29/09/15 13:03, Mike Belopuhov wrote:
> > On Mon, Sep 28, 2015 at 11:24 +0200, Mike Belopuhov wrote:
> >> On Fri, Sep 25, 2015 at 01:12 -0600, Jan Beulich wrote:
> >> On 22.09.15 at 16:02,  wrote:
>  --- xen/include/public/arch-x86/pmu.h
>  +++ xen/include/public/arch-x86/pmu.h
> >>> I fixed this up for you this time, but in the future please make sure
> >>> you send patches in conventional format (applicable with patch's -p1
> >>> or whatever tool's equivalent).
> >>>
> >>> Thanks, Jan
> >>>
> >> Thanks, Jan.  I'll keep that in mind.
> > Hi Guys,
> >
> > Are we waiting for others to Ack the diff or is there anything
> > I should do?
> >
> > With best regards,
> > Mike
> 
> This has been committed (staging and staging-4.6)
> 
> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=8d6ff9ef259e296e6aee32ad8840cc5081280e0d
> 
> In both cases, they have not yet passed the automatic test gate, which
> is why they have not appeared in master and stable-4.6
> 
> ~Andrew

Oh, pardon my impatience then!  And thanks a lot!

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

Re: [Xen-devel] [PATCH v5 3/3] tools & docs: add tools and docs support for Intel CDP

[Ian Campbell]

On Mon, 2015-09-28 at 16:29 +0800, He Chen wrote:
> This is the xl/xc changes to support Intel Code/Data Prioritization.
> CAT xl commands to set/get CBMs are extended to support CDP.
> Add new CDP options with CAT commands in xl interface man page.
> Add description of CDP in xl-psr.markdown.
> 
> Signed-off-by: He Chen 
> ---
> Changes in v5:
> * merge tools and docs patches
> * replace EINVAL with ENXIO in libxl__psr_cat_log_err_msg
> * revise options parsing in psr-cat-cbm-set and invalidate passing -c
>   and -d at the same time
> * refine CDP status output codes in psr_cat_hwinfo
> * adjust CBM output format in command xl psr-cat-show
> * docs revision
> 
> Example of new output format for command xl psr-cat-show:
> 
> CAT-only:
> 
> Socket ID   : 0
> L3 Cache: 56320KB
> Default CBM : 0xf
>ID NAME   CBM
> 0 Domain-0   0xf
> 1   centos.hvm   0xf
> 
> CDP enabled:
> 
> Socket ID   : 0
> L3 Cache: 56320KB
> Default CBM : 0xf
>ID NAME   CBM
> 0 Domain-0 code: 0xf data: 0xf
> 1   centos.hvm code: 0xf data: 0xf

The CBM column header seems a bit out of place.

Perhaps have separate headings, e.g. "CBM (code)" and "CBD (data)"?

diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown

> index 3545912..0527211 100644
> --- a/docs/misc/xl-psr.markdown
> +++ b/docs/misc/xl-psr.markdown
> @@ -14,7 +14,7 @@ tracks cache utilization of memory accesses according
> to the RMID and reports
>  monitored data via a counter register.
>  
>  For more detailed information please refer to Intel SDM chapter
> -"17.14 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
> +"Platform Shared Resource Monitoring: Cache Monitoring Technology".

I think these will clash with Cheng Pao's PSR updates, which have already
been applied.
 
> 
> +Code/Data Prioritization (CDP) Technology is an extension of CAT, which is

Other bits of documentation added here seem to suggest it is actually an
extension of CBM, is that right?

> diff --git a/tools/libxc/include/xenctrl.h
> b/tools/libxc/include/xenctrl.h
> index 2000f12..8f4acec 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2794,7 +2794,9 @@ enum xc_psr_cmt_type {
>  typedef enum xc_psr_cmt_type xc_psr_cmt_type;
>  
>  enum xc_psr_cat_type {
> -XC_PSR_CAT_L3_CBM = 1,
> +XC_PSR_CAT_L3_CBM  = 1,
> +XC_PSR_CAT_L3_CODE = 2,
> +XC_PSR_CAT_L3_DATA = 3,

If CDP is an extension of CBM, then would CBM_CODE + CBM_DATA be better
names (more important in the libxl API than here)

> +if (opt_data && opt_code) {
> +fprintf(stderr, "Invalid to pass both -c and -d at the same time\n");

To be correct this would need to say "It is invalid..." but a more natural
sounding (to me) way to express this would be

"Cannot handle -c and -d at the same time"

I think.


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

Re: [Xen-devel] [PATCH v2] xen: credit1: fix tickling when it happens from a remote pCPU

[George Dunlap]

On 25/09/15 08:46, Dario Faggioli wrote:
> especially if that is also from a different cpupool than the
> processor of the vCPU that triggered the tickling.
> 
> In fact, it is possible that we get as far as calling vcpu_unblock()-->
> vcpu_wake()-->csched_vcpu_wake()-->__runq_tickle() for the vCPU 'vc',
> but all while running on a pCPU that is different from 'vc->processor'.
> 
> For instance, this can happen when an HVM domain runs in a cpupool,
> with a different scheduler than the default one, and issues IOREQs
> to Dom0, running in Pool-0 with the default scheduler.
> In fact, right in this case, the following crash can be observed:
> 
> (XEN) [ Xen-4.7-unstable  x86_64  debug=y  Tainted:C ]
> (XEN) CPU:7
> (XEN) RIP:e008:[] __runq_tickle+0x18f/0x430
> (XEN) RFLAGS: 00010086   CONTEXT: hypervisor (d1v0)
> (XEN) rax: 0001   rbx: 8303184fee00   rcx: 
> (XEN) ... ... ...
> (XEN) Xen stack trace from rsp=83031fa57a08:
> (XEN)82d0801fe664 82d08033c820 00010002 000a0001
> (XEN)6831   
> (XEN) ... ... ...
> (XEN) Xen call trace:
> (XEN)[] __runq_tickle+0x18f/0x430
> (XEN)[] csched_vcpu_wake+0x10b/0x110
> (XEN)[] vcpu_wake+0x20a/0x3ce
> (XEN)[] vcpu_unblock+0x4b/0x4e
> (XEN)[] vcpu_kick+0x17/0x61
> (XEN)[] vcpu_mark_events_pending+0x2c/0x2f
> (XEN)[] evtchn_fifo_set_pending+0x381/0x3f6
> (XEN)[] notify_via_xen_event_channel+0xc9/0xd6
> (XEN)[] hvm_send_ioreq+0x3e9/0x441
> (XEN)[] hvmemul_do_io+0x23f/0x2d2
> (XEN)[] hvmemul_do_io_buffer+0x33/0x64
> (XEN)[] hvmemul_do_pio_buffer+0x35/0x37
> (XEN)[] handle_pio+0x58/0x14c
> (XEN)[] vmx_vmexit_handler+0x16b3/0x1bea
> (XEN)[] vmx_asm_vmexit_handler+0x41/0xc0
> 
> In this case, pCPU 7 is not in Pool-0, while the (Dom0's) vCPU being
> woken is. pCPU's 7 pool has a different scheduler than credit, but it
> is, however, right from pCPU 7 that we are waking the Dom0's vCPUs.
> Therefore, the current code tries to access csched_balance_mask for
> pCPU 7, but that is not defined, and hence the Oops.
> 
> (Note that, in case the two pools run the same scheduler we see no
> Oops, but things are still conceptually wrong.)
> 
> Cure things by making the csched_balance_mask macro accept a
> parameter for fetching a specific pCPU's mask (instead than always
> using smp_processor_id()).
> 
> Signed-off-by: Dario Faggioli 
> Reviewed-by: Juergen Gross 

Looks good!

Reviewed-by: George Dunlap 


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

Re: [Xen-devel] [PATCH OSSTEST 2/3] standalone: Check job status at end of run-job.

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST 2/3] standalone: Check job status at end 
of run-job."):
> Check if the job passed and if not (so status is fail, broken, running
> etc) then return an error.
> 
> This is convenient for scripting.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH OSSTEST 1/3] standalone: Add get-job-status to pick status out of standalone.db

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST 1/3] standalone: Add get-job-status to 
pick status out of standalone.db"):
> The return code of sg-run-job does not reflect the state of the job,
> which is instead written to the database. For the benefit of running
> tests in a loop until failure add a command to retrieve the status to
> stdout.

Maybe this would be better as a cs-* utility, useable on production
instances too ?

Ian.

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

[Xen-devel] [ovmf baseline-only test] 38088: all pass

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 38088 ovmf real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/38088/

Perfect :-)
All tests in this flight passed
version targeted for testing:
 ovmf 28f27af6f007c3794fcc9d098ef91713160f4e5b
baseline version:
 ovmf 8cfd008ef8da26d97314816e0635691955d475d5

Last test of basis38024  2015-09-23 13:54:22 Z5 days
Testing same since38088  2015-09-27 11:36:09 Z1 days1 attempts


People who touched revisions under test:
  Ard Biesheuvel 
  Feng Tian 
  Gabriel Somlo 
  Heyi Guo 
  Jaben Carsey 
  Jeff Fan 
  Jiaxin Wu 
  Laszlo Ersek 
  Reza Jelveh 
  Ruiyu Ni 
  Tapan  Shah 
  Ye Ting 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

Test harness code can be found at
http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary


Push not applicable.

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

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

[Xen-devel] [linux-3.18 test] 62450: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail REGR. vs. 58581

Tests which are failing intermittently (not blocking):
 test-amd64-i386-qemut-rhel6hvm-amd 14 leak-check/check  fail pass in 62365

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt  6 xen-boot  fail REGR. vs. 58581
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 13 guest-localmigrate 
fail baseline untested
 test-amd64-i386-libvirt-pair 22 guest-migrate/dst_host/src_host fail baseline 
untested
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 16 
guest-start/debianhvm.repeat fail baseline untested
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 13 guest-localmigrate 
fail baseline untested
 test-armhf-armhf-xl-rtds 15 guest-start.2  fail in 62365 baseline untested
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install fail 
in 62365 baseline untested
 test-armhf-armhf-xl-credit2   6 xen-boot fail   like 58581
 test-armhf-armhf-libvirt-xsm  6 xen-boot fail   like 58581
 test-armhf-armhf-xl-multivcpu  6 xen-boot fail  like 58581
 test-armhf-armhf-xl   6 xen-boot fail   like 58581
 test-armhf-armhf-xl-xsm   6 xen-boot fail   like 58581
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail like 58581
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail like 58581
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail like 58581

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-armhf-armhf-xl-qcow2 9 debian-di-installfail   never pass
 test-armhf-armhf-xl-raw   9 debian-di-installfail   never pass
 test-armhf-armhf-xl-cubietruck  6 xen-boot fail never pass
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-amd64-i386-libvirt-qcow2 11 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-raw  11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 test-amd64-i386-libvirt-vhd  11 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass

version targeted for testing:
 linuxfcd9bfdb9d884f1aab89124dee894e7d821bb5dc
baseline version:
 linuxd048c068d00da7d4cfa5ea7651933b99026958cf

Last test of basis58581  2015-06-15 09:42:22 Z  105 days
Failing since 58976  2015-06-29 19:43:23 Z   91 days   64 attempts
Testing same since61524  2015-09-07 11:48:03 Z   21 days   10 attempts


462 people touched revisions under test,
not listing them all

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   

Re: [Xen-devel] [PATCH v2 for Xen 4.6 1/6] tools/libxl: introduce libxl_get_online_socketmap

[Wei Liu]

On Tue, Sep 29, 2015 at 03:49:50PM +0800, Chao Peng wrote:
> It sets the bit on the given bitmap if the corresponding socket is
> available and clears the bit when the corresponding socket is not
> available.
> 
> Signed-off-by: Chao Peng 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 for Xen 4.6 3/6] tools/libxl: return socket id from libxl_psr_cat_get_l3_info

[Wei Liu]

On Tue, Sep 29, 2015 at 03:49:52PM +0800, Chao Peng wrote:
> The entries returned from libxl_psr_cat_get_l3_info are assumed
> to be socket-continuous. But this is not true in the hotplug case.
> 
> This patch gets the socket bitmap for all the sockets on the system
> first and stores the socket id in the structure libxl_psr_cat_info in
> libxl_psr_cat_get_l3_info. The xl or similar consumers then can display
> socket information correctly.
> 
> Signed-off-by: Chao Peng 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH 12/13] vmx: Add a call-back to apply TSC scaling ratio to hardware

[Andrew Cooper]

On 29/09/15 02:07, Haozhong Zhang wrote:
> On Mon, Sep 28, 2015 at 12:02:08PM -0400, Boris Ostrovsky wrote:
>> On 09/28/2015 03:13 AM, Haozhong Zhang wrote:
>>> This patch adds a new call-back setup_tsc_scaling in struct
>>> hvm_function_table to apply the TSC scaling ratio to hardware. For VMX,
>>> it writes the TSC scaling ratio to VMCS field TSC_MULTIPLIER.
>>>
>>> Signed-off-by: Haozhong Zhang 
>>> ---
>>>  xen/arch/x86/hvm/hvm.c| 1 +
>>>  xen/arch/x86/hvm/svm/svm.c| 5 +
>>>  xen/arch/x86/hvm/vmx/vmx.c| 8 
>>>  xen/include/asm-x86/hvm/hvm.h | 3 +++
>>>  4 files changed, 17 insertions(+)
>>>
>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>> index 3522d20..2d8a148 100644
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -376,6 +376,7 @@ void hvm_setup_tsc_scaling(struct vcpu *v)
>>>  }
>>>  v->arch.tsc_scaling_ratio = ratio;
>>> +hvm_funcs.setup_tsc_scaling(v);
>>>  }
>>>  void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
>>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
>>> index 73bc863..d890c1f 100644
>>> --- a/xen/arch/x86/hvm/svm/svm.c
>>> +++ b/xen/arch/x86/hvm/svm/svm.c
>>> @@ -2236,6 +2236,10 @@ static void svm_invlpg_intercept(unsigned long vaddr)
>>>  svm_asid_g_invlpg(curr, vaddr);
>>>  }
>>> +static void svm_setup_tsc_scaling(struct vcpu *v)
>>> +{
>>> +}
>>> +
>> Should this be wrmsrl(MSR_AMD64_TSC_RATIO, v->arch.tsc_scaling_ratio) ?
>>
>> -boris
>>
> MSR_AMD64_TSC_RATIO is set in svm_ctxt_switch_to() before entering guest.
>
> For VMX, the ratio is set to a VMCS field TSC_MULTIPLIER and it's not
> necessary to set it every time entering guest. Therefore, I introduce
> the call-back setup_tsc_scaling() to do this. For SVM, as the ratio is
> set every time entering guest, I leave the SVM version of setup_tsc_scaling()
> empty.

VT-x has a per-VMCS scale, while SVM has a per-core MSR to adjust the
scale.  These do require different modification algorithms.

However, if there is any chance that any part of the system can update
the ratio while an SVM VCPU is in context (which appears to be the
case), then MSR_AMD64_TSC_RATIO needs updating synchronously, or it will
be deferred until the next full context switch which could be an
arbitrary time into the future.  This appears to be a latent bug in the
SVM side.

~Andrew

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

Re: [Xen-devel] [PATCH v2 for Xen 4.6 0/6] Several PSR fixes in libxl

[Wei Liu]

Now the reasoning bits. Yes, I'm arguing with myself, :-)

We can of course fix it post-4.6, but the released APIs need to be
maintained forever (even if it is in fact broken). That would definitely
involve lots of compatibility cruft if we fix it post 4.6. 

This patch series is simple enough to reason about and has received
adequate review from expert in the field, so I have hight confidence in
it being correct.

I think the benefit of accepting it out-weights the downside.

Wei.

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

Re: [Xen-devel] [Patch RFC 04/13] vt-d: Clear invalidation table in invaidation interrupt handler

[Jan Beulich]

>>> On 16.09.15 at 15:23,  wrote:
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1098,6 +1098,28 @@ static void _qi_msi_mask(struct iommu *iommu)
>  
>  static void _do_iommu_qi(struct iommu *iommu)
>  {
> +unsigned long nr_dom, i;
> +struct domain *d = NULL;
> +
> +nr_dom = cap_ndoms(iommu->cap);
> +i = find_first_bit(iommu->domid_bitmap, nr_dom);
> +while ( i < nr_dom )
> +{
> +if ( iommu->domid_map[i] > 0 )

This is a pointless check when the bit was already found set. What
instead you need to consider are races with table entries getting
removed (unless following the suggestions made on the previous
patch already make this impossible).

> +{
> +d = rcu_lock_domain_by_id(iommu->domid_map[i]);
> +if ( d == NULL )
> +continue;
> +
> +if ( qi_table_pollslot(d) == qi_table_data(d) )

So qi_table_data() gets (non-atomically) incremented in the
previous patch when a new wait command gets issued. How is
this check safe (and the zapping below) against races, and
against out of order completion of invalidations?

Jan

> +{
> +qi_table_data(d) = 0;
> +qi_table_pollslot(d) = 0;
> +}
> +rcu_unlock_domain(d);
> +}
> +i = find_next_bit(iommu->domid_bitmap, nr_dom, i+1);
> +}
>  }
>  
>  static void do_iommu_qi_completion(unsigned long data)



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

[Xen-devel] [PATCH OSSTEST 3/3] standalone: Rotate logs rather than clobbering them

[Ian Campbell]

Keep 300, for no better reason than cr-for-branches does.

Signed-off-by: Ian Campbell 
---
 standalone | 1 +
 1 file changed, 1 insertion(+)

diff --git a/standalone b/standalone
index e85457d..c3ff9e2 100755
--- a/standalone
+++ b/standalone
@@ -196,6 +196,7 @@ ensure_logs() {
 with_logging() {
 local log=$1; shift
 ensure_logs
+savelog -c 300 "$log" >/dev/null
 $@ 2>&1 | tee "$log"
 rc=${PIPESTATUS[0]}
 if [ $rc -ne 0 ] ; then
-- 
2.5.3


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

[Xen-devel] [PATCH OSSTEST 1/3] standalone: Add get-job-status to pick status out of standalone.db

[Ian Campbell]

The return code of sg-run-job does not reflect the state of the job,
which is instead written to the database. For the benefit of running
tests in a loop until failure add a command to retrieve the status to
stdout.

Signed-off-by: Ian Campbell 
---
 standalone | 32 ++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/standalone b/standalone
index 60b..0a5d96f 100755
--- a/standalone
+++ b/standalone
@@ -35,6 +35,11 @@ Operations:
   hosts next time. Otherwise osstest will complain if you change the
   host(s) which a job is running on on successive runs.
 
+* get-job-status [cf] [JOB]
+
+  Prints the status (pass, fail, running, etc) of the given job to
+  stdout.
+
 Options:
 
 -c FILE, --config=FILEUse FILE as configuration file
@@ -140,8 +145,9 @@ if [ $reuse -eq 0 ]; then
 read
 fi
 
-if [ ! -f standalone.db ] ; then
-echo "No standalone.db? Run standalone-reset." >&2
+db="standalone.db"
+if [ ! -f $db ] ; then
+echo "No $db? Run standalone-reset." >&2
 exit 1
 fi
 
@@ -198,6 +204,14 @@ with_logging() {
 fi
 }
 
+job_status() {
+flight=$1; shift
+job=$1; shift
+
+sqlite3 $db \
+"SELECT status FROM jobs WHERE flight='$flight' AND job='$job'"
+}
+
 # other potential ops:
 # - run standalone reset
 
@@ -303,6 +317,20 @@ case $op in
OSSTEST_JOB=$job \
with_logging logs/$flight/$job.$ts.log ./$ts $hosts $@
;;
+
+get-job-status)
+   need_flight;
+
+   if [ $# -ne 1 ] ; then
+   echo "get-job-status: Need job" >&2
+   exit 1
+   fi
+
+   job=$1; shift
+
+   job_status $flight $job
+
+   ;;
 *)
echo "Unknown op $op" ; exit 1 ;;
 esac
-- 
2.5.3


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

Re: [Xen-devel] [PATCH 13/13] tools/libxl: Add 'vtsc_khz' option to set guest TSC rate

[Haozhong Zhang]

On Tue, Sep 29, 2015 at 10:20:21AM +0100, Wei Liu wrote:
> On Tue, Sep 29, 2015 at 08:40:23AM +0800, Haozhong Zhang wrote:
> > On Mon, Sep 28, 2015 at 03:19:25PM +0100, Wei Liu wrote:
> > > On Mon, Sep 28, 2015 at 03:13:58PM +0800, Haozhong Zhang wrote:
> > > > This patch adds an option 'vtsc_khz' to allow users to set vcpu's TSC
> > > > rate in KHz. In the case that tsc_mode = 'default', the default value of
> > > > 'vtsc_khz' option is the host TSC rate which is used when 'vtsc_khz'
> > > > option is set to 0 or does not appear in the configuration. In all other
> > > > cases of tsc_mode, 'vtsc_khz' option is just ignored.
> > > > 
> > > > Another purpose of adding this option is to keep vcpu's TSC rate across
> > > > guest reboot. In existing code, a new domain is created from the
> > > > configuration of the previous domain which was just rebooted. vcpu's TSC
> > > > rate is not stored in the configuration and the host TSC rate is the
> > > > used as vcpu's TSC rate. This works fine unless the previous domain was
> > > > migrated from another host machine with a different host TSC rate than
> > > > the current one.
> > > > 
> > > > Signed-off-by: Haozhong Zhang 
> > > > ---
> > > >  tools/libxl/libxl_types.idl |  1 +
> > > >  tools/libxl/libxl_x86.c |  4 +++-
> > > >  tools/libxl/xl_cmdimpl.c| 22 ++
> > > >  3 files changed, 26 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > > > index 9f6ec00..91cb0be 100644
> > > > --- a/tools/libxl/libxl_types.idl
> > > > +++ b/tools/libxl/libxl_types.idl
> > > > @@ -413,6 +413,7 @@ libxl_domain_build_info = 
> > > > Struct("domain_build_info",[
> > > >  ("vcpu_soft_affinity", Array(libxl_bitmap, 
> > > > "num_vcpu_soft_affinity")),
> > > >  ("numa_placement",  libxl_defbool),
> > > >  ("tsc_mode",libxl_tsc_mode),
> > > > +("vtsc_khz",uint32),
> > > 
> > > This field should be in arch-specific substructure, i.e. "hvm".
> > >
> > 
> > Julien also pointed out this and suggested to moving to an
> > arch-specific substructure. I'm going to add a new substructure
> > "arch_x86" and move "vtsc_khz" there. Is this good for you?
> > 
> 
> My initial thought was that this was a feature of HVM. I don't recollect
> why I got that idea. I could be wrong.  Does it work with (or intent to
> work with) PV too?  If yes, adding it to arch_x86 would be appropriate.
> If not, hvm specific is good enough.  Please correct my
> misunderstanding, I'm definitely no expert on x86.  
>

No, it only works with HVM. So it should be in 'hvm'? Julien, what is
your opinion?

- Haozhong

> > > >  ("max_memkb",   MemKB),
> > > >  ("target_memkb",MemKB),
> > > >  ("video_memkb", MemKB),
> > > > diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> > > > index 896f34c..7baaee4 100644
> > > > --- a/tools/libxl/libxl_x86.c
> > > > +++ b/tools/libxl/libxl_x86.c
> > > > @@ -276,6 +276,7 @@ int libxl__arch_domain_create(libxl__gc *gc, 
> > > > libxl_domain_config *d_config,
> > > >  {
> > > >  int ret = 0;
> > > >  int tsc_mode;
> > > > +uint32_t vtsc_khz;
> > > >  uint32_t rtc_timeoffset;
> > > >  libxl_ctx *ctx = libxl__gc_owner(gc);
> > > >  
> > > > @@ -300,7 +301,8 @@ int libxl__arch_domain_create(libxl__gc *gc, 
> > > > libxl_domain_config *d_config,
> > > >  default:
> > > >  abort();
> > > >  }
> > > > -xc_domain_set_tsc_info(ctx->xch, domid, tsc_mode, 0, 0, 0);
> > > > +vtsc_khz = d_config->b_info.vtsc_khz;
> > > > +xc_domain_set_tsc_info(ctx->xch, domid, tsc_mode, 0, vtsc_khz, 0);
> > > >  if (libxl_defbool_val(d_config->b_info.disable_migrate))
> > > >  xc_domain_disable_migrate(ctx->xch, domid);
> > > >  rtc_timeoffset = d_config->b_info.rtc_timeoffset;
> > > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > > > index 2706759..5fabda7 100644
> > > > --- a/tools/libxl/xl_cmdimpl.c
> > > > +++ b/tools/libxl/xl_cmdimpl.c
> > > > @@ -1462,6 +1462,28 @@ static void parse_config_data(const char 
> > > > *config_source,
> > > >  }
> > > >  }
> > > >  
> > > > +/* "vtsc_khz" option works only if "tsc_mode" option is
> > > > + * "default". In this case, if "vtsc_khz" option is set to 0, we
> > > > + * will reset it to the host TSC rate. In all other cases, we just
> > > > + * ignore any given value and always set it to 0.
> > > > + */
> > > > +if (!xlu_cfg_get_long(config, "vtsc_khz", , 0))
> > > > +b_info->vtsc_khz = l;
> > > > +if (b_info->tsc_mode == LIBXL_TSC_MODE_DEFAULT) {
> > > > +if (b_info->vtsc_khz == 0) {
> > > > +libxl_physinfo physinfo;
> > > > +if (!libxl_get_physinfo(ctx, ))
> > > > +b_info->vtsc_khz = physinfo.cpu_khz;
> > > > +else
> > > > +fprintf(stderr, "WARNING: cannot get host TSC 
> > 

Re: [Xen-devel] [PATCH 13/13] tools/libxl: Add 'vtsc_khz' option to set guest TSC rate

[Ian Campbell]

On Mon, 2015-09-28 at 15:13 +0800, Haozhong Zhang wrote:
> This patch adds an option 'vtsc_khz' to allow users to set vcpu's TSC
> rate in KHz. In the case that tsc_mode = 'default', the default value of
> 'vtsc_khz' option is the host TSC rate which is used when 'vtsc_khz'
> option is set to 0 or does not appear in the configuration. In all other
> cases of tsc_mode, 'vtsc_khz' option is just ignored.
> 
> Another purpose of adding this option is to keep vcpu's TSC rate across
> guest reboot. In existing code, a new domain is created from the
> configuration of the previous domain which was just rebooted. vcpu's TSC
> rate is not stored in the configuration and the host TSC rate is the
> used as vcpu's TSC rate. This works fine unless the previous domain was
> migrated from another host machine with a different host TSC rate than
> the current one.

I understand why this is necessary over a migration, but why is it
important to be able to retain the TSC rate across a reboot? What is the
usecase there?

> Signed-off-by: Haozhong Zhang 
> ---
>  tools/libxl/libxl_types.idl |  1 +
>  tools/libxl/libxl_x86.c |  4 +++-
>  tools/libxl/xl_cmdimpl.c| 22 ++

The documentation should be patched at the same time. At least the xl.cfg
manpage, but I think there is also a specific document about time and the
TSC which should also be updated.

Ian.

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

Re: [Xen-devel] [PATCH 13/13] tools/libxl: Add 'vtsc_khz' option to set guest TSC rate

[Haozhong Zhang]

On Tue, Sep 29, 2015 at 11:04:14AM +0100, Ian Campbell wrote:
> On Mon, 2015-09-28 at 15:13 +0800, Haozhong Zhang wrote:
> > This patch adds an option 'vtsc_khz' to allow users to set vcpu's TSC
> > rate in KHz. In the case that tsc_mode = 'default', the default value of
> > 'vtsc_khz' option is the host TSC rate which is used when 'vtsc_khz'
> > option is set to 0 or does not appear in the configuration. In all other
> > cases of tsc_mode, 'vtsc_khz' option is just ignored.
> > 
> > Another purpose of adding this option is to keep vcpu's TSC rate across
> > guest reboot. In existing code, a new domain is created from the
> > configuration of the previous domain which was just rebooted. vcpu's TSC
> > rate is not stored in the configuration and the host TSC rate is the
> > used as vcpu's TSC rate. This works fine unless the previous domain was
> > migrated from another host machine with a different host TSC rate than
> > the current one.
> 
> I understand why this is necessary over a migration, but why is it
> important to be able to retain the TSC rate across a reboot? What is the
> usecase there?
>

No usecase so far. Is 'making a virtual machine more like a physical
machine' reasonable here? (I suppose a physical machine retains TSC
rate across reboot)

> > Signed-off-by: Haozhong Zhang 
> > ---
> >  tools/libxl/libxl_types.idl |  1 +
> >  tools/libxl/libxl_x86.c |  4 +++-
> >  tools/libxl/xl_cmdimpl.c| 22 ++
> 
> The documentation should be patched at the same time. At least the xl.cfg
> manpage, but I think there is also a specific document about time and the
 ~~~
I think it's doc/misc/tscmode.txt? Will update it as well.

- Haozhong

> TSC which should also be updated.
>
> Ian.

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

Re: [Xen-devel] [PATCH v2 for Xen 4.6 0/6] Several PSR fixes in libxl

[Ian Campbell]

On Tue, 2015-09-29 at 10:33 +0100, Wei Liu wrote:
> Now the reasoning bits. Yes, I'm arguing with myself, :-)
> 
> We can of course fix it post-4.6, but the released APIs need to be
> maintained forever (even if it is in fact broken). That would definitely
> involve lots of compatibility cruft if we fix it post 4.6. 
> 
> This patch series is simple enough to reason about and has received
> adequate review from expert in the field, so I have hight confidence in
> it being correct.
> 
> I think the benefit of accepting it out-weights the downside.

Applied all 6 to staging and staging-4.6, with the one tweak discussed in
reply to #5.


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

Re: [Xen-devel] [PATCH 13/13] tools/libxl: Add 'vtsc_khz' option to set guest TSC rate

[Andrew Cooper]

On 29/09/15 11:28, Ian Campbell wrote:
> On Tue, 2015-09-29 at 11:24 +0100, Andrew Cooper wrote:
>> On 29/09/15 11:13, Haozhong Zhang wrote:
>>> On Tue, Sep 29, 2015 at 11:04:14AM +0100, Ian Campbell wrote:
 On Mon, 2015-09-28 at 15:13 +0800, Haozhong Zhang wrote:
> This patch adds an option 'vtsc_khz' to allow users to set vcpu's
> TSC
> rate in KHz. In the case that tsc_mode = 'default', the default
> value of
> 'vtsc_khz' option is the host TSC rate which is used when
> 'vtsc_khz'
> option is set to 0 or does not appear in the configuration. In all
> other
> cases of tsc_mode, 'vtsc_khz' option is just ignored.
>
> Another purpose of adding this option is to keep vcpu's TSC rate
> across
> guest reboot. In existing code, a new domain is created from the
> configuration of the previous domain which was just rebooted.
> vcpu's TSC
> rate is not stored in the configuration and the host TSC rate is
> the
> used as vcpu's TSC rate. This works fine unless the previous domain
> was
> migrated from another host machine with a different host TSC rate
> than
> the current one.
 I understand why this is necessary over a migration, but why is it
 important to be able to retain the TSC rate across a reboot? What is
 the
 usecase there?

>>> No usecase so far. Is 'making a virtual machine more like a physical
>>> machine' reasonable here? (I suppose a physical machine retains TSC
>>> rate across reboot)
>> There are situations such as altering firmware settings which can cause
>> the TSC rate to differ across reboot.  I don't see any reason to try and
>> maintain it across VM reboots.
> Right. If it happens to come for free as a side effect of making it work
> for migration then fine.
>
> But it seems to me that tsc rate could/should be in the hypervisors save
> blob and require no interaction with the toolstack once it is latched when
> the domain is built.

There are a lot of blobs which fall into this category.  Others are
cpuid policy and guest-MSRs.  I have a longterm plan to fix them, which
is under very slow progress.

~Andrew

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

Re: [Xen-devel] [PATCH v1 3/8] xen/arm: Support sign-extension for every read access

[Ian Campbell]

On Mon, 2015-09-28 at 17:42 +0100, Julien Grall wrote:

> > > + */
> > > +if ( info->dabt.sign && (*r & (1UL << (size - 1)) ))
> > > +{
> > > +/*
> > > + * We are relying on register_t as the same size as
> > > + * an unsigned long or order to keep the 32bit some smaller
> > 
> > "order"? I'm not sure what you meant here so I can't suggest an
> > alternative.
> 
> hmmm... the end of the comment is badly written :/. I wanted to say
> "We are relying on register_t using the same size as and unsigned long
> in order to keep the 32-bit assembly code smaller"

"as an unsigned long", then it works.

> 
> > 
> > > + */
> > > +BUILD_BUG_ON(sizeof(register_t) != sizeof(unsigned long));
> > > +*r |= (~0UL) << size;
> > 
> > I think here and in the initial if you need to be careful of the case
> > where
> > size == 32 (on arm32) or == 64 (on arm64), since a shift by >= the size
> > of
> > the variable is, I think, undefined behaviour.
> > 
> > It's also a waste of time sign extending in that case.
> 
> From the spec, dabt.sign is only set when smaller size than the register
> size. For instance for ARMv7 spec (ARM DDI 0406C.b page B3-1433):
> 
> "SSE, Syndrome sign extend. For a byte or halfword load operation,
> indicates whether
> the data item must be sign extended. [...] For all other operations this
> bit is 0."
> 
> So we don't have to worry about waste of time and undefined behavior.
> Note that mention it in the commit message. Maybe it wasn't clear enough?

It's a fairly oblique mention in the commit message, so I think it indeed
wasn't explicit enough.

I'm unsure if we need to worry about the fact that the compiler does't know
about that bit of the spec, so it might assume that size could be >=32 or
64 and do something unhelpful. Probably not.

Ian.

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

Re: [Xen-devel] [PATCH 3/4 RFC] x86/p2m: use large pages for MMIO mappings

[Jan Beulich]

>>> On 29.09.15 at 13:33,  wrote:
> On 15/09/15 08:34, Jan Beulich wrote:
>> RFC reasons:
>> - ARM side unimplemented (and hence libxc for now made cope with both
>>   models), the main issue (besides my inability to test any change
>>   there) being the many internal uses of map_mmio_regions())
> 
> map_mmio_regions is used in ARM to map all the device memory in a guest.
> We expect this function to map everything at once when called during
> DOM0 build and/or when a guest is created (used to map the interrupt
> controller).
> 
> I would rather prefer to avoid introducing specific helpers with
> slightly different behavior (i.e one is only mapping N page, the other
> everything).
> 
> What about extending map_mmio_regions to take a parameter telling if we
> want to limit the number of mapping in a single invocation?

Sure an option, albeit something that would be sufficient to be
done in ARM specific code, albeit the only user using variable
length is map_range_to_domain(). All the others, using fixed
lengths up to 32 pages, would implicitly get everything done at
once as long as the threshold is >= 32.

Jan


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

Re: [Xen-devel] [PATCH v2 for Xen 4.6 3/6] tools/libxl: return socket id from libxl_psr_cat_get_l3_info

[Dario Faggioli]

On Tue, 2015-09-29 at 15:49 +0800, Chao Peng wrote:
> The entries returned from libxl_psr_cat_get_l3_info are assumed
> to be socket-continuous. But this is not true in the hotplug case.
> 
> This patch gets the socket bitmap for all the sockets on the system
> first and stores the socket id in the structure libxl_psr_cat_info in
> libxl_psr_cat_get_l3_info. The xl or similar consumers then can
> display
> socket information correctly.
> 
> Signed-off-by: Chao Peng 
>
Reviewed-by: Dario Faggioli 

Thanks and Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R 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 13/13] tools/libxl: Add 'vtsc_khz' option to set guest TSC rate

[Wei Liu]

On Tue, Sep 29, 2015 at 08:40:23AM +0800, Haozhong Zhang wrote:
> On Mon, Sep 28, 2015 at 03:19:25PM +0100, Wei Liu wrote:
> > On Mon, Sep 28, 2015 at 03:13:58PM +0800, Haozhong Zhang wrote:
> > > This patch adds an option 'vtsc_khz' to allow users to set vcpu's TSC
> > > rate in KHz. In the case that tsc_mode = 'default', the default value of
> > > 'vtsc_khz' option is the host TSC rate which is used when 'vtsc_khz'
> > > option is set to 0 or does not appear in the configuration. In all other
> > > cases of tsc_mode, 'vtsc_khz' option is just ignored.
> > > 
> > > Another purpose of adding this option is to keep vcpu's TSC rate across
> > > guest reboot. In existing code, a new domain is created from the
> > > configuration of the previous domain which was just rebooted. vcpu's TSC
> > > rate is not stored in the configuration and the host TSC rate is the
> > > used as vcpu's TSC rate. This works fine unless the previous domain was
> > > migrated from another host machine with a different host TSC rate than
> > > the current one.
> > > 
> > > Signed-off-by: Haozhong Zhang 
> > > ---
> > >  tools/libxl/libxl_types.idl |  1 +
> > >  tools/libxl/libxl_x86.c |  4 +++-
> > >  tools/libxl/xl_cmdimpl.c| 22 ++
> > >  3 files changed, 26 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > > index 9f6ec00..91cb0be 100644
> > > --- a/tools/libxl/libxl_types.idl
> > > +++ b/tools/libxl/libxl_types.idl
> > > @@ -413,6 +413,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
> > >  ("vcpu_soft_affinity", Array(libxl_bitmap, 
> > > "num_vcpu_soft_affinity")),
> > >  ("numa_placement",  libxl_defbool),
> > >  ("tsc_mode",libxl_tsc_mode),
> > > +("vtsc_khz",uint32),
> > 
> > This field should be in arch-specific substructure, i.e. "hvm".
> >
> 
> Julien also pointed out this and suggested to moving to an
> arch-specific substructure. I'm going to add a new substructure
> "arch_x86" and move "vtsc_khz" there. Is this good for you?
> 

My initial thought was that this was a feature of HVM. I don't recollect
why I got that idea. I could be wrong.  Does it work with (or intent to
work with) PV too?  If yes, adding it to arch_x86 would be appropriate.
If not, hvm specific is good enough.  Please correct my
misunderstanding, I'm definitely no expert on x86.  

> > >  ("max_memkb",   MemKB),
> > >  ("target_memkb",MemKB),
> > >  ("video_memkb", MemKB),
> > > diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
> > > index 896f34c..7baaee4 100644
> > > --- a/tools/libxl/libxl_x86.c
> > > +++ b/tools/libxl/libxl_x86.c
> > > @@ -276,6 +276,7 @@ int libxl__arch_domain_create(libxl__gc *gc, 
> > > libxl_domain_config *d_config,
> > >  {
> > >  int ret = 0;
> > >  int tsc_mode;
> > > +uint32_t vtsc_khz;
> > >  uint32_t rtc_timeoffset;
> > >  libxl_ctx *ctx = libxl__gc_owner(gc);
> > >  
> > > @@ -300,7 +301,8 @@ int libxl__arch_domain_create(libxl__gc *gc, 
> > > libxl_domain_config *d_config,
> > >  default:
> > >  abort();
> > >  }
> > > -xc_domain_set_tsc_info(ctx->xch, domid, tsc_mode, 0, 0, 0);
> > > +vtsc_khz = d_config->b_info.vtsc_khz;
> > > +xc_domain_set_tsc_info(ctx->xch, domid, tsc_mode, 0, vtsc_khz, 0);
> > >  if (libxl_defbool_val(d_config->b_info.disable_migrate))
> > >  xc_domain_disable_migrate(ctx->xch, domid);
> > >  rtc_timeoffset = d_config->b_info.rtc_timeoffset;
> > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > > index 2706759..5fabda7 100644
> > > --- a/tools/libxl/xl_cmdimpl.c
> > > +++ b/tools/libxl/xl_cmdimpl.c
> > > @@ -1462,6 +1462,28 @@ static void parse_config_data(const char 
> > > *config_source,
> > >  }
> > >  }
> > >  
> > > +/* "vtsc_khz" option works only if "tsc_mode" option is
> > > + * "default". In this case, if "vtsc_khz" option is set to 0, we
> > > + * will reset it to the host TSC rate. In all other cases, we just
> > > + * ignore any given value and always set it to 0.
> > > + */
> > > +if (!xlu_cfg_get_long(config, "vtsc_khz", , 0))
> > > +b_info->vtsc_khz = l;
> > > +if (b_info->tsc_mode == LIBXL_TSC_MODE_DEFAULT) {
> > > +if (b_info->vtsc_khz == 0) {
> > > +libxl_physinfo physinfo;
> > > +if (!libxl_get_physinfo(ctx, ))
> > > +b_info->vtsc_khz = physinfo.cpu_khz;
> > > +else
> > > +fprintf(stderr, "WARNING: cannot get host TSC rate.\n");
> > > +}
> > 
> > And this hunk (the decision making bit) should be in libxl, not xl.
> > 
> > Consider there are other toolstack that uses libxl, say libvirt.
> >
> 
> Good to know this.
> 
> I'm going to move it to libxl__arch_domain_create() where
> b_info->vtsc_khz is used.
> 

Right, that seems appropriate.

Wei.