Re: [Xen-devel] [Minios-devel] [PATCH MINI-OS v2] xenbus: notify the other end when necessary

[Wei Liu]

On Mon, Oct 26, 2015 at 01:52:47PM +0100, Samuel Thibault wrote:
> Wei Liu, le Mon 26 Oct 2015 12:47:56 +, a écrit :
> > -if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < 
> > sizeof(msg))
> > +if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < 
> > sizeof(msg)) {
> > +notify_remote_via_evtchn(start_info.store_evtchn);
> >  break;
> > +}
> 
> >  if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons <
> > -sizeof(msg) + msg.len)
> > +sizeof(msg) + msg.len) {
> > +notify_remote_via_evtchn(start_info.store_evtchn);
> 
> Actually thinking...  In principle we should not need these two
> notifications: we have already notified last time we consumed a message.
> Notifying again shouldn't be bringing anything new.  Do you actually see
> a difference with these?
> 

Yes. The ring still gets stalled somehow without those two
notifications.

Wei.

> Samuel
> 
> ___
> Minios-devel mailing list
> minios-de...@lists.xenproject.org
> http://lists.xenproject.org/cgi-bin/mailman/listinfo/minios-devel

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

[Xen-devel] Make the Wiki 4.6 ready: Xen Project Document Day is Wednesday

[Russ Pavlicek]

OUR THEME OF THE MONTH: "Ready for 4.6"

This month, we continue with the effort to make the Wiki reflect the
realities of the 4.6 release.  Many pages need reviewing and may need
updating.  Main tasks include:

- 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 v11 3/3] iommu: add rmrr Xen command line option for extra rmrrs

[Jan Beulich]

>>> On 22.10.15 at 19:13,  wrote:
> From: Elena Ufimtseva 
> 
> On some platforms RMRR regions may be not specified in ACPI and thus will not
> be mapped 1:1 in dom0.

I think this may be misleading to readers: It sounds as if there was
the option for RMRRs to not be specified in ACPI tables, while in
fact this is a firmware bug. How about "On some platforms firmware
fails to specify RMRR regions may in ACPI tables, and thus those
regions will not be mapped in dom0 or guests the respective device(s)
get passed through to"?

> +static void __init add_extra_rmrr(void)
> +{
> +struct acpi_rmrr_unit *acpi_rmrr;
> +struct acpi_rmrr_unit *rmrru;
> +unsigned int dev, seg, i;
> +unsigned long pfn;
> +bool_t overlap;
> +
> +for ( i = 0; i < nr_rmrr; i++ )
> +{
> +if ( extra_rmrr_units[i].base_pfn > extra_rmrr_units[i].end_pfn )
> +{
> +printk(XENLOG_ERR VTDPREFIX
> +   "Invalid RMRR Range "ERMRRU_FMT"\n",
> +   ERMRRU_ARG(extra_rmrr_units[i]));
> +continue;
> +}
> +
> +if ( extra_rmrr_units[i].end_pfn - extra_rmrr_units[i].base_pfn >=
> + MAX_EXTRA_RMRR_PAGES )
> +{
> +printk(XENLOG_ERR VTDPREFIX
> +   "RMRR range "ERMRRU_FMT" exceeds 
> "__stringify(MAX_EXTRA_RMRR_PAGES)" pages\n",
> +   ERMRRU_ARG(extra_rmrr_units[i]));
> +continue;
> +}
> +
> +overlap = 0;
> +list_for_each_entry(rmrru, &acpi_rmrr_units, list)
> +{
> +if ( pfn_to_paddr(extra_rmrr_units[i].base_pfn ) < 
> rmrru->end_address &&

Stray blank inside the inner parentheses.

> + rmrru->base_address < 
> pfn_to_paddr(extra_rmrr_units[i].end_pfn + 1) )
> +{
> +printk(XENLOG_ERR VTDPREFIX
> +   "Overlapping RMRRs: "ERMRRU_FMT" and [%lx - %lx]\n",

ERMRRU_FMT doesn't have any blanks inside the square brackets,
so I'd suggest the other format to nt have them either.

> +   ERMRRU_ARG(extra_rmrr_units[i]),
> +   paddr_to_pfn(rmrru->base_address),
> +   paddr_to_pfn(rmrru->end_address));
> +overlap = 1;
> +break;
> +}
> +}
> +/* Dont add overlapping RMRR */

"Don't" and missing full stop.

> +if ( overlap )
> +continue;
> +
> +pfn = extra_rmrr_units[i].base_pfn;
> +do
> +{
> +if ( !mfn_valid(pfn) || (pfn >> (paddr_bits - PAGE_SHIFT)) )

Actually I think the right side is redundant with the max_pfn check
mfn_valid() does.

> +{
> +printk(XENLOG_ERR VTDPREFIX
> +   "Invalid pfn in RMRR range "ERMRRU_FMT"\n",
> +   ERMRRU_ARG(extra_rmrr_units[i]));
> +break;

Wrong indentation.

> +}
> +
> +} while ( pfn++ < extra_rmrr_units[i].end_pfn );

Stray blank line before the end of the do/while body.

> +
> +/* Invalid pfn in range as the loop ended before end_pfn was 
> reached. */
> +if ( pfn <= extra_rmrr_units[i].end_pfn )
> +continue;
> +
> +acpi_rmrr = xzalloc(struct acpi_rmrr_unit);
> +if ( !acpi_rmrr )
> +return;
> +
> +acpi_rmrr->scope.devices = xmalloc_array(u16,
> + 
> extra_rmrr_units[i].dev_count);
> +if ( !acpi_rmrr->scope.devices )
> +{
> +xfree(acpi_rmrr);
> +return;
> +}
> +
> +seg = 0;
> +for ( dev = 0; dev < extra_rmrr_units[i].dev_count; dev++ )
> +{
> +acpi_rmrr->scope.devices[dev] = extra_rmrr_units[i].sbdf[dev];
> +seg = seg | PCI_SEG(extra_rmrr_units[i].sbdf[dev]);

|=

> +static void __init parse_rmrr_param(const char *str)
> +{
> +const char *s = str, *cur, *stmp;
> +unsigned int seg, bus, dev, func;
> +unsigned long start, end;
> +
> +do {
> +start = simple_strtoul(cur = s, &s, 0);
> +if ( cur == s )
> +break;
> +
> +if ( *s == '-' )
> +{
> +end = simple_strtoul(cur = s + 1, &s, 0);
> +if ( cur == s )
> +break;
> +}
> +else
> +end = start;
> +
> +extra_rmrr_units[nr_rmrr].base_pfn = start;
> +extra_rmrr_units[nr_rmrr].end_pfn = end;
> +extra_rmrr_units[nr_rmrr].dev_count = 0;

The last assignment isn't really needed.

> +if ( *s != '=' )
> +continue;
> +
> +do {
> +bool_t default_segment = 0;
> +
> +if ( *s == ';' )
> +break;

Can this if() ever be true? *s == '=' on the first iteration, and *s == ','
on any subsequent one afaics.

> +stmp = __parse_pci(s + 1, &seg, &bus, &dev, &func, 
> &default_segment);
> +if ( !stmp )
> +  

Re: [Xen-devel] [PATCH v1 1/5] xsplice: Design document.

[Ross Lagerwall]

On 10/26/2015 12:01 PM, Martin Pohlack wrote:

On 16.09.2015 23:01, Konrad Rzeszutek Wilk wrote:

[...]


+### xsplice_patch
+
+This structure has the binary code (or data) to be patched. Depending on the
+type it can either an inline patch (data or text) or require an relocation
+change (which requires a trampoline). Naturally it also points to a blob
+of the binary data to patch in, and the size of the patch.


On the Xen developer summit we agreed to start with a minimal approach
first.  Based on looking at the last ~50 XSA patches, I do think we can
do *without* the in-place patching and would propose to tackle this
approach later or not at all.



Indeed. The prototype V1 that I posted last Friday works at a function 
level. I guess the only reason to make anything more complicated would 
be to patch the asm code in entry.S.


--
Ross Lagerwall

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

Re: [Xen-devel] [PATCH v11 1/3] iommu VT-d: separate rmrr addition function

[Jan Beulich]

>>> On 22.10.15 at 19:13,  wrote:
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -583,6 +583,68 @@ out:
>  return ret;
>  }
>  
> +static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
> +{
> +bool_t ignore = 0;
> +unsigned int i = 0;
> +int ret = 0;
> +
> +/* Skip checking if segment is not accessible yet. */
> +if ( !pci_known_segment(rmrru->segment) )
> +i = UINT_MAX;
> +
> +for ( ; i < rmrru->scope.devices_cnt; i++ )
> +{
> +u8 b = PCI_BUS(rmrru->scope.devices[i]);
> +u8 d = PCI_SLOT(rmrru->scope.devices[i]);
> +u8 f = PCI_FUNC(rmrru->scope.devices[i]);
> +
> +if ( pci_device_detect(rmrru->segment, b, d, f) == 0 )
> +{
> +dprintk(XENLOG_WARNING VTDPREFIX,
> +" Non-existent device (%04x:%02x:%02x.%u) is reported"
> +" in RMRR (%"PRIx64", %"PRIx64")'s scope!\n",
> +rmrru->segment, b, d, f,
> +rmrru->base_address, rmrru->end_address);
> +ignore = 1;
> +}
> +else
> +{
> +ignore = 0;
> +break;
> +}
> +}
> +
> +if ( ignore )
> +{
> +dprintk(XENLOG_WARNING VTDPREFIX,
> +"  Ignore the RMRR (%"PRIx64", %"PRIx64") due to "
> +"devices under its scope are not PCI discoverable!\n",
> +rmrru->base_address, rmrru->end_address);

Broken indentation, but since everything else looks okay this can of
course be fixed up while committing, provided we can get a VT-d
maintainer ack.

Jan


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

[Xen-devel] Ping: [PATCH v3] x86/HVM: correct page dirty marking in hvm_map_guest_frame_rw()

[Jan Beulich]

>>> On 19.10.15 at 16:53,  wrote:
> --- a/tools/libxc/xc_sr_save.c
> +++ b/tools/libxc/xc_sr_save.c
> @@ -537,7 +537,8 @@ static int suspend_and_send_dirty(struct
>  if ( xc_shadow_control(
>   xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_CLEAN,
>   HYPERCALL_BUFFER(dirty_bitmap), ctx->save.p2m_size,
> - NULL, 0, &stats) != ctx->save.p2m_size )
> + NULL, XEN_DOMCTL_SHADOW_LOGDIRTY_FINAL, &stats) !=
> + ctx->save.p2m_size )
>  {
>  PERROR("Failed to retrieve logdirty bitmap");
>  rc = -1;

Tools maintainers, could I please get an ack or otherwise on this
adjustment?

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -208,6 +208,13 @@ struct xen_domctl_getpageframeinfo3 {
>*/
>  #define XEN_DOMCTL_SHADOW_ENABLE_EXTERNAL  (1 << 4)
>  
> +/* Mode flags for XEN_DOMCTL_SHADOW_OP_{CLEAN,PEEK}. */
> + /*
> +  * This is the final iteration: Requesting to include pages mapped
> +  * writably by the hypervisor in the dirty bitmap.
> +  */
> +#define XEN_DOMCTL_SHADOW_LOGDIRTY_FINAL   (1 << 0)
> +
>  struct xen_domctl_shadow_op_stats {
>  uint32_t fault_count;
>  uint32_t dirty_count;
> @@ -219,8 +226,9 @@ struct xen_domctl_shadow_op {
>  /* IN variables. */
>  uint32_t   op;   /* XEN_DOMCTL_SHADOW_OP_* */
>  
> -/* OP_ENABLE */
> -uint32_t   mode; /* XEN_DOMCTL_SHADOW_ENABLE_* */
> +/* OP_ENABLE: XEN_DOMCTL_SHADOW_ENABLE_* */
> +/* OP_PEAK / OP_CLEAN: XEN_DOMCTL_SHADOW_LOGDIRTY_* */
> +uint32_t   mode;
>  
>  /* OP_GET_ALLOCATION / OP_SET_ALLOCATION */
>  uint32_t   mb;   /* Shadow memory allocation in MB */

"REST" maintainers, could I please get an ack or otherwise on this
interface change?

Thanks, Jan



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

[Xen-devel] Ping: [PATCH] use clear_domain_page() instead of open coding it

[Jan Beulich]

>>> On 19.10.15 at 16:51,  wrote:

"REST" maintainers, could I please get an ack or otherwise on this
common code change?

Thanks, Jan

> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -1959,22 +1959,16 @@ __initcall(pagealloc_keyhandler_init);
>  
>  void scrub_one_page(struct page_info *pg)
>  {
> -void *p;
> -
>  if ( unlikely(pg->count_info & PGC_broken) )
>  return;
>  
> -p = __map_domain_page(pg);
> -
>  #ifndef NDEBUG
>  /* Avoid callers relying on allocations returning zeroed pages. */
> -memset(p, 0xc2, PAGE_SIZE);
> +unmap_domain_page(memset(__map_domain_page(pg), 0xc2, PAGE_SIZE));
>  #else
>  /* For a production build, clear_page() is the fastest way to scrub. */
> -clear_page(p);
> +clear_domain_page(_mfn(page_to_mfn(pg)));
>  #endif
> -
> -unmap_domain_page(p);
>  }
>  
>  static void dump_heap(unsigned char key)




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

Re: [Xen-devel] [PATCH v2 4/5] x86/mm: build map_domain_gfn() just once

[Andrew Cooper]

On 26/10/15 11:52, Jan Beulich wrote:
> It doesn't depend on GUEST_PAGING_LEVELS. Moving the function to p2m.c
> at once allows a bogus #define/#include pair to be removed from
> hap/nested_ept.c.
>
> 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 MINI-OS v2] xenbus: notify the other end when necessary

[Samuel Thibault]

Wei Liu, le Mon 26 Oct 2015 12:47:56 +, a écrit :
> -if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < 
> sizeof(msg))
> +if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < 
> sizeof(msg)) {
> +notify_remote_via_evtchn(start_info.store_evtchn);
>  break;
> +}

>  if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons <
> -sizeof(msg) + msg.len)
> +sizeof(msg) + msg.len) {
> +notify_remote_via_evtchn(start_info.store_evtchn);

Actually thinking...  In principle we should not need these two
notifications: we have already notified last time we consumed a message.
Notifying again shouldn't be bringing anything new.  Do you actually see
a difference with these?

Samuel

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

Re: [Xen-devel] [PATCH v2 3/5] x86/mm: override stored file names for multiply built sources

[Andrew Cooper]

On 26/10/15 11:51, Jan Beulich wrote:
> To make it possible to tell apart the static symbols therein, use their
> object file names instead of their source ones.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

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

[Xen-devel] [PATCH MINI-OS v2] xenbus: notify the other end when necessary

[Wei Liu]

The xenbus thread didn't send notification to other end when it expected
more data or consumed responses, which led to stalling the ring from
time to time.

This is the culprit that guest was less responsive when using stubdom
because the device model was stalled.

Fix this by sending notification to the other end at the right places.

Signed-off-by: Wei Liu 
---
Cc: Ian Campbell 
Cc: Ian Jackson 
Cc: Samuel Thibault 

v2: add two more mb()s.

With this path I can migrate a guest with stubdom a few thousand times
without any issue, while before I could easily trigger time out
within a few iterations. This should make OSSTest stubdom test case more
reliable.

Ian, this is a patch suitable for backporting to 4.6. It's good time
branch mini-os now.
---
 xenbus/xenbus.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
index 4613ed6..bc669f2 100644
--- a/xenbus/xenbus.c
+++ b/xenbus/xenbus.c
@@ -205,8 +205,10 @@ static void xenbus_thread_func(void *ign)
 prod = xenstore_buf->rsp_prod;
 DEBUG("Rsp_cons %d, rsp_prod %d.\n", xenstore_buf->rsp_cons,
 xenstore_buf->rsp_prod);
-if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < sizeof(msg))
+if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < sizeof(msg)) 
{
+notify_remote_via_evtchn(start_info.store_evtchn);
 break;
+}
 rmb();
 memcpy_from_ring(xenstore_buf->rsp,
 &msg,
@@ -217,8 +219,10 @@ static void xenbus_thread_func(void *ign)
 xenstore_buf->rsp_prod - xenstore_buf->rsp_cons,
 msg.req_id);
 if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons <
-sizeof(msg) + msg.len)
+sizeof(msg) + msg.len) {
+notify_remote_via_evtchn(start_info.store_evtchn);
 break;
+}
 
 DEBUG("Message is good.\n");
 
@@ -237,6 +241,7 @@ static void xenbus_thread_func(void *ign)
event->path = data;
event->token = event->path + strlen(event->path) + 1;
 
+mb();
 xenstore_buf->rsp_cons += msg.len + sizeof(msg);
 
 for (watch = watches; watch; watch = watch->next)
@@ -262,9 +267,13 @@ static void xenbus_thread_func(void *ign)
 req_info[msg.req_id].reply,
 MASK_XENSTORE_IDX(xenstore_buf->rsp_cons),
 msg.len + sizeof(msg));
+mb();
 xenstore_buf->rsp_cons += msg.len + sizeof(msg);
 wake_up(&req_info[msg.req_id].waitq);
 }
+
+wmb();
+notify_remote_via_evtchn(start_info.store_evtchn);
 }
 }
 }
-- 
2.1.4


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

Re: [Xen-devel] [PATCH 1/2] xen/x86: Helpers for cpu feature manipuation

[Andrew Cooper]

On 26/10/15 12:06, Jan Beulich wrote:
 On 26.10.15 at 12:17,  wrote:
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -202,7 +202,7 @@ static void __init early_cpu_detect(void)
>>  c->x86_mask = tfms & 15;
>>  cap0 &= ~cleared_caps[0];
>>  cap4 &= ~cleared_caps[4];
>> -if (cap0 & (1<<19))
>> +if (cap0 & cpufeat_mask(X86_FEATURE_CLFLSH))
> This one's particularly well spotted! Thanks!

I doubt I have found all instances like this.  I only found this one
because of the associated changes in the subsequent patch.

I will be fixing them as I find them.

~Andrew

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

Re: [Xen-devel] [Minios-devel] [PATCH MINI-OS] xenbus: notify the other end when necessary

[Wei Liu]

On Mon, Oct 26, 2015 at 01:33:12PM +0100, Samuel Thibault wrote:
> Wei Liu, le Mon 26 Oct 2015 12:30:28 +, a écrit :
> > On Mon, Oct 26, 2015 at 01:21:51PM +0100, Samuel Thibault wrote:
> > > Wei Liu, le Mon 26 Oct 2015 12:14:43 +, a écrit :
> > > > In my patch, mini-os notifies remote whenever it consumes a message,
> > > > which I think it's slightly better because backend can start putting
> > > > things in the ring as mini-os processes them.
> > > 
> > > That makes more notifications, but that can lead to more pipelining
> > > indeed.  That's what the Linux driver does, so let's do the same.
> > > 
> > > Also, I'm realizing: aren't we missing a full memory barrier between
> > > the memcpy_from_ring call and xenstore_buf->rsp_cons += ? (in the two
> > > places) We need to make sure to have finished copying from the ring
> > > before writing the new rsp_cons.
> > > 
> > 
> > You're right.
> > 
> > I think we should just turn that wmb() into two mb()s and place them
> > before xenstore_buf->rsp_cons +=.
> 
> We *also* need some barrier between rsp_cons += and the notification,
> otherwise the notified domain may miss the rsp_cons update and thus
> believe it was a spurious notification.
> 

Actually notify_remote_via_evtchn normally implies a mb(). But I think
I'd stay on the safe side. I will have a wmb() there. :-)

Thanks for your review. V2 coming soon.

Wei.

> Samuel
> 
> ___
> Minios-devel mailing list
> minios-de...@lists.xenproject.org
> http://lists.xenproject.org/cgi-bin/mailman/listinfo/minios-devel

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

Re: [Xen-devel] [PATCH MINI-OS] xenbus: notify the other end when necessary

[Samuel Thibault]

Wei Liu, le Mon 26 Oct 2015 12:30:28 +, a écrit :
> On Mon, Oct 26, 2015 at 01:21:51PM +0100, Samuel Thibault wrote:
> > Wei Liu, le Mon 26 Oct 2015 12:14:43 +, a écrit :
> > > In my patch, mini-os notifies remote whenever it consumes a message,
> > > which I think it's slightly better because backend can start putting
> > > things in the ring as mini-os processes them.
> > 
> > That makes more notifications, but that can lead to more pipelining
> > indeed.  That's what the Linux driver does, so let's do the same.
> > 
> > Also, I'm realizing: aren't we missing a full memory barrier between
> > the memcpy_from_ring call and xenstore_buf->rsp_cons += ? (in the two
> > places) We need to make sure to have finished copying from the ring
> > before writing the new rsp_cons.
> > 
> 
> You're right.
> 
> I think we should just turn that wmb() into two mb()s and place them
> before xenstore_buf->rsp_cons +=.

We *also* need some barrier between rsp_cons += and the notification,
otherwise the notified domain may miss the rsp_cons update and thus
believe it was a spurious notification.

Samuel

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

Re: [Xen-devel] [PATCH MINI-OS] xenbus: notify the other end when necessary

[Wei Liu]

On Mon, Oct 26, 2015 at 01:21:51PM +0100, Samuel Thibault wrote:
> Wei Liu, le Mon 26 Oct 2015 12:14:43 +, a écrit :
> > In my patch, mini-os notifies remote whenever it consumes a message,
> > which I think it's slightly better because backend can start putting
> > things in the ring as mini-os processes them.
> 
> That makes more notifications, but that can lead to more pipelining
> indeed.  That's what the Linux driver does, so let's do the same.
> 
> Also, I'm realizing: aren't we missing a full memory barrier between
> the memcpy_from_ring call and xenstore_buf->rsp_cons += ? (in the two
> places) We need to make sure to have finished copying from the ring
> before writing the new rsp_cons.
> 

You're right.

I think we should just turn that wmb() into two mb()s and place them
before xenstore_buf->rsp_cons +=.

Wei.

> Samuel

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

Re: [Xen-devel] [PATCH MINI-OS] xenbus: notify the other end when necessary

[Samuel Thibault]

Wei Liu, le Mon 26 Oct 2015 12:14:43 +, a écrit :
> In my patch, mini-os notifies remote whenever it consumes a message,
> which I think it's slightly better because backend can start putting
> things in the ring as mini-os processes them.

That makes more notifications, but that can lead to more pipelining
indeed.  That's what the Linux driver does, so let's do the same.

Also, I'm realizing: aren't we missing a full memory barrier between
the memcpy_from_ring call and xenstore_buf->rsp_cons += ? (in the two
places) We need to make sure to have finished copying from the ring
before writing the new rsp_cons.

Samuel

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

Re: [Xen-devel] [PATCH MINI-OS] xenbus: notify the other end when necessary

[Wei Liu]

On Mon, Oct 26, 2015 at 01:02:45PM +0100, Samuel Thibault wrote:
> Hello,
> 
> Indeed, notification seems to have been missing since basically 2006...
> 
> Wei Liu, le Mon 26 Oct 2015 09:47:48 +, a écrit :
> > diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
> > index 4613ed6..7451161 100644
> > --- a/xenbus/xenbus.c
> > +++ b/xenbus/xenbus.c
> > @@ -205,8 +205,10 @@ static void xenbus_thread_func(void *ign)
> >  prod = xenstore_buf->rsp_prod;
> >  DEBUG("Rsp_cons %d, rsp_prod %d.\n", xenstore_buf->rsp_cons,
> >  xenstore_buf->rsp_prod);
> > -if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < 
> > sizeof(msg))
> > +if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < 
> > sizeof(msg)) {
> > +notify_remote_via_evtchn(start_info.store_evtchn);
> >  break;
> > +}
> >  rmb();
> >  memcpy_from_ring(xenstore_buf->rsp,
> >  &msg,
> > @@ -217,8 +219,10 @@ static void xenbus_thread_func(void *ign)
> >  xenstore_buf->rsp_prod - xenstore_buf->rsp_cons,
> >  msg.req_id);
> >  if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons <
> > -sizeof(msg) + msg.len)
> > +sizeof(msg) + msg.len) {
> > +notify_remote_via_evtchn(start_info.store_evtchn);
> >  break;
> > +}
> >  
> >  DEBUG("Message is good.\n");
> >  
> > @@ -265,6 +269,9 @@ static void xenbus_thread_func(void *ign)
> >  xenstore_buf->rsp_cons += msg.len + sizeof(msg);
> >  wake_up(&req_info[msg.req_id].waitq);
> >  }
> > +
> > +wmb();
> > +notify_remote_via_evtchn(start_info.store_evtchn);
> >  }
> >  }
> >  }
> 
> The wmb() position seems right, but the notification could be put a bit
> later, after the exit of the while(1) loop.  That'd make it factorized
> for all cases were the processing could want to stop, and make it quite
> naturally enough just before the wait_event call, so something like
> below (untested).
> 
> Samuel
> 
> diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
> index 4613ed6..858aa98 100644
> --- a/xenbus/xenbus.c
> +++ b/xenbus/xenbus.c
> @@ -265,7 +265,10 @@ static void xenbus_thread_func(void *ign)
>  xenstore_buf->rsp_cons += msg.len + sizeof(msg);
>  wake_up(&req_info[msg.req_id].waitq);
>  }
> +
> +wmb();
>  }
> +notify_remote_via_evtchn(start_info.store_evtchn);

I am sure your patch works too but there is a subtle difference.  In my
patch, mini-os notifies remote whenever it consumes a message, which I
think it's slightly better because backend can start putting things in
the ring as mini-os processes them.

Wei.

>  }
>  }
>  

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

Re: [Xen-devel] [PATCH v1 1/5] xsplice: Design document.

[Jan Beulich]

>>> On 26.10.15 at 13:01,  wrote:
> On 16.09.2015 23:01, Konrad Rzeszutek Wilk wrote:
>> +The convention for file-type symbols (that would allow to map many
>> +symbols to their compilation unit) says that only the basename (i.e.,
>> +without directories) is embedded.  This creates another layer of
>> +confusion for duplicate file names in the build tree.
>> +
>> +That would have to be resolved.
> 
> Another approach here would be to qualify symbol names by the
> object-file name which contains them + a unique path component, for example:
> 
> drivers/pci/pci.o
> arch/x86/pci.o
> arch/x86/x86_64/pci.o

I wouldn't want to do this uniformly. A patch series doing this where
needed has been proposed already:
http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02702.html

Jan


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

Re: [Xen-devel] [PATCH 1/2] xen/x86: Helpers for cpu feature manipuation

[Jan Beulich]

>>> On 26.10.15 at 12:17,  wrote:
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -202,7 +202,7 @@ static void __init early_cpu_detect(void)
>   c->x86_mask = tfms & 15;
>   cap0 &= ~cleared_caps[0];
>   cap4 &= ~cleared_caps[4];
> - if (cap0 & (1<<19))
> + if (cap0 & cpufeat_mask(X86_FEATURE_CLFLSH))

This one's particularly well spotted! Thanks!

Jan


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

Re: [Xen-devel] ovmf fails to build in stagin-4.6

[Wei Liu]

On Mon, Oct 26, 2015 at 12:13:40PM +0100, Olaf Hering wrote:
> On Mon, Oct 26, Wei Liu wrote:
> 
> > Wait, so you're using gcc-5.1.1 but OVMF is reporting gcc-4.4 (see in
> > the path of output string), there might be another problem with
> > toolchain detection then.
> 
> As Linus said: detect old and known to be problematic, everything else has to
> be handled as "current". But see 
> tools/firmware/ovmf-dir-remote/OvmfPkg/build.sh
> 
> gcc_version=$(gcc -v 2>&1 | tail -1 | awk '{print $3}')
> case $gcc_version in
>   4.5.*)
> TARGET_TOOLS=GCC45
> ;;
>   4.6.*)
> TARGET_TOOLS=GCC46
> ;;
>   4.7.*)
> TARGET_TOOLS=GCC47
> ;;
>   4.8.*)
> TARGET_TOOLS=GCC48
> ;;
>   4.9.*|4.1[0-9].*)
> TARGET_TOOLS=GCC49
> ;;
>   *)
> TARGET_TOOLS=GCC44
> ;;
> esac

I think you have a stale repository. Try deleting ovmf-dir-remote ?

I can't comment how they handle toolchain detection though.

Wei.

> 
> Olaf

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

[Xen-devel] (XEN) Fatal machine check

[John Doe]

Hi, i'm getting this problem with linux 4.3.0-rcX branch.
Full log is attached. My hardware is based on intel skylake i7-6700k on
z170 chipset and it is working properly. It does boot correctly with
older kernels (3.12).
With mce=off on xen parameters it does boot unti during pci detection
where it hangs (see attached log).

I tried with both xen 4.4.2 and 4.6.0 with the same effect

[2.957615] VFS: Disk quotas d(XEN) Fatal machine check: MCE: Fatal
error happened on CPUs ff
(XEN)
(XEN) 
(XEN)
(XEN)The processor has reported a hardware error which cannot
(XEN)be recovered from.  Xen will now reboot the machine.
(XEN) mce.c:1533: Begin dump mc_info
(XEN) CPU0: Machine Check Exception:5
(XEN) Bank 4: ba0011000402[   0]
(XEN) CPU0: Machine Check Exception:5
(XEN) Bank 4: ba0011000402[   0]
(XEN) CPU1: Machine Check Exception:5
(XEN) Bank 4: ba0011000402[   0]
(XEN) CPU1: Machine Check Exception:5
(XEN) Bank 4: ba0011000402[   0]
(XEN) CPU2: Machine Check Exception:5
(XEN) Bank 4: ba0011000402[   0]
(XEN) CPU2: Machine Check Exception:5
(XEN) Bank 4: ba0011000402[   0]
(XEN) CPU3: Machine Check Exception:5
(XEN) Bank 4: ba0011000402[   0]
(XEN) CPU3: Machine Check Exception:5
(XEN) Bank 4: ba0011000402[   0]
(XEN) mce.c:1536: End dump mc_info, 8 mcinfo dumped
(XEN)
(XEN) 
(XEN) Panic on CPU 0:
(XEN) HARDWARE ERROR
(XEN) 
(XEN)
(XEN) Reboot in five seconds...

Any help?

Thanks,
John.
Loading Xen 4.4.2 ...
Loading Linux 4.3.0-1.pvops.qubes.x86_64 ...
Loading initial ramdisk ...
 Xen 4.4.2-7.fc21
(XEN) Xen version 4.4.2 (user@) (gcc (GCC) 4.9.2 20150212 (Red Hat 4.9.2-6)) debug=n Tue Oct 20 14:48:15 CEST 2015
(XEN) Latest ChangeSet: 
(XEN) Bootloader: GRUB 2.00
(XEN) Command line: placeholder iommu=pt iommu=1 unrestricted_guest=1 msi=1 swiotlb=force console=ttyS0 com1=115200,8n1 console=com1 dom0_mem=min:1024Mi loglvl=all apic_verbosity=debug e820t
(XEN) Video information:
(XEN)  VGA is text mode 80x25, font 8x16
(XEN)  VBE/DDC methods: V2; EDID transfer time: 1 seconds
(XEN) Disc information:
(XEN)  Found 1 MBR signatures
(XEN)  Found 1 EDD information structures
(XEN) Initial Xen-e820 RAM map:
(XEN)   - 0009c800 (usable)
(XEN)  0009c800 - 000a (reserved)
(XEN)  000e - 0010 (reserved)
(XEN)  0010 - 63911000 (usable)
(XEN)  63911000 - 63912000 (ACPI NVS)
(XEN)  63912000 - 6395c000 (reserved)
(XEN)  6395c000 - 639af000 (usable)
(XEN)  639af000 - 640b4000 (reserved)
(XEN)  640b4000 - 66b99000 (usable)
(XEN)  66b99000 - 677ab000 (reserved)
(XEN)  677ab000 - 67f9a000 (ACPI NVS)
(XEN)  67f9a000 - 67fff000 (ACPI data)
(XEN)  67fff000 - 6800 (usable)
(XEN)  0001 - 00089200 (usable)
(XEN)  6800 - 6810 (reserved)
(XEN)  e000 - f000 (reserved)
(XEN)  fe00 - fe011000 (reserved)
(XEN)  fec0 - fec01000 (reserved)
(XEN)  fee0 - fee01000 (reserved)
(XEN)  ff00 - 0001 (reserved)
(XEN) Checking MTRR ranges...
(XEN)  MTRR cap: 1d0a type: c06
(XEN) Xen-e820 RAM map:
(XEN)   - 0009c800 (usable)
(XEN)  0009c800 - 000a (reserved)
(XEN)  000e - 0010 (reserved)
(XEN)  0010 - 63911000 (usable)
(XEN)  63911000 - 63912000 (ACPI NVS)
(XEN)  63912000 - 6395c000 (reserved)
(XEN)  6395c000 - 639af000 (usable)
(XEN)  639af000 - 640b4000 (reserved)
(XEN)  640b4000 - 66b99000 (usable)
(XEN)  66b99000 - 677ab000 (reserved)
(XEN)  677ab000 - 67f9a000 (ACPI NVS)
(XEN)  67f9a000 - 67fff000 (ACPI data)
(XEN)  67fff000 - 6800 (usable)
(XEN)  6800 - 6810 (reserved)
(XEN)  e000 - f000 (reserved)
(XEN)  fe00 - fe011000 (reserved)
(XEN)  fec0 - fec01000 (reserved)
(XEN)  fee0 - fee01000 (reserved)
(XEN)  ff00 - 0001 (reserved)
(XEN)  0001 - 00089200 (usable)
(XEN) ACPI: RSDP 000F05B0, 0024 (r2 ALASKA)
(XEN) ACPI: XSDT 67F59098, 00AC (r1 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: FACP 67F7BB20, 010C (r5 ALASKA   A M I   1072009 AMI 10013)
(XEN) ACPI: DSDT 67F591D8, 22943 (r2 ALASKA   A M I   1072009 IN

Re: [Xen-devel] [PATCH MINI-OS] xenbus: notify the other end when necessary

[Samuel Thibault]

Hello,

Indeed, notification seems to have been missing since basically 2006...

Wei Liu, le Mon 26 Oct 2015 09:47:48 +, a écrit :
> diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
> index 4613ed6..7451161 100644
> --- a/xenbus/xenbus.c
> +++ b/xenbus/xenbus.c
> @@ -205,8 +205,10 @@ static void xenbus_thread_func(void *ign)
>  prod = xenstore_buf->rsp_prod;
>  DEBUG("Rsp_cons %d, rsp_prod %d.\n", xenstore_buf->rsp_cons,
>  xenstore_buf->rsp_prod);
> -if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < 
> sizeof(msg))
> +if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < 
> sizeof(msg)) {
> +notify_remote_via_evtchn(start_info.store_evtchn);
>  break;
> +}
>  rmb();
>  memcpy_from_ring(xenstore_buf->rsp,
>  &msg,
> @@ -217,8 +219,10 @@ static void xenbus_thread_func(void *ign)
>  xenstore_buf->rsp_prod - xenstore_buf->rsp_cons,
>  msg.req_id);
>  if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons <
> -sizeof(msg) + msg.len)
> +sizeof(msg) + msg.len) {
> +notify_remote_via_evtchn(start_info.store_evtchn);
>  break;
> +}
>  
>  DEBUG("Message is good.\n");
>  
> @@ -265,6 +269,9 @@ static void xenbus_thread_func(void *ign)
>  xenstore_buf->rsp_cons += msg.len + sizeof(msg);
>  wake_up(&req_info[msg.req_id].waitq);
>  }
> +
> +wmb();
> +notify_remote_via_evtchn(start_info.store_evtchn);
>  }
>  }
>  }

The wmb() position seems right, but the notification could be put a bit
later, after the exit of the while(1) loop.  That'd make it factorized
for all cases were the processing could want to stop, and make it quite
naturally enough just before the wait_event call, so something like
below (untested).

Samuel

diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
index 4613ed6..858aa98 100644
--- a/xenbus/xenbus.c
+++ b/xenbus/xenbus.c
@@ -265,7 +265,10 @@ static void xenbus_thread_func(void *ign)
 xenstore_buf->rsp_cons += msg.len + sizeof(msg);
 wake_up(&req_info[msg.req_id].waitq);
 }
+
+wmb();
 }
+notify_remote_via_evtchn(start_info.store_evtchn);
 }
 }
 

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

Re: [Xen-devel] [PATCH v1 1/5] xsplice: Design document.

[Martin Pohlack]

On 16.09.2015 23:01, Konrad Rzeszutek Wilk wrote:

[...]

> +### xsplice_patch
> +
> +This structure has the binary code (or data) to be patched. Depending on the
> +type it can either an inline patch (data or text) or require an relocation
> +change (which requires a trampoline). Naturally it also points to a blob
> +of the binary data to patch in, and the size of the patch.

On the Xen developer summit we agreed to start with a minimal approach
first.  Based on looking at the last ~50 XSA patches, I do think we can
do *without* the in-place patching and would propose to tackle this
approach later or not at all.

[...]

> +### Symbol names
> +
> +
> +Xen as it is now, has a couple of non-unique symbol names which will
> +make runtime symbol identification hard.  Sometimes, static symbols
> +simply have the same name in C files, sometimes such symbols get
> +included via header files, and some C files are also compiled
> +multiple times and linked under different names (guest_walk.c).
> +
> +As such we need to modify the linker to make sure that the symbol
> +table qualifies also symbols by their source file name.
> +
> +For the awkward situations in which C-files are compiled multiple
> +times patches we would need to some modification in the Xen code.
> +
> +
> +The convention for file-type symbols (that would allow to map many
> +symbols to their compilation unit) says that only the basename (i.e.,
> +without directories) is embedded.  This creates another layer of
> +confusion for duplicate file names in the build tree.
> +
> +That would have to be resolved.

Another approach here would be to qualify symbol names by the
object-file name which contains them + a unique path component, for example:

drivers/pci/pci.o
arch/x86/pci.o
arch/x86/x86_64/pci.o

> +
> +
> +> find . -name \*.c -print0 | xargs -0 -n1 basename | sort | uniq -c | sort 
> -n | tail -n10
> +  3 shutdown.c
> +  3 sysctl.c
> +  3 time.c
> +  3 xenoprof.c
> +  4 gdbstub.c
> +  4 irq.c
> +  5 domain.c
> +  5 mm.c
> +  5 pci.c
> +  5 traps.c
> +

Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B


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

[Xen-devel] [PATCH v2 5/5] x86/mm: only a single instance of gw_page_flags[] is needed

[Jan Beulich]

None of its elements depends on GUEST_PAGING_LEVELS.

Signed-off-by: Jan Beulich 
Reviewed-by: Andrew Cooper 
---
v2: Re-base on top of earlier changes.

--- a/xen/arch/x86/mm/guest_walk.c
+++ b/xen/arch/x86/mm/guest_walk.c
@@ -32,30 +32,32 @@ asm(".file \"" __OBJECT_FILE__ "\"");
 #include 
 #include 
 
+extern const uint32_t gw_page_flags[];
+#if GUEST_PAGING_LEVELS == CONFIG_PAGING_LEVELS
+const uint32_t gw_page_flags[] = {
+/* I/F -  Usr Wr */
+/* 0   0   0   0 */ _PAGE_PRESENT,
+/* 0   0   0   1 */ _PAGE_PRESENT|_PAGE_RW,
+/* 0   0   1   0 */ _PAGE_PRESENT|_PAGE_USER,
+/* 0   0   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER,
+/* 0   1   0   0 */ _PAGE_PRESENT,
+/* 0   1   0   1 */ _PAGE_PRESENT|_PAGE_RW,
+/* 0   1   1   0 */ _PAGE_PRESENT|_PAGE_USER,
+/* 0   1   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER,
+/* 1   0   0   0 */ _PAGE_PRESENT|_PAGE_NX_BIT,
+/* 1   0   0   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_NX_BIT,
+/* 1   0   1   0 */ _PAGE_PRESENT|_PAGE_USER|_PAGE_NX_BIT,
+/* 1   0   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER|_PAGE_NX_BIT,
+/* 1   1   0   0 */ _PAGE_PRESENT|_PAGE_NX_BIT,
+/* 1   1   0   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_NX_BIT,
+/* 1   1   1   0 */ _PAGE_PRESENT|_PAGE_USER|_PAGE_NX_BIT,
+/* 1   1   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER|_PAGE_NX_BIT,
+};
+#endif
 
 /* Flags that are needed in a pagetable entry, with the sense of NX inverted */
 static uint32_t mandatory_flags(struct vcpu *v, uint32_t pfec) 
 {
-static const uint32_t flags[] = {
-/* I/F -  Usr Wr */
-/* 0   0   0   0 */ _PAGE_PRESENT, 
-/* 0   0   0   1 */ _PAGE_PRESENT|_PAGE_RW,
-/* 0   0   1   0 */ _PAGE_PRESENT|_PAGE_USER,
-/* 0   0   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER,
-/* 0   1   0   0 */ _PAGE_PRESENT, 
-/* 0   1   0   1 */ _PAGE_PRESENT|_PAGE_RW,
-/* 0   1   1   0 */ _PAGE_PRESENT|_PAGE_USER,
-/* 0   1   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER,
-/* 1   0   0   0 */ _PAGE_PRESENT|_PAGE_NX_BIT, 
-/* 1   0   0   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_NX_BIT,
-/* 1   0   1   0 */ _PAGE_PRESENT|_PAGE_USER|_PAGE_NX_BIT,
-/* 1   0   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER|_PAGE_NX_BIT,
-/* 1   1   0   0 */ _PAGE_PRESENT|_PAGE_NX_BIT, 
-/* 1   1   0   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_NX_BIT,
-/* 1   1   1   0 */ _PAGE_PRESENT|_PAGE_USER|_PAGE_NX_BIT,
-/* 1   1   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER|_PAGE_NX_BIT,
-};
-
 /* Don't demand not-NX if the CPU wouldn't enforce it. */
 if ( !guest_supports_nx(v) )
 pfec &= ~PFEC_insn_fetch;
@@ -65,7 +67,7 @@ static uint32_t mandatory_flags(struct v
  && !(pfec & PFEC_user_mode) )
 pfec &= ~PFEC_write_access;
 
-return flags[(pfec & 0x1f) >> 1] | _PAGE_INVALID_BITS;
+return gw_page_flags[(pfec & 0x1f) >> 1] | _PAGE_INVALID_BITS;
 }
 
 /* Modify a guest pagetable entry to set the Accessed and Dirty bits.



x86/mm: only a single instance of gw_page_flags[] is needed

None of its elements depends on GUEST_PAGING_LEVELS.

Signed-off-by: Jan Beulich 
Reviewed-by: Andrew Cooper 
---
v2: Re-base on top of earlier changes.

--- a/xen/arch/x86/mm/guest_walk.c
+++ b/xen/arch/x86/mm/guest_walk.c
@@ -32,30 +32,32 @@ asm(".file \"" __OBJECT_FILE__ "\"");
 #include 
 #include 
 
+extern const uint32_t gw_page_flags[];
+#if GUEST_PAGING_LEVELS == CONFIG_PAGING_LEVELS
+const uint32_t gw_page_flags[] = {
+/* I/F -  Usr Wr */
+/* 0   0   0   0 */ _PAGE_PRESENT,
+/* 0   0   0   1 */ _PAGE_PRESENT|_PAGE_RW,
+/* 0   0   1   0 */ _PAGE_PRESENT|_PAGE_USER,
+/* 0   0   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER,
+/* 0   1   0   0 */ _PAGE_PRESENT,
+/* 0   1   0   1 */ _PAGE_PRESENT|_PAGE_RW,
+/* 0   1   1   0 */ _PAGE_PRESENT|_PAGE_USER,
+/* 0   1   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER,
+/* 1   0   0   0 */ _PAGE_PRESENT|_PAGE_NX_BIT,
+/* 1   0   0   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_NX_BIT,
+/* 1   0   1   0 */ _PAGE_PRESENT|_PAGE_USER|_PAGE_NX_BIT,
+/* 1   0   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER|_PAGE_NX_BIT,
+/* 1   1   0   0 */ _PAGE_PRESENT|_PAGE_NX_BIT,
+/* 1   1   0   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_NX_BIT,
+/* 1   1   1   0 */ _PAGE_PRESENT|_PAGE_USER|_PAGE_NX_BIT,
+/* 1   1   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER|_PAGE_NX_BIT,
+};
+#endif
 
 /* Flags that are needed in a pagetable entry, with the sense of NX inverted */
 static uint32_t mandatory_flags(struct vcpu *v, uint32_t pfec) 
 {
-static const uint32_t flags[] = {
-/* I/F -  Usr Wr */
-/* 0   0   0   0 */ _PAGE_PRESENT, 
-/* 0   0   0   1 */ _PAGE_PRESENT|_PAGE_RW,
-/* 0   0   1   0 */ _PAGE_PRESENT|_PAGE_USER,
-/* 0   0   1   1 */ _PAGE_PRESENT|_PAGE_RW|_PAGE_USER,
-/* 0   1   0   0 */ _PAGE_PRESENT, 
-/* 0   1   0   1 */ _PAGE_

Re: [Xen-devel] [PATCH net-next 0/8] xen-netback/core: packet hashing

[David Miller]

From: David Vrabel 
Date: Mon, 26 Oct 2015 10:38:50 +

> On 24/10/15 12:55, David Miller wrote:
>> From: Paul Durrant 
>> Date: Wed, 21 Oct 2015 11:36:17 +0100
>> 
>>> This series adds xen-netback support for hash negotiation with a frontend
>>> driver, and an implementation of toeplitz hashing as the initial negotiable
>>> algorithm.
>> 
>> Ping, I want to see some review from some other xen networking folks.
> 
> There's been some review of the front/back protocol (on a different
> thread) and some significant changes have been suggested.

Ok, I'll mark this series as "changes requested" then, thanks.

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

[Xen-devel] [PATCH v2 4/5] x86/mm: build map_domain_gfn() just once

[Jan Beulich]

It doesn't depend on GUEST_PAGING_LEVELS. Moving the function to p2m.c
at once allows a bogus #define/#include pair to be removed from
hap/nested_ept.c.

Signed-off-by: Jan Beulich 
---
v2: To ensure no dependency on GUEST_PAGING_LEVELS, move the function
to p2m.c.

--- a/xen/arch/x86/mm/guest_walk.c
+++ b/xen/arch/x86/mm/guest_walk.c
@@ -88,46 +88,6 @@ static uint32_t set_ad_bits(void *guest_
 return 0;
 }
 
-/* If the map is non-NULL, we leave this function having 
- * acquired an extra ref on mfn_to_page(*mfn) */
-void *map_domain_gfn(struct p2m_domain *p2m, gfn_t gfn, mfn_t *mfn,
- p2m_type_t *p2mt, p2m_query_t q, uint32_t *rc)
-{
-struct page_info *page;
-void *map;
-
-/* Translate the gfn, unsharing if shared */
-page = get_page_from_gfn_p2m(p2m->domain, p2m, gfn_x(gfn), p2mt, NULL,
- q);
-if ( p2m_is_paging(*p2mt) )
-{
-ASSERT(p2m_is_hostp2m(p2m));
-if ( page )
-put_page(page);
-p2m_mem_paging_populate(p2m->domain, gfn_x(gfn));
-*rc = _PAGE_PAGED;
-return NULL;
-}
-if ( p2m_is_shared(*p2mt) )
-{
-if ( page )
-put_page(page);
-*rc = _PAGE_SHARED;
-return NULL;
-}
-if ( !page )
-{
-*rc |= _PAGE_PRESENT;
-return NULL;
-}
-*mfn = _mfn(page_to_mfn(page));
-ASSERT(mfn_valid(mfn_x(*mfn)));
-
-map = map_domain_page(*mfn);
-return map;
-}
-
-
 /* Walk the guest pagetables, after the manner of a hardware walker. */
 /* Because the walk is essentially random, it can cause a deadlock 
  * warning in the p2m locking code. Highly unlikely this is an actual
--- a/xen/arch/x86/mm/hap/nested_ept.c
+++ b/xen/arch/x86/mm/hap/nested_ept.c
@@ -34,10 +34,6 @@
 #include 
 #include 
 
-/* EPT always use 4-level paging structure */
-#define GUEST_PAGING_LEVELS 4
-#include 
-
 /* Must reserved bits in all level entries  */
 #define EPT_MUST_RSV_BITS (((1ull << PADDR_BITS) - 1) & \
~((1ull << paddr_bits) - 1))
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2053,6 +2053,44 @@ unsigned long paging_gva_to_gfn(struct v
 return hostmode->gva_to_gfn(v, hostp2m, va, pfec);
 }
 
+/*
+ * If the map is non-NULL, we leave this function having
+ * acquired an extra ref on mfn_to_page(*mfn).
+ */
+void *map_domain_gfn(struct p2m_domain *p2m, gfn_t gfn, mfn_t *mfn,
+ p2m_type_t *p2mt, p2m_query_t q, uint32_t *rc)
+{
+struct page_info *page;
+
+/* Translate the gfn, unsharing if shared. */
+page = get_page_from_gfn_p2m(p2m->domain, p2m, gfn_x(gfn), p2mt, NULL, q);
+if ( p2m_is_paging(*p2mt) )
+{
+ASSERT(p2m_is_hostp2m(p2m));
+if ( page )
+put_page(page);
+p2m_mem_paging_populate(p2m->domain, gfn_x(gfn));
+*rc = _PAGE_PAGED;
+return NULL;
+}
+if ( p2m_is_shared(*p2mt) )
+{
+if ( page )
+put_page(page);
+*rc = _PAGE_SHARED;
+return NULL;
+}
+if ( !page )
+{
+*rc |= _PAGE_PRESENT;
+return NULL;
+}
+*mfn = page_to_mfn(page);
+ASSERT(mfn_valid(*mfn));
+
+return map_domain_page(*mfn);
+}
+
 int map_mmio_regions(struct domain *d,
  unsigned long start_gfn,
  unsigned long nr,
--- a/xen/include/asm-x86/guest_pt.h
+++ b/xen/include/asm-x86/guest_pt.h
@@ -305,10 +305,6 @@ guest_walk_to_page_order(walk_t *gw)
 #define GPT_RENAME2(_n, _l) _n ## _ ## _l ## _levels
 #define GPT_RENAME(_n, _l) GPT_RENAME2(_n, _l)
 #define guest_walk_tables GPT_RENAME(guest_walk_tables, GUEST_PAGING_LEVELS)
-#define map_domain_gfn GPT_RENAME(map_domain_gfn, GUEST_PAGING_LEVELS)
-
-void *map_domain_gfn(struct p2m_domain *p2m, gfn_t gfn, mfn_t *mfn,
- p2m_type_t *p2mt, p2m_query_t q, uint32_t *rc);
 
 extern uint32_t 
 guest_walk_tables(struct vcpu *v, struct p2m_domain *p2m, unsigned long va,
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -680,6 +680,9 @@ int p2m_set_entry(struct p2m_domain *p2m
 /* Set up function pointers for PT implementation: only for use by p2m code */
 extern void p2m_pt_init(struct p2m_domain *p2m);
 
+void *map_domain_gfn(struct p2m_domain *p2m, gfn_t gfn, mfn_t *mfn,
+ p2m_type_t *p2mt, p2m_query_t q, uint32_t *rc);
+
 /* Debugging and auditing of the P2M code? */
 #ifndef NDEBUG
 #define P2M_AUDIT 1


x86/mm: build map_domain_gfn() just once

It doesn't depend on GUEST_PAGING_LEVELS. Moving the function to p2m.c
at once allows a bogus #define/#include pair to be removed from
hap/nested_ept.c.

Signed-off-by: Jan Beulich 
---
v2: To ensure no dependency on GUEST_PAGING_LEVELS, move the function
to p2m.c.

--- a/xen/arch/x86/mm/guest_walk.c
+++ b/xen/arch/x86/mm/guest_walk.c
@@ -88,46 +88,6 @@ static uint32_t set_ad_bits(void *guest_
 return 0;
 }
 
-/* If the map is non-NULL, we leave t

[Xen-devel] [distros-debian-sid test] 38212: tolerable FAIL

[Platform Team regression test user]

flight 38212 distros-debian-sid real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/38212/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-i386-sid-netboot-pygrub  9 debian-di-install  fail like 38180
 test-amd64-i386-amd64-sid-netboot-pygrub  9 debian-di-install  fail like 38180
 test-amd64-amd64-amd64-sid-netboot-pvgrub  9 debian-di-install fail like 38180
 test-amd64-i386-i386-sid-netboot-pvgrub  9 debian-di-install   fail like 38180

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-armhf-sid-netboot-pygrub 11 migrate-support-check fail never 
pass
 test-armhf-armhf-armhf-sid-netboot-pygrub 12 saverestore-support-check fail 
never pass

baseline version:
 flight   38180

jobs:
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-amd64-sid-netboot-pvgrubfail
 test-amd64-i386-i386-sid-netboot-pvgrub  fail
 test-amd64-i386-amd64-sid-netboot-pygrub fail
 test-armhf-armhf-armhf-sid-netboot-pygrubpass
 test-amd64-amd64-i386-sid-netboot-pygrub fail



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.


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

[Xen-devel] [PATCH v2 3/5] x86/mm: override stored file names for multiply built sources

[Jan Beulich]

To make it possible to tell apart the static symbols therein, use their
object file names instead of their source ones.

Signed-off-by: Jan Beulich 
---
v2: Introduce __OBJECT_FILE__.

--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -42,10 +42,10 @@ ALL_OBJS-y   += $(BASEDIR)/x
 ALL_OBJS-y   += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(x86)  += $(BASEDIR)/crypto/built_in.o
 
-CFLAGS += -fno-builtin -fno-common
+CFLAGS += -nostdinc -fno-builtin -fno-common
 CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
 CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
-CFLAGS += -nostdinc
+CFLAGS += '-D__OBJECT_FILE__="$@"'
 
 CFLAGS-$(XSM_ENABLE)+= -DXSM_ENABLE
 CFLAGS-$(FLASK_ENABLE)  += -DFLASK_ENABLE
--- a/xen/arch/x86/mm/guest_walk.c
+++ b/xen/arch/x86/mm/guest_walk.c
@@ -21,6 +21,9 @@
  * along with this program; If not, see .
  */
 
+/* Allow uniquely identifying static symbols in the 3 generated objects. */
+asm(".file \"" __OBJECT_FILE__ "\"");
+
 #include 
 #include 
 #include 
--- a/xen/arch/x86/mm/hap/guest_walk.c
+++ b/xen/arch/x86/mm/hap/guest_walk.c
@@ -18,6 +18,8 @@
  * this program; If not, see .
  */
 
+/* Allow uniquely identifying static symbols in the 3 generated objects. */
+asm(".file \"" __OBJECT_FILE__ "\"");
 
 #include 
 #include 
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -20,7 +20,9 @@
  * along with this program; If not, see .
  */
 
-#include 
+/* Allow uniquely identifying static symbols in the 3 generated objects. */
+asm(".file \"" __OBJECT_FILE__ "\"");
+
 #include 
 #include 
 #include 



x86/mm: override stored file names for multiply built sources

To make it possible to tell apart the static symbols therein, use their
object file names instead of their source ones.

Signed-off-by: Jan Beulich 
---
v2: Introduce __OBJECT_FILE__.

--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -42,10 +42,10 @@ ALL_OBJS-y   += $(BASEDIR)/x
 ALL_OBJS-y   += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(x86)  += $(BASEDIR)/crypto/built_in.o
 
-CFLAGS += -fno-builtin -fno-common
+CFLAGS += -nostdinc -fno-builtin -fno-common
 CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
 CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
-CFLAGS += -nostdinc
+CFLAGS += '-D__OBJECT_FILE__="$@"'
 
 CFLAGS-$(XSM_ENABLE)+= -DXSM_ENABLE
 CFLAGS-$(FLASK_ENABLE)  += -DFLASK_ENABLE
--- a/xen/arch/x86/mm/guest_walk.c
+++ b/xen/arch/x86/mm/guest_walk.c
@@ -21,6 +21,9 @@
  * along with this program; If not, see .
  */
 
+/* Allow uniquely identifying static symbols in the 3 generated objects. */
+asm(".file \"" __OBJECT_FILE__ "\"");
+
 #include 
 #include 
 #include 
--- a/xen/arch/x86/mm/hap/guest_walk.c
+++ b/xen/arch/x86/mm/hap/guest_walk.c
@@ -18,6 +18,8 @@
  * this program; If not, see .
  */
 
+/* Allow uniquely identifying static symbols in the 3 generated objects. */
+asm(".file \"" __OBJECT_FILE__ "\"");
 
 #include 
 #include 
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -20,7 +20,9 @@
  * along with this program; If not, see .
  */
 
-#include 
+/* Allow uniquely identifying static symbols in the 3 generated objects. */
+asm(".file \"" __OBJECT_FILE__ "\"");
+
 #include 
 #include 
 #include 
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 2/5] compat: enforce distinguishable file names in symbol table

[Jan Beulich]

To make it possible to tell apart the static symbols in files built a
second for compat guest support, arrange for their source file names to
be prefixed by a suitable path. We can't do this without explicit .file
directives, since gcc has always been stripping paths from file names
handed to the internally generated .file directive. However, we can
leverage __FILE__ if we make sure the second instance gets compiled out
of other than the very directory the wrapper sits in.

Where suitable, remove the long redundant explicit inclusions of
xen/config.h at once.

Signed-off-by: Jan Beulich 

--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -86,8 +86,7 @@ AFLAGS-$(clang) += -no-integrate
 ALL_OBJS := $(ALL_OBJS-y)
 
 # Get gcc to generate the dependencies for us.
-CFLAGS-y += -MMD -MF .$(@F).d
-DEPS = .*.d
+CFLAGS-y += -MMD -MF $(@D)/.$(@F).d
 
 CFLAGS += $(CFLAGS-y)
 
@@ -103,6 +102,14 @@ LDFLAGS += $(LDFLAGS-y)
 
 include Makefile
 
+DEPS = .*.d
+define gendep
+ifneq ($(1),$(subst /,:,$(1)))
+DEPS += $(dir $(1)).$(basename $(notdir $(1))).d
+endif
+endef
+$(foreach o,$(filter-out %/,$(obj-y)),$(eval $(call gendep,$(o
+
 # Ensure each subdirectory has exactly one trailing slash.
 subdir-n := $(patsubst %,%/,$(patsubst %/,%,$(subdir-n) $(subdir-)))
 subdir-y := $(patsubst %,%/,$(patsubst %/,%,$(subdir-y)))
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -13,7 +13,7 @@ obj-y += bitops.o
 obj-bin-y += bzimage.init.o
 obj-bin-y += clear_page.o
 obj-bin-y += copy_page.o
-obj-y += compat.o
+obj-y += compat.o x86_64/compat.o
 obj-$(CONFIG_KEXEC) += crash.o
 obj-y += debug.o
 obj-y += delay.o
@@ -25,7 +25,6 @@ obj-y += domain_page.o
 obj-y += e820.o
 obj-y += extable.o
 obj-y += flushtlb.o
-obj-y += platform_hypercall.o
 obj-y += i387.o
 obj-y += i8259.o
 obj-y += io_apic.o
@@ -37,14 +36,15 @@ obj-y += microcode_amd.o
 obj-y += microcode_intel.o
 # This must come after the vendor specific files.
 obj-y += microcode.o
-obj-y += mm.o
+obj-y += mm.o x86_64/mm.o
 obj-y += monitor.o
 obj-y += mpparse.o
 obj-y += nmi.o
 obj-y += numa.o
 obj-y += pci.o
 obj-y += percpu.o
-obj-y += physdev.o
+obj-y += physdev.o x86_64/physdev.o
+obj-y += platform_hypercall.o x86_64/platform_hypercall.o
 obj-y += psr.o
 obj-y += setup.o
 obj-y += shutdown.o
--- a/xen/arch/x86/x86_64/Makefile
+++ b/xen/arch/x86/x86_64/Makefile
@@ -2,7 +2,6 @@ subdir-y += compat
 
 obj-bin-y += entry.o
 obj-bin-y += gpr_switch.o
-obj-y += mm.o
 obj-y += traps.o
 obj-y += machine_kexec.o
 obj-y += pci.o
@@ -10,10 +9,7 @@ obj-y += acpi_mmcfg.o
 obj-y += mmconf-fam10h.o
 obj-y += mmconfig_64.o
 obj-y += mmconfig-shared.o
-obj-y += compat.o
 obj-y += domain.o
-obj-y += physdev.o
-obj-y += platform_hypercall.o
 obj-y += cpu_idle.o
 obj-y += cpufreq.o
 obj-bin-y += kexec_reloc.o
--- a/xen/arch/x86/x86_64/compat.c
+++ b/xen/arch/x86/x86_64/compat.c
@@ -2,7 +2,8 @@
  * compat.c
  */
 
-#include 
+asm(".file \"" __FILE__ "\"");
+
 #include 
 #include 
 #include 
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -16,7 +16,8 @@
  * with this program; If not, see .
  */
 
-#include 
+asm(".file \"" __FILE__ "\"");
+
 #include 
 #include 
 #include 
--- a/xen/arch/x86/x86_64/physdev.c
+++ b/xen/arch/x86/x86_64/physdev.c
@@ -2,7 +2,8 @@
  * physdev.c
  */
 
-#include 
+asm(".file \"" __FILE__ "\"");
+
 #include 
 #include 
 #include 
--- a/xen/arch/x86/x86_64/platform_hypercall.c
+++ b/xen/arch/x86/x86_64/platform_hypercall.c
@@ -2,7 +2,8 @@
  * platform_hypercall.c
  */
 
-#include 
+asm(".file \"" __FILE__ "\"");
+
 #include 
 #include 
 
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -63,7 +63,7 @@ obj-$(perfc)   += perfc.o
 obj-$(crash_debug) += gdbstub.o
 obj-$(xenoprof)+= xenoprof.o
 
-subdir-$(CONFIG_COMPAT) += compat
+obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o 
multicall.o tmem_xen.o xlat.o)
 
 subdir-$(x86_64) += hvm
 
--- a/xen/common/compat/Makefile
+++ /dev/null
@@ -1,6 +0,0 @@
-obj-y += domain.o
-obj-y += kernel.o
-obj-y += memory.o
-obj-y += multicall.o
-obj-y += xlat.o
-obj-y += tmem_xen.o
--- a/xen/common/compat/domain.c
+++ b/xen/common/compat/domain.c
@@ -3,7 +3,8 @@
  *
  */
 
-#include 
+asm(".file \"" __FILE__ "\"");
+
 #include 
 #include 
 #include 
--- a/xen/common/compat/kernel.c
+++ b/xen/common/compat/kernel.c
@@ -2,7 +2,8 @@
  * kernel.c
  */
 
-#include 
+asm(".file \"" __FILE__ "\"");
+
 #include 
 #include 
 #include 
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -1,4 +1,5 @@
-#include 
+asm(".file \"" __FILE__ "\"");
+
 #include 
 #include 
 #include 
--- a/xen/common/compat/multicall.c
+++ b/xen/common/compat/multicall.c
@@ -2,7 +2,8 @@
  * multicall.c
  */
 
-#include 
+asm(".file \"" __FILE__ "\"");
+
 #include 
 #include 
 #include 
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -58,7 +58,7 @@ compat/%.h: compat/%.i Makefile $(BASEDI
mv -f $@.new $@
 
 compat/%.i: compat/%.c Makefi

[Xen-devel] [PATCH v2 1/5] symbols: prefix static symbols with their source file names

[Jan Beulich]

This requires adjustments to the tool generating the symbol table and
its as well as nm's invocation.

Note: Not warning about duplicate symbols in the EFI case for now, as
a binutils bug causes misnamed file name entries to appear in EFI
binaries' symbol tables when the file name is longer than 18 chars.
(Not doing so also avoids other duplicates getting printed twice.)

Signed-off-by: Jan Beulich 
---
v2: Also special case file names with directory part (along with
object ones). Mirror xen-syms linking changes to ARM, but for now
without generating warnings.

--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -80,11 +80,13 @@ $(BASEDIR)/common/symbols-dummy.o:
 $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o
$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
$(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
-   $(NM) -n $(@D)/.$(@F).0 | $(BASEDIR)/tools/symbols >$(@D)/.$(@F).0.S
+   $(NM) -pa --format=sysv $(@D)/.$(@F).0 \
+   | $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).0.S
$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
$(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
-   $(NM) -n $(@D)/.$(@F).1 | $(BASEDIR)/tools/symbols >$(@D)/.$(@F).1.S
+   $(NM) -pa --format=sysv $(@D)/.$(@F).1 \
+   | $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).1.S
$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
$(@D)/.$(@F).1.o -o $@
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -106,11 +106,13 @@ $(BASEDIR)/common/symbols-dummy.o:
 $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o
$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
$(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
-   $(NM) -n $(@D)/.$(@F).0 | $(BASEDIR)/tools/symbols >$(@D)/.$(@F).0.S
+   $(NM) -pa --format=sysv $(@D)/.$(@F).0 \
+   | $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).0.S
$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
$(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
-   $(NM) -n $(@D)/.$(@F).1 | $(BASEDIR)/tools/symbols >$(@D)/.$(@F).1.S
+   $(NM) -pa --format=sysv $(@D)/.$(@F).1 \
+   | $(BASEDIR)/tools/symbols --sysv --sort --warn-dup 
>$(@D)/.$(@F).1.S
$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
$(@D)/.$(@F).1.o -o $@
@@ -133,13 +135,15 @@ $(TARGET).efi: prelink-efi.o efi.lds efi
  $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< 
efi/relocs-dummy.o \
$(BASEDIR)/common/symbols-dummy.o -o 
$(@D)/.$(@F).$(base).0 &&) :
$(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) 
$(ALT_BASE),$(@D)/.$(@F).$(base).0) >$(@D)/.$(@F).0r.S
-   $(guard) $(NM) -n $(@D)/.$(@F).$(VIRT_BASE).0 | $(guard) 
$(BASEDIR)/tools/symbols >$(@D)/.$(@F).0s.S
+   $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).0 \
+   | $(guard) $(BASEDIR)/tools/symbols --sysv --sort 
>$(@D)/.$(@F).0s.S
$(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0r.o 
$(@D)/.$(@F).0s.o
$(foreach base, $(VIRT_BASE) $(ALT_BASE), \
  $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< \
$(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o -o 
$(@D)/.$(@F).$(base).1 &&) :
$(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) 
$(ALT_BASE),$(@D)/.$(@F).$(base).1) >$(@D)/.$(@F).1r.S
-   $(guard) $(NM) -n $(@D)/.$(@F).$(VIRT_BASE).1 | $(guard) 
$(BASEDIR)/tools/symbols >$(@D)/.$(@F).1s.S
+   $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).1 \
+   | $(guard) $(BASEDIR)/tools/symbols --sysv --sort 
>$(@D)/.$(@F).1s.S
$(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1r.o 
$(@D)/.$(@F).1s.o
$(guard) $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds -N $< \
$(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o -o $@
--- a/xen/tools/symbols.c
+++ b/xen/tools/symbols.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define KSYM_NAME_LEN  127
@@ -40,13 +41,14 @@ struct sym_entry {
unsigned int len;
unsigned char *sym;
 };
-
+#define SYMBOL_NAME(s) ((char *)(s)->sym + 1)
 
 static struct sym_entry *table;
 static unsigned int table_size, table_cnt;
 static unsigned long long _stext, _etext, _sinittext, _einittext, _sextratext, 
_eextratext;
 static int all_symbols = 0;
 static char symbol_prefix_char = '\0';
+static enum { fmt_bsd, fmt_sysv } input_format;
 
 int token_profit[0x1];
 
@@ -73,11 +75,25 @@ static inline int is_arm_mapping_symbol(
 
 static int read_symbol(FILE *in, struct sym_entry *s)
 {
-   char str[500];
+   char str[500], type[20] = "";
char *sym, stype;
-   int rc;
-
-   rc = fs

Re: [Xen-devel] does cpu_down makes brings core to deepest sleep state?

[Jan Beulich]

>>> On 26.10.15 at 12:13,  wrote:
>>> 
>>>
>>> Did you look at the code generating the stats?
>> 
>> No.

Then may I suggest you do so, since ...

>> C0 gets accounted
>> to everything not explicitly accounted to any other C state (i.e. only
>> C1...Cn get explicit stats maintained).
>>
>> I dont think so. C0 only accounted when all parts of cpu being on. I was
> wondering if cpu_down turns off the cpu, why the 20 second sleep is
> considered as C0?

... this is being answered by what I said (i.e. how the statistics for
C0 get calculated). Meaning "I don't think so" as an answer implies
you see a bug in the code, which then I would expect you to spell
out.

Jan


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

Re: [Xen-devel] Question about Xen on Freescale IMX6 SABRE Board?

[Julien Grall]

On 25/10/15 15:35, Meng Xu wrote:
> Hi,

Hi Meng,

> Does anyone know if Xen 4.5+ can run on Freescale IMX6 QuadCore SABRE board 
> [1]?
> I found a video back to 2013 which shows an automotive  demo [2] of
> Mentor Graphics' Embedded ARM Hypervisor on Freescale i.MX6 Board.
> 
> I'm curious if anyone has run Xen on Freescale IMX6 QuadCore board?

Xen ARM requires virtualization extension in order to work. AFAICT this
board is using Cortex A-9 which doesn't have virtualization extension.

I suspect the demo was done with the Xen ARM PV port [3] which is not
maintained.

> If Xen can run on the board, we want to test how the RTDS real-time
> scheduler works on the ARM board.

FIY, You can find a list of ARM board where we know that Xen boots on
the wiki:

http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions#Hardware

> Thank you very much for your time in answering this question!
> 
> Best regards,

Regards,

[3] http://wiki.xenproject.org/wiki/Archived/Xen_ARM_(PV)



-- 
Julien Grall

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

[Xen-devel] [PATCH] Revert "libxl: use correct command line for arm guests."

[Ian Jackson]

This reverts commit 9befcd335c21818caaf5c5ab764d31a4006d2800.

This commit breaks the build:
 libxl_arm.c: In function 'libxl__arch_domain_init_hw_description':
 libxl_arm.c:591:76: error: 'state' undeclared (first use in this function)
 libxl_arm.c:591:76: note: each undeclared identifier is reported only once fo\
r each function it appears in
 make[3]: *** [libxl_arm.o] Error 1

"state" was introduced in a7511905 "xen: Extend DOMCTL createdomain to
support arch configuration".

On Julien's recommendation: a7511905 ought not to be backported, so
revert this and wait for Ian Campbell to get back.

Signed-off-by: Ian Jackson 
CC: Julien Grall 
CC: Ian Campbell 
CC: Jan Beulich 
---
 tools/libxl/libxl_arm.c |8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index f89533b..448ac07 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -195,7 +195,6 @@ static int make_root_properties(libxl__gc *gc,
 }
 
 static int make_chosen_node(libxl__gc *gc, void *fdt, bool ramdisk,
-libxl__domain_build_state *state,
 const libxl_domain_build_info *info)
 {
 int res;
@@ -204,9 +203,8 @@ static int make_chosen_node(libxl__gc *gc, void *fdt, bool 
ramdisk,
 res = fdt_begin_node(fdt, "chosen");
 if (res) return res;
 
-if (state->pv_cmdline) {
-LOG(DEBUG, "/chosen/bootargs = %s", state->pv_cmdline);
-res = fdt_property_string(fdt, "bootargs", state->pv_cmdline);
+if (info->cmdline) {
+res = fdt_property_string(fdt, "bootargs", info->cmdline);
 if (res) return res;
 }
 
@@ -588,7 +586,7 @@ next_resize:
 FDT( fdt_begin_node(fdt, "") );
 
 FDT( make_root_properties(gc, vers, fdt) );
-FDT( make_chosen_node(gc, fdt, !!dom->ramdisk_blob, state, info) );
+FDT( make_chosen_node(gc, fdt, !!dom->ramdisk_blob, info) );
 FDT( make_cpus_node(gc, fdt, info->max_vcpus, ainfo) );
 FDT( make_psci_node(gc, fdt) );
 
-- 
1.7.10.4


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

[Xen-devel] [PATCH 1/2] xen/x86: Helpers for cpu feature manipuation

[Andrew Cooper]

Expose them to assembly code, and replace open-coded versions.

No functional change.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/arch/x86/boot/head.S |  2 +-
 xen/arch/x86/boot/trampoline.S   |  2 +-
 xen/arch/x86/cpu/common.c|  4 +-
 xen/arch/x86/efi/efi-boot.h  |  2 +-
 xen/arch/x86/hvm/hvm.c   | 10 ++---
 xen/arch/x86/traps.c |  2 +-
 xen/arch/x86/xstate.c|  4 +-
 xen/include/asm-x86/amd.h| 89 +++-
 xen/include/asm-x86/asm_defns.h  |  2 +-
 xen/include/asm-x86/cpufeature.h |  7 +++-
 10 files changed, 63 insertions(+), 61 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index f63b349..ac4962b 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -142,7 +142,7 @@ __start:
 mov 
%edx,sym_phys(boot_cpu_data)+CPUINFO_FEATURE_OFFSET(X86_FEATURE_LM)
 
 /* Check for availability of long mode. */
-bt  $X86_FEATURE_LM & 0x1f,%edx
+bt  $cpufeat_bit(X86_FEATURE_LM),%edx
 jnc bad_cpu
 
 /* Stash TSC to calculate a good approximation of time-since-boot */
diff --git a/xen/arch/x86/boot/trampoline.S b/xen/arch/x86/boot/trampoline.S
index 8b0d9c1..c8f32cd 100644
--- a/xen/arch/x86/boot/trampoline.S
+++ b/xen/arch/x86/boot/trampoline.S
@@ -89,7 +89,7 @@ trampoline_protmode_entry:
 movl$MSR_EFER,%ecx
 rdmsr
 or  $EFER_LME|EFER_SCE,%eax   /* Long Mode + SYSCALL/SYSRET */
-bt  $X86_FEATURE_NX % 32,%edi /* No Execute? */
+bt  $cpufeat_bit(X86_FEATURE_NX),%edi /* No Execute? */
 jnc 1f
 btsl$_EFER_NX,%eax  /* No Execute */
 1:  wrmsr
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index f256444..a5caa84 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -202,7 +202,7 @@ static void __init early_cpu_detect(void)
c->x86_mask = tfms & 15;
cap0 &= ~cleared_caps[0];
cap4 &= ~cleared_caps[4];
-   if (cap0 & (1<<19))
+   if (cap0 & cpufeat_mask(X86_FEATURE_CLFLSH))
c->x86_cache_alignment = ((misc >> 8) & 0xff) * 8;
/* Leaf 0x1 capabilities filled in early for Xen. */
c->x86_capability[0] = cap0;
@@ -263,7 +263,7 @@ static void __cpuinit generic_identify(struct cpuinfo_x86 
*c)
if ( c->cpuid_level >= 0x0007 ) {
u32 dummy;
cpuid_count(0x0007, 0, &dummy, &ebx, &dummy, &dummy);
-   c->x86_capability[X86_FEATURE_FSGSBASE / 32] = ebx;
+   c->x86_capability[cpufeat_word(X86_FEATURE_FSGSBASE)] = ebx;
}
 }
 
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 4c7f383..d8ca862 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -225,7 +225,7 @@ static void __init noreturn efi_arch_post_exit_boot(void)
 asm volatile("pushq $0\n\tpopfq");
 rdmsrl(MSR_EFER, efer);
 efer |= EFER_SCE;
-if ( cpuid_ext_features & (1 << (X86_FEATURE_NX & 0x1f)) )
+if ( cpuid_ext_features & cpufeat_mask(X86_FEATURE_NX) )
 efer |= EFER_NX;
 wrmsrl(MSR_EFER, efer);
 write_cr0(X86_CR0_PE | X86_CR0_MP | X86_CR0_ET | X86_CR0_NE | X86_CR0_WP |
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 3fa2280..965bfbf 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1850,8 +1850,8 @@ static const char * hvm_efer_valid(const struct vcpu *v, 
uint64_t value,
 }
 else
 {
-ext1_edx = boot_cpu_data.x86_capability[X86_FEATURE_LM / 32];
-ext1_ecx = boot_cpu_data.x86_capability[X86_FEATURE_SVM / 32];
+ext1_edx = boot_cpu_data.x86_capability[cpufeat_word(X86_FEATURE_LM)];
+ext1_ecx = boot_cpu_data.x86_capability[cpufeat_word(X86_FEATURE_SVM)];
 }
 
 /*
@@ -1917,9 +1917,9 @@ static unsigned long hvm_cr4_guest_reserved_bits(const 
struct vcpu *v,
 }
 else
 {
-leaf1_edx = boot_cpu_data.x86_capability[X86_FEATURE_VME / 32];
-leaf1_ecx = boot_cpu_data.x86_capability[X86_FEATURE_PCID / 32];
-leaf7_0_ebx = boot_cpu_data.x86_capability[X86_FEATURE_FSGSBASE / 32];
+leaf1_edx = 
boot_cpu_data.x86_capability[cpufeat_word(X86_FEATURE_VME)];
+leaf1_ecx = 
boot_cpu_data.x86_capability[cpufeat_word(X86_FEATURE_PCID)];
+leaf7_0_ebx = 
boot_cpu_data.x86_capability[cpufeat_word(X86_FEATURE_FSGSBASE)];
 }
 
 return ~(unsigned long)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 8093535..b32f696 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -935,7 +935,7 @@ void pv_cpuid(struct cpu_user_regs *regs)
 goto unsupported;
 if ( regs->_ecx == 1 )
 {
-a &= boot_cpu_data.x86_capability[X86_FEATURE_XSAVEOPT / 32];
+a &= 
boot_cpu_data.x86_capability[cpufeat_word(X86_FEATURE_XSAVEOPT)];
 if ( 

[Xen-devel] [PATCH 0/2] xen/x86: Improvements to x86_capability[] handling

[Andrew Cooper]

More patches as a result of my cpuid levelling improvement work.

These changes are all purely mechanical and neither of these two patches have
any functional change; the compiled binaries are identical other than their
timestamps.

Andrew Cooper (2):
  xen/x86: Helpers for cpu feature manipuation
  xen/x86: Remove assumptions about the layout of x86_capabilities

 xen/arch/x86/boot/head.S |  2 +-
 xen/arch/x86/boot/trampoline.S   |  2 +-
 xen/arch/x86/cpu/amd.c   |  2 +-
 xen/arch/x86/cpu/centaur.c   |  6 +--
 xen/arch/x86/cpu/common.c| 22 +-
 xen/arch/x86/efi/efi-boot.h  |  5 ++-
 xen/arch/x86/hvm/hvm.c   | 10 ++---
 xen/arch/x86/mpparse.c   |  6 ++-
 xen/arch/x86/traps.c |  2 +-
 xen/arch/x86/xstate.c|  4 +-
 xen/include/asm-x86/amd.h| 89 +++-
 xen/include/asm-x86/asm_defns.h  |  2 +-
 xen/include/asm-x86/cpufeature.h |  8 +++-
 13 files changed, 83 insertions(+), 77 deletions(-)

-- 
2.1.4


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

[Xen-devel] [PATCH 2/2] xen/x86: Remove assumptions about the layout of x86_capabilities

[Andrew Cooper]

Future work will rearange it, invalidating these assumptions.

No functional change.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/arch/x86/cpu/amd.c   |  2 +-
 xen/arch/x86/cpu/centaur.c   |  6 ++
 xen/arch/x86/cpu/common.c| 18 ++
 xen/arch/x86/efi/efi-boot.h  |  3 ++-
 xen/arch/x86/mpparse.c   |  6 --
 xen/include/asm-x86/cpufeature.h |  1 +
 6 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index fd57370..74a0152 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -444,7 +444,7 @@ static void __devinit init_amd(struct cpuinfo_x86 *c)
 
/* Bit 31 in normal CPUID used for nonstandard 3DNow ID;
   3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */
-   __clear_bit(0*32+31, c->x86_capability);
+   __clear_bit(X86_FEATURE_3DNOW_ALT, c->x86_capability);

if (c->x86 == 0xf && c->x86_model < 0x14
&& cpu_has(c, X86_FEATURE_LAHF_LM)) {
diff --git a/xen/arch/x86/cpu/centaur.c b/xen/arch/x86/cpu/centaur.c
index 66b1995..c0ac117 100644
--- a/xen/arch/x86/cpu/centaur.c
+++ b/xen/arch/x86/cpu/centaur.c
@@ -39,10 +39,8 @@ static void init_c3(struct cpuinfo_x86 *c)
printk(KERN_INFO "CPU: Enabled h/w RNG\n");
}
 
-   /* store Centaur Extended Feature Flags as
-* word 5 of the CPU capability bit array
-*/
-   c->x86_capability[5] = cpuid_edx(0xC001);
+   c->x86_capability[cpufeat_word(X86_FEATURE_XSTORE)]
+= cpuid_edx(0xC001);
}
 
if (c->x86 == 0x6 && c->x86_model >= 0xf) {
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index a5caa84..653b052 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -200,13 +200,13 @@ static void __init early_cpu_detect(void)
if (c->x86 >= 0x6)
c->x86_model += ((tfms >> 16) & 0xF) << 4;
c->x86_mask = tfms & 15;
-   cap0 &= ~cleared_caps[0];
-   cap4 &= ~cleared_caps[4];
+   cap0 &= ~cleared_caps[cpufeat_word(X86_FEATURE_FPU)];
+   cap4 &= ~cleared_caps[cpufeat_word(X86_FEATURE_XMM3)];
if (cap0 & cpufeat_mask(X86_FEATURE_CLFLSH))
c->x86_cache_alignment = ((misc >> 8) & 0xff) * 8;
/* Leaf 0x1 capabilities filled in early for Xen. */
-   c->x86_capability[0] = cap0;
-   c->x86_capability[4] = cap4;
+   c->x86_capability[cpufeat_word(X86_FEATURE_FPU)] = cap0;
+   c->x86_capability[cpufeat_word(X86_FEATURE_XMM3)] = cap4;
 }
 
 static void __cpuinit generic_identify(struct cpuinfo_x86 *c)
@@ -225,8 +225,8 @@ static void __cpuinit generic_identify(struct cpuinfo_x86 
*c)

/* Intel-defined flags: level 0x0001 */
cpuid(0x0001, &tfms, &ebx, &excap, &capability);
-   c->x86_capability[0] = capability;
-   c->x86_capability[4] = excap;
+   c->x86_capability[cpufeat_word(X86_FEATURE_FPU)] = capability;
+   c->x86_capability[cpufeat_word(X86_FEATURE_XMM3)] = excap;
c->x86 = (tfms >> 8) & 15;
c->x86_model = (tfms >> 4) & 15;
if (c->x86 == 0xf)
@@ -247,8 +247,10 @@ static void __cpuinit generic_identify(struct cpuinfo_x86 
*c)
c->extended_cpuid_level = cpuid_eax(0x8000);
if ( (c->extended_cpuid_level & 0x) == 0x8000 ) {
if ( c->extended_cpuid_level >= 0x8001 ) {
-   c->x86_capability[1] = cpuid_edx(0x8001);
-   c->x86_capability[6] = cpuid_ecx(0x8001);
+   c->x86_capability[cpufeat_word(X86_FEATURE_SYSCALL)]
+   = cpuid_edx(0x8001);
+   c->x86_capability[cpufeat_word(X86_FEATURE_LAHF_LM)]
+   = cpuid_ecx(0x8001);
}
if ( c->extended_cpuid_level >= 0x8004 )
get_model_name(c); /* Default name */
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index d8ca862..6dbb14d 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -608,7 +608,8 @@ static void __init efi_arch_cpu(void)
 if ( cpuid_eax(0x8000) > 0x8000 )
 {
 cpuid_ext_features = cpuid_edx(0x8001);
-boot_cpu_data.x86_capability[1] = cpuid_ext_features;
+boot_cpu_data.x86_capability[cpufeat_word(X86_FEATURE_SYSCALL)]
+= cpuid_ext_features;
 }
 }
 
diff --git a/xen/arch/x86/mpparse.c b/xen/arch/x86/mpparse.c
index 8609f4a..dc74e37 100644
--- a/xen/arch/x86/mpparse.c
+++ b/xen/arch/x86/mpparse.c
@@ -495,7 +495,8 @@ static inline void __init construct_default_ISA_mptable(int 
mpc_default_type)
processor.mpc_cpufeature = (boot_cpu_data.x86 << 8) |
   (boot_cpu_data.x86_model << 4) |
   

[Xen-devel] [linux-3.10 test] 63306: regressions - trouble: broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-pvops3 host-install(3) broken in 63280 REGR. vs. 62642
 build-armhf-pvops 5 kernel-build  fail REGR. vs. 62642

Tests which are failing intermittently (not blocking):
 test-amd64-i386-libvirt-xsm   3 host-install(3)  broken in 63280 pass in 63306
 test-amd64-amd64-pygrub   3 host-install(3)   broken pass in 63224
 test-amd64-amd64-xl-qcow2 3 host-install(3)   broken pass in 63224
 test-amd64-i386-qemut-rhel6hvm-amd  3 host-install(3) broken pass in 63224
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 3 host-install(3) broken 
pass in 63224
 test-amd64-amd64-libvirt-vhd  3 host-install(3)   broken pass in 63224
 test-amd64-i386-xl-qemut-debianhvm-amd64 3 host-install(3) broken pass in 63280
 test-amd64-i386-rumpuserxen-i386 15 
rumpuserxen-demo-xenstorels/xenstorels.repeat fail pass in 63280

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 15 guest-localmigrate.2 
fail in 63224 like 62642
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail like 62642
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail like 62642

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-amd   1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-xl-pvh-intel  1 build-check(1)   blocked in 63280 n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)  blocked in 63280 n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-libvirt  1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked in 63280 n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked in 63280 n/a
 test-amd64-amd64-pygrub   1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-xl   1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked in 63280 n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-pair 1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked in 
63280 n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 1 build-check(1) blocked in 
63280 n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked in 
63280 n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked in 63280 n/a
 test-amd64-amd64-xl-qemut-debianhvm-amd64 1 build-check(1) blocked in 63280 n/a
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked 
in 63280 n/a
 test-amd64-amd64-xl-qemut-win7-amd64  1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-xl-qemuu-winxpsp3  1 build-check(1)  blocked in 63280 n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1)blocked in 63280 n/a
 test-amd64-amd64-xl-qemut-winxpsp3  1 build-check(1)  blocked in 63280 n/a
 test-amd64-amd64-libvirt-vhd 11 migrate-support-check fail in 63224 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-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-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-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass

version targeted for testing:
 linux61ce1520149bb1cfdc9a2946a1b6a33119742881
baseline version:
 linuxf5552cd830e58c46dffae3617b3ce0c839771981

Last test of basis62642  2015-10-03 17:59:45 Z   22 days
Testing same since63224  2015-10-22 22:20:05 Z3 days3 attempts

-

Re: [Xen-devel] [PATCH v3 0/9] libxc: support building large pv-domains

[Juergen Gross]

Ping?

On 10/13/2015 03:11 PM, Juergen Gross wrote:

The Xen hypervisor supports starting a dom0 with large memory (up to
the TB range) by not including the initrd and p2m list in the initial
kernel mapping. Especially the p2m list can grow larger than the
available virtual space in the initial mapping.

The started kernel is indicating the support of each feature via
elf notes.

This series enables the domain builder in libxc to do the same as the
hypervisor. This enables starting of huge pv-domUs via xl.

Unmapped initrd is supported for 64 and 32 bit domains, omitting the
p2m from initial kernel mapping is possible for 64 bit domains only.

Tested with:
- 32 bit domU (kernel not supporting unmapped initrd)
- 32 bit domU (kernel supporting unmapped initrd)
- 1 GB 64 bit domU (kernel supporting unmapped initrd, not p2m)
- 1 GB 64 bit domU (kernel supporting unmapped initrd and p2m)
- 900GB 64 bit domU (kernel supporting unmapped initrd and p2m)
- HVM domU

Changes in v3:
- Rebased the complete series to new staging (hvm builder patches by
   Roger Pau Monne)
- Removed old patch 1 as it broke stubdom build
- Introduced new Patch 1 to make allocation of guest memory more clear
   regarding virtual/physical memory allocation (requested by Ian Campbell)
- Change name of flag to indicate support of unmapped initrd in patch 2
   (requested by Ian Campbell)
- Introduce new patches 3, 4, 5 ("rename domain builder count_pgtables to
   alloc_pgtables", "introduce domain builder architecture specific data",
   "use domain builder architecture private data for x86 pv domains") to
   assist later page table work
- don't fiddle with initrd virtual address in patch 6 (was patch 3 in v2),
   add explicit initrd parameters for start_info in struct xc_dom_image
   instead (requested by Ian Campbell)
- Introduce new patch 8 ("rework of domain builder's page table handler")
   to be able to use common helpers for unmapped p2m list (requested by
   Ian Campbell)
- use now known segment size in pages for p2m list in patch 9 (was patch
   5 in v2) instead of fiddling with segment end address (requested by
   Ian Campbell)
- split alloc_p2m_list() in patch 9 (was patch 5 in v2) to 32/64 bit
   variants (requested by Ian Campbell)

Changes in v2:
- patch 2 has been removed as it has been applied already
- introduced new patch 2 as suggested by Ian Campbell: add a flag
   indicating support of an unmapped initrd to the parsed elf data of
   the elf_dom_parms structure
- updated patch description of patch 3 as requested by Ian Campbell


Juergen Gross (9):
   libxc: reorganize domain builder guest memory allocator
   xen: add generic flag to elf_dom_parms indicating support of unmapped
 initrd
   libxc: rename domain builder count_pgtables to alloc_pgtables
   libxc: introduce domain builder architecture specific data
   libxc: use domain builder architecture private data for x86 pv domains
   libxc: create unmapped initrd in domain builder if supported
   libxc: split p2m allocation in domain builder from other magic pages
   libxc: rework of domain builder's page table handler
   libxc: create p2m list outside of kernel mapping if supported

  stubdom/grub/kexec.c   |  12 +-
  tools/libxc/include/xc_dom.h   |  32 +--
  tools/libxc/xc_dom_arm.c   |   6 +-
  tools/libxc/xc_dom_core.c  | 174 +
  tools/libxc/xc_dom_x86.c   | 496 -
  xen/arch/x86/domain_build.c|   4 +-
  xen/common/libelf/libelf-dominfo.c |   3 +
  xen/include/xen/libelf.h   |   1 +
  8 files changed, 480 insertions(+), 248 deletions(-)




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

Re: [Xen-devel] ovmf fails to build in stagin-4.6

[Olaf Hering]

On Mon, Oct 26, Wei Liu wrote:

> Wait, so you're using gcc-5.1.1 but OVMF is reporting gcc-4.4 (see in
> the path of output string), there might be another problem with
> toolchain detection then.

As Linus said: detect old and known to be problematic, everything else has to
be handled as "current". But see tools/firmware/ovmf-dir-remote/OvmfPkg/build.sh

gcc_version=$(gcc -v 2>&1 | tail -1 | awk '{print $3}')
case $gcc_version in
  4.5.*)
TARGET_TOOLS=GCC45
;;
  4.6.*)
TARGET_TOOLS=GCC46
;;
  4.7.*)
TARGET_TOOLS=GCC47
;;
  4.8.*)
TARGET_TOOLS=GCC48
;;
  4.9.*|4.1[0-9].*)
TARGET_TOOLS=GCC49
;;
  *)
TARGET_TOOLS=GCC44
;;
esac

Olaf

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

Re: [Xen-devel] [xen-4.5-testing test] 63303: regressions - trouble: blocked/broken/fail/pass

[Julien Grall]

Hi Jan,

On 26/10/15 09:29, Jan Beulich wrote:
 On 26.10.15 at 08:07,  wrote:
>> flight 63303 xen-4.5-testing real [real]
>> http://logs.test-lab.xenproject.org/osstest/logs/63303/ 
>>
>> Regressions :-(
>>
>> Tests which did not succeed and are blocking,
>> including tests which could not be run:
>>  build-armhf   5 xen-build fail REGR. vs. 
>> 63099
> 
> This being
> 
> libxl_arm.c: In function 'libxl__arch_domain_init_hw_description':
> libxl_arm.c:591:76: error: 'state' undeclared (first use in this function)
> libxl_arm.c:591:76: note: each undeclared identifier is reported only once 
> for each function it appears in
> make[3]: *** [libxl_arm.o] Error 1
> 
> is 9befcd335c perhaps missing some prereq backport?
> 
> Since 4.5.2 is supposed to go out asap, can you please fix this or
> revert?

The parameter "state" has been introduced by
a7511905fae7ba592c5bf63cd77d8ff78087d689 "xen: Extend DOMCTL
createdomain to support arch configuration".

But this patch shouldn't be backported entirely. As Ian is away this
week, I would revert this patch and wait him to be back to fix it.

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] does cpu_down makes brings core to deepest sleep state?

[Hamed Rs]

>
>
> Did you look at the code generating the stats?

No.

> C0 gets accounted
> to everything not explicitly accounted to any other C state (i.e. only
> C1...Cn get explicit stats maintained).
>
> Jan
>
> I dont think so. C0 only accounted when all parts of cpu being on. I was
wondering if cpu_down turns off the cpu, why the 20 second sleep is
considered as C0?
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 8/9] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Wang, Wei W]

On 26/10/2015 18:52,  Jan Beulich wrote:
> >>> On 26.10.15 at 11:45,  wrote:
> > On 26/10/2015 18:37,  Jan Beulich wrote:
> >> >>> On 26.10.15 at 11:19,  wrote:
> >> > On 26/10/2015 17:54,  Jan Beulich wrote:
> >> >> That wasn't the question; I rather inquired what "meaning at the same
> time"
> >> >> both fields have.
> >> >
> >> > turbo_enable: indicates if turbo is enabled or not.
> >> > turbo_pct: shows the capability of turbo in percentage. For example
> >> > if the CPU has the following operating frequency range:
> >> > From [Min] to [Max]: 1.2GH, 1.3GHZ,1.4GHZ,...,2.3GHZ, so
> >> > 1.2--->2.3, length = 12 Turbo frequency: 3.7GHZ, so 1.2>3.7,
> >> > length = 26 Then turbo_pct = (26 - 12) / 26 = 54%, that is turbo_pct = 
> >> > 54.
> >>
> >> So what meaning does e.g. "turbo_enabled = 1" plus "turbo_pct = 0"
> >> have? Or "turbo_enabled = 0" plus "turbo_pct > 0"?
> >
> > " turbo_pct = 54" is the property of the CPU, just like MAX=3.7GHZ. It
> > should be something engraved on stone, not changeable. So, I think
> > it's not a good idea to put them together.
> 
> That's a valid statement, but not really an answer to the question(s) raised.

My point is that we should not discuss the two together, since they are not 
related to each other.
The answer is obvious:
"turbo_enabled = 1" plus "turbo_pct = 0", is not a possible case, since 
turbo_pct always equals to 54, as the valid statement states. However, if the 
CPU doesn't support turbo, then turbo_enabled won't be 1.
"turbo_enabled = 0" plus "turbo_pct =54", simply means that the CPU has its 
turbo function switched off, though it has a turbo capability of 54%.

Best,
Wei

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

Re: [Xen-devel] [PATCH v2] x86/mm: pod: Use the correct memory flags for alloc_domheap_page{, s}

[George Dunlap]

On 26/10/15 10:40, Jan Beulich wrote:
 On 26.10.15 at 11:29,  wrote:
>> On 23/10/15 11:33, Julien Grall wrote:
>>> Note that the patch has only been build tested.
>>
>> It would be nice if we could properly test the codepath in question
>> before checking it in, but we have lots of time before the release for
>> people to find this sort of thing.
> 
> To be honest, I'd rather put it in right away. There's no strict need
> to backport it, and in case there is a problem we can always fix/revert
> in -unstable. Most of the changes to memory hot (un)plug paths go
> in that way, due to the rareness of systems to test such on.

Indeed, that's what I meant -- we should check it in right away, to
maximize the possibility that if there is a bug, it will be caught in
all the testing (both ad-hoc and explicit) that will happen between now
and the next release.

 -George

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

Re: [Xen-devel] [PATCH] xl: Die on unknown options

[Wei Liu]

On Fri, Oct 23, 2015 at 04:44:11PM +0100, Ian Jackson wrote:
> def_getopt would print a message to stderr, but blunder on anyway.
> 
> Sadly this is probably not a backport candidate.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Wei Liu 

> ---
>  tools/libxl/xl_cmdimpl.c |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index ea43761..de28593 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2759,6 +2759,7 @@ static int def_getopt(int argc, char * const argv[],
>  exit(0);
>  }
>  fprintf(stderr, "option `%c' not supported.\n", optopt);
> +exit(2);
>  }
>  if (opt == 'h') {
>  help(helpstr);
> -- 
> 1.7.10.4

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

Re: [Xen-devel] [PATCH v5 8/9] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Jan Beulich]

>>> On 26.10.15 at 11:45,  wrote:
> On 26/10/2015 18:37,  Jan Beulich wrote:
>> >>> On 26.10.15 at 11:19,  wrote:
>> > On 26/10/2015 17:54,  Jan Beulich wrote:
>> >> That wasn't the question; I rather inquired what "meaning at the same 
>> >> time"
>> >> both fields have.
>> >
>> > turbo_enable: indicates if turbo is enabled or not.
>> > turbo_pct: shows the capability of turbo in percentage. For example if
>> > the CPU has the following operating frequency range:
>> > From [Min] to [Max]: 1.2GH, 1.3GHZ,1.4GHZ,...,2.3GHZ, so 1.2--->2.3,
>> > length = 12 Turbo frequency: 3.7GHZ, so 1.2>3.7, length = 26 Then
>> > turbo_pct = (26 - 12) / 26 = 54%, that is turbo_pct = 54.
>> 
>> So what meaning does e.g. "turbo_enabled = 1" plus "turbo_pct = 0"
>> have? Or "turbo_enabled = 0" plus "turbo_pct > 0"?
> 
> " turbo_pct = 54" is the property of the CPU, just like MAX=3.7GHZ. It 
> should be something engraved on stone, not changeable. So, I think it's not a 
> good idea to put them together.

That's a valid statement, but not really an answer to the question(s)
raised.

Jan


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

Re: [Xen-devel] does cpu_down makes brings core to deepest sleep state?

[Jan Beulich]

>>> On 26.10.15 at 11:40,  wrote:
> On Mon, Oct 26, 2015 at 1:14 PM, Jan Beulich  wrote:
>> ing maintained by (and hence re

This is rather too little context you left in place.

> I only turned off the cpu which has ID = 1 from 80th second to 100th
> second. As you can see in the result of "xenpm get-cpuidle-state" in time
> 100.7, the residency in C0 of cpu1 is equal to C0 of cpu0! How it is
> possible? if the software does not maintain statistics, the results must be
> 80 sec residency for cpu1.

Did you look at the code generating the stats? C0 gets accounted
to everything not explicitly accounted to any other C state (i.e. only
C1...Cn get explicit stats maintained).

Jan


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

Re: [Xen-devel] Understanding Xen event channel bit operation

[David Vrabel]

On 25/10/15 09:25, amin.fall...@gmail.com wrote:
> Hi everybody
> I need to understand when these pending and mask bits are set and
> cleared. It seems pending bits are set by evtchn_set_pending method in
> event_channel.c but I don't understand where pending bit is cleared by
> the guest and where mask bit is set and reset?

The mask and pending bits are in memory shared with the guest and the
guest (mostly) modifies these bits by writing directly to the shared memory.

You need to look at the guest kernel (e.g.,
drivers/xen/events/events_base.c in Linux).

David

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

[Xen-devel] [PATCH v2 0/5] disambiguate symbol names (part 2)

[Jan Beulich]

Triggered by Konrad's needs for xSplice, but having been on my todo
list for a very long time to help interpretation of stack traces, here's a
revised remainder of changes allowing to tell apart identically named
static symbols. The main and general thing is to prefix them by file
name. Some remaining collisions need other treatment: In some cases
the duplicates simply can be folded (most of which already went in). In
other cases renaming is necessary. And then there are possibly -
compiler dependent - a few collisions left in place by the end of this
series, but in v2 it's just one that I actively observed (in cpufreq.c).

01: symbols: prefix static symbols with their source file names
02: compat: enforce distinguishable file names in symbol table
03: x86/mm: override stored file names for multiply built sources
04: x86/mm: build map_domain_gfn() just once
05: x86/mm: only a single instance of gw_page_flags[] is needed

Signed-off-by: Jan Beulich 


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

Re: [Xen-devel] [PATCH v5 8/9] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Wang, Wei W]

On 26/10/2015 18:37,  Jan Beulich wrote:
> >>> On 26.10.15 at 11:19,  wrote:
> > On 26/10/2015 17:54,  Jan Beulich wrote:
> >> That wasn't the question; I rather inquired what "meaning at the same time"
> >> both fields have.
> >
> > turbo_enable: indicates if turbo is enabled or not.
> > turbo_pct: shows the capability of turbo in percentage. For example if
> > the CPU has the following operating frequency range:
> > From [Min] to [Max]: 1.2GH, 1.3GHZ,1.4GHZ,...,2.3GHZ, so 1.2--->2.3,
> > length = 12 Turbo frequency: 3.7GHZ, so 1.2>3.7, length = 26 Then
> > turbo_pct = (26 - 12) / 26 = 54%, that is turbo_pct = 54.
> 
> So what meaning does e.g. "turbo_enabled = 1" plus "turbo_pct = 0"
> have? Or "turbo_enabled = 0" plus "turbo_pct > 0"?

" turbo_pct = 54" is the property of the CPU, just like MAX=3.7GHZ. It should 
be something engraved on stone, not changeable. So, I think it's not a good 
idea to put them together.

Best,
Wei

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

Re: [Xen-devel] does cpu_down makes brings core to deepest sleep state?

[Hamed Rs]

On Mon, Oct 26, 2015 at 1:14 PM, Jan Beulich  wrote:

> ing maintained by (and hence re


I only turned off the cpu which has ID = 1 from 80th second to 100th
second. As you can see in the result of "xenpm get-cpuidle-state" in time
100.7, the residency in C0 of cpu1 is equal to C0 of cpu0! How it is
possible? if the software does not maintain statistics, the results must be
80 sec residency for cpu1.
The other possible result may be 80 sec residency for C0 and 20 sec for C4
( according to 20 second residency in CC6).

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

Re: [Xen-devel] [PATCH v2 5/5] xl: improve return and exit codes of parse related functions

[Dario Faggioli]

On Sat, 2015-10-24 at 11:01 +0530, Harmandeep Kaur wrote:
> turning  parsing related functions xl exit codes towards using the
> EXIT_[SUCCESS|FAILURE] macros, instead of instead of arbitrary
> numbers
> or libxl return codes.
> 
> it doesn't include parse_config_data() which is big enough to deserve
> its
> own patch
> 
> Signed-off-by: Harmandeep Kaur 

> libxl_bitmap *cpumap)
>  static int cpurange_parse(const char *cpu, libxl_bitmap *cpumap)
>  {
>  char *ptr, *saveptr = NULL, *buf = strdup(cpu);
> -int rc = 0;
>  
>  for (ptr = strtok_r(buf, ",", &saveptr); ptr;
>   ptr = strtok_r(NULL, ",", &saveptr)) {
> -rc = update_cpumap_range(ptr, cpumap);
> -if (rc)
> +if (update_cpumap_range(ptr, cpumap))
>  break;
>  }
>  free(buf);
>  
> -return rc;
> +return 0;
>  }
>  
Oh, and also, here: I think rc is needed, in this case, to properly
deal with the failure of update_cpumap_range(), and poperly propagate
that failure to the caller.

If you want to get rid of it, you should do something like this, inside
the loop:

 if (update_cpumap_range(ptr, cpumap)) {
 free(buf);
 return 1;
 }

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



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

Re: [Xen-devel] [PATCH] Handles the error returned by the xc_dom_allocate function

[Wei Liu]

Aside from what Dario said.

On Sun, Oct 25, 2015 at 03:32:24PM +0530, Lasya Venneti wrote:
> ---
>  tools/xenstore/init-xenstore-domain.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/xenstore/init-xenstore-domain.c 
> b/tools/xenstore/init-xenstore-domain.c
> index 0d12169..d17aab5 100644
> --- a/tools/xenstore/init-xenstore-domain.c
> +++ b/tools/xenstore/init-xenstore-domain.c
> @@ -42,6 +42,8 @@ static int build(xc_interface *xch, int argc, char** argv)
>   snprintf(cmdline, 512, "--event %d --internal-db", rv);
>  
>   dom = xc_dom_allocate(xch, cmdline, NULL);
> + if(dom==NULL)

Coding style is wrong. It should be

if (dom == NULL)

Note the whitespaces.


> + return -1;

And, please set rv to a proper error code (presumably ENOMEM) and use
goto err, otherwise you're leaking xs_fd.

BTW I notice that xs_fd is leaked in success path. You can submit
another path for it if you feel keen enough.

Wei.

>   rv = xc_dom_kernel_file(dom, argv[1]);
>   if (rv) goto err;
>  
> -- 
> 1.9.1

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

Re: [Xen-devel] [PATCH v2] x86/mm: pod: Use the correct memory flags for alloc_domheap_page{, s}

[Jan Beulich]

>>> On 26.10.15 at 11:29,  wrote:
> On 23/10/15 11:33, Julien Grall wrote:
>> Note that the patch has only been build tested.
> 
> It would be nice if we could properly test the codepath in question
> before checking it in, but we have lots of time before the release for
> people to find this sort of thing.

To be honest, I'd rather put it in right away. There's no strict need
to backport it, and in case there is a problem we can always fix/revert
in -unstable. Most of the changes to memory hot (un)plug paths go
in that way, due to the rareness of systems to test such on.

Jan


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

Re: [Xen-devel] [PATCH net-next 0/8] xen-netback/core: packet hashing

[David Vrabel]

On 24/10/15 12:55, David Miller wrote:
> From: Paul Durrant 
> Date: Wed, 21 Oct 2015 11:36:17 +0100
> 
>> This series adds xen-netback support for hash negotiation with a frontend
>> driver, and an implementation of toeplitz hashing as the initial negotiable
>> algorithm.
> 
> Ping, I want to see some review from some other xen networking folks.

There's been some review of the front/back protocol (on a different
thread) and some significant changes have been suggested.

David

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

Re: [Xen-devel] [PATCH v5 8/9] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Jan Beulich]

>>> On 26.10.15 at 11:19,  wrote:
> On 26/10/2015 17:54,  Jan Beulich wrote:
>> That wasn't the question; I rather inquired what "meaning at the same time"
>> both fields have.
> 
> turbo_enable: indicates if turbo is enabled or not.
> turbo_pct: shows the capability of turbo in percentage. For example if the 
> CPU has the following operating frequency range:
> From [Min] to [Max]: 1.2GH, 1.3GHZ,1.4GHZ,...,2.3GHZ, so 1.2--->2.3, length = 
> 12
> Turbo frequency: 3.7GHZ, so 1.2>3.7, length = 26
> Then turbo_pct = (26 - 12) / 26 = 54%, that is turbo_pct = 54.

So what meaning does e.g. "turbo_enabled = 1" plus "turbo_pct = 0"
have? Or "turbo_enabled = 0" plus "turbo_pct > 0"?

Jan


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

Re: [Xen-devel] [PATCH v2 5/5] xl: improve return and exit codes of parse related functions

[Dario Faggioli]

On Sat, 2015-10-24 at 11:01 +0530, Harmandeep Kaur wrote:
> turning  parsing related functions xl exit codes towards using the
> EXIT_[SUCCESS|FAILURE] macros, instead of instead of arbitrary
> numbers
> or libxl return codes.
> 
I think the changelog, in this case, can be restructured and improved
just like I said for patch 2.

> it doesn't include parse_config_data() which is big enough to deserve
> its
> own patch
> 
"Don't touch parse_config_data() which..."

> Signed-off-by: Harmandeep Kaur 
>
Again, this is almost ok, but with some issues:

> @@ -600,7 +600,7 @@ static void parse_vif_rate(XLU_Config **config,
> const char *rate,
>  if (e == EINVAL || e == EOVERFLOW) exit(-1);
>
What about this one? :-D :-D

>  if (e) {
>  fprintf(stderr,"xlu_vif_parse_rate failed:
> %s\n",strerror(errno));
> -exit(-1);
> +exit(EXIT_FAILURE);
>  }
>  }

> @@ -3126,7 +3124,7 @@ static int64_t parse_mem_size_kb(const char
> *mem)
>  kbytes = strtoll(mem, &endptr, 10);
>  
>  if (strlen(endptr) > 1)
> -return -1;
> +return 1;
>  
>  switch (tolower((uint8_t)*endptr)) {
>  case 't':
> @@ -3145,7 +3143,7 @@ static int64_t parse_mem_size_kb(const char
> *mem)
>  kbytes >>= 10;
>  break;
>  default:
> -return -1;
> +return 1;
>  }
>  
>  return kbytes;
>
I see why you're doing this, and I saw you're took care of the call
sites, which is good.

However, in this case, I think the return value should stay -1. Tools
people may advise better than me, but it looks like this function is
meant at returning the size of some amount of memory, in kilobytes. So,
although I agree that it would be very unlikely that someone specifies
1 Kb, we really can't rule it out (not in this patch, at least!).

So, I really think it's better to keep using negative numbers here (on
the ground that a negative amount of memory is a clearer indication of
an error).

One good thing that you can do in the case of this function is, maybe,
adding a comment just above it saying (very quickly, as it's pretty
evident and straightforward, IMO) exactly that, i.e., that the
functions returns -1 if the parsing fails.

> @@ -3259,17 +3257,14 @@ static int def_getopt(int argc, char * const
> argv[],
>  static int set_memory_max(uint32_t domid, const char *mem)
>  {
>  int64_t memorykb;
> -int rc;
>  
>  memorykb = parse_mem_size_kb(mem);
> -if (memorykb == -1) {
> +if (memorykb == 1) {
>
This will therefore be left as it is, of course.

>  fprintf(stderr, "invalid memory size: %s\n", mem);
> -exit(3);
> +exit(EXIT_FAILURE);
>  }
> 
While this is, of course, ok.
 
>  int main_memmax(int argc, char **argv)
> @@ -3277,7 +3272,6 @@ int main_memmax(int argc, char **argv)
>  uint32_t domid;
>  int opt = 0;
>  char *mem;
> -int rc;
>  
>  SWITCH_FOREACH_OPT(opt, "", NULL, "mem-max", 2) {
>  /* No options */
> @@ -3286,8 +3280,7 @@ int main_memmax(int argc, char **argv)
>  domid = find_domain(argv[optind]);
>  mem = argv[optind + 1];
>  
> -rc = set_memory_max(domid, mem);
> -if (rc) {
> +if (set_memory_max(domid, mem)){
>  fprintf(stderr, "cannot set domid %d static max memory to :
> %s\n", domid, mem);
>  return 1;
>  }
I'm not sure about this specific change. It is ok, of course, but it
seems a bit out of what you declared the scope of this patch was.

In fact, you are fixing and improving error reporting and exit codes,
not getting rid of unnecessary variables. Then, yes, in most cases mean
we can get rid of those variables, as a result of the declared goal,
but in this case there is no return value/exit code involved. At the
end, I think you should leave this hunk out of this patch.

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



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

Re: [Xen-devel] [PATCH v2] x86/mm: pod: Use the correct memory flags for alloc_domheap_page{, s}

[George Dunlap]

On 23/10/15 11:33, Julien Grall wrote:
> The last parameter of alloc_domheap_page{s,} contain the memory flags and
> not the order of the allocation.
> 
> Use 0 for the call in p2m_pod_set_cache_target as it was before
> 1069d63c5ef2510d08b83b2171af660e5bb18c63 "x86/mm/p2m: use defines for
> page sizes". Note that PAGE_ORDER_4K is also equal to 0 so the behavior
> stays the same.
> 
> For the call in p2m_pod_offline_or_broken_replace we want to allocate
> the new page on the same numa node as the previous page. So retrieve the
> numa node and pass it in the memory flags.
> 
> Signed-off-by: Julien Grall 

Acked-by: George Dunlap 

> 
> ---
> 
> Note that the patch has only been build tested.

It would be nice if we could properly test the codepath in question
before checking it in, but we have lots of time before the release for
people to find this sort of thing.

 -George


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

Re: [Xen-devel] [PATCH v5 8/9] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Wang, Wei W]

On 26/10/2015 17:54,  Jan Beulich wrote:
> >>> On 26.10.15 at 10:48,  wrote:
> > On 26/10/2015 17:42,  Jan Beulich wrote:
> >> >>> On 26.10.15 at 08:59,  wrote:
> >> > On 26/10/2015 15:03,  Jan Beulich wrote:
> >> >> >>> "Wang, Wei W"  10/26/15 7:27 AM >>>
> >> >> >On 08/10/2015 12:11,  Jan Beulich wrote:
> >> >> >> >>> On 14.09.15 at 04:32,  wrote:
> >> >> >> > @@ -309,23 +326,13 @@ struct xen_get_cpufreq_para {
> >> >> >> >  XEN_GUEST_HANDLE_64(uint32) scaling_available_frequencies;
> >> >> >> >  XEN_GUEST_HANDLE_64(char)   scaling_available_governors;
> >> >> >> >  char scaling_driver[CPUFREQ_NAME_LEN];
> >> >> >> > -
> >> >> >> > -uint32_t cpuinfo_cur_freq;
> >> >> >> > -uint32_t cpuinfo_max_freq;
> >> >> >> > -uint32_t cpuinfo_min_freq;
> >> >> >> > -uint32_t scaling_cur_freq;
> >> >> >> > -
> >> >> >> >  char scaling_governor[CPUFREQ_NAME_LEN];
> >> >> >> > -uint32_t scaling_max_freq;
> >> >> >> > -uint32_t scaling_min_freq;
> >> >> >> >
> >> >> >> >  /* for specific governor */
> >> >> >> >  union {
> >> >> >> >  struct  xen_userspace userspace;
> >> >> >> >  struct  xen_ondemand ondemand;
> >> >> >> >  } u;
> >> >> >> > -
> >> >> >> > -int32_t turbo_enabled;
> >> >> >> >  };
> >> >> >>
> >> >> >> Is all of this re-arrangement really needed? Also can't
> >> >> >> turbo_enabled and scaling_turbo_pct be combined into a single field?
> >> >> >
> >> >> >Personally, we should not combine the two.
> >> >> > turbo_enabled is used by both the old pstate driver and
> >> >> >intel_pstate, but  scaling_turbo_pct is only used in intel_pstate.
> >> >> >If we use
> >> >> "scaling_turbo_pct=0"
> >> >> > to represent "turbo_enabled=0", and "scaling_turbo_pct>0" to
> >> >> >represent " turbo_enabled=1", then we will need to modify the old
> >> >> >driver to use scaling_turbo_pct, i.e. changing the old driver to
> >> >> >be aware
> > of
> >> the "percentage"
> >> >> > concept, which is proposed in intel_pstate. On the other side, I
> >> >> >think keeping  turbo_enabled and scaling_turbo_pct separated
> >> >> >makes the code
> >> >> easier to read.
> >> >>
> >> >> Note that "combine" doesn't necessarily mean "eliminate the old one"
> >> >> - they could as well become field of a union. The basic question
> >> >> you should ask yourself in such cases is: "Do both fields have a
> >> >> meaning at the same time?" If the answer is yes, then of course
> >> >> they should remain separate. If the answer is no _and_ their
> >> >> purpose is reasonably similar, then combining them should at least be
> considered.
> >> >
> >> > Ok. I will keep the two separated, since they do have their own
> >> > meaning at the same time.
> >>
> >> Being which?
> >
> > Keeping both of the two there. Just as what they are now - two
> > independent fields.
> 
> That wasn't the question; I rather inquired what "meaning at the same time"
> both fields have.

turbo_enable: indicates if turbo is enabled or not.
turbo_pct: shows the capability of turbo in percentage. For example if the CPU 
has the following operating frequency range:
>From [Min] to [Max]: 1.2GH, 1.3GHZ,1.4GHZ,...,2.3GHZ, so 1.2--->2.3, length = 
>12
Turbo frequency: 3.7GHZ, so 1.2>3.7, length = 26
Then turbo_pct = (26 - 12) / 26 = 54%, that is turbo_pct = 54.

Best,
Wei


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

Re: [Xen-devel] ovmf fails to build in stagin-4.6

[Wei Liu]

On Mon, Oct 26, 2015 at 11:11:16AM +0100, Olaf Hering wrote:
> On Mon, Oct 26, Wei Liu wrote:
> 
> > On Mon, Oct 26, 2015 at 10:43:15AM +0100, Olaf Hering wrote:
> 
> > > Does it compile for anyone?
> > It compiles for me -- but I'm using gcc 4.9.
> 
> I noticed that just now, fails only in Tumbleweed which used gcc-5.1.1.
> Sorry for the noise.
> 

Wait, so you're using gcc-5.1.1 but OVMF is reporting gcc-4.4 (see in
the path of output string), there might be another problem with
toolchain detection then.

Wei.

> Olaf

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

Re: [Xen-devel] ovmf fails to build in stagin-4.6

[Olaf Hering]

On Mon, Oct 26, Wei Liu wrote:

> On Mon, Oct 26, 2015 at 10:43:15AM +0100, Olaf Hering wrote:

> > Does it compile for anyone?
> It compiles for me -- but I'm using gcc 4.9.

I noticed that just now, fails only in Tumbleweed which used gcc-5.1.1.
Sorry for the noise.

Olaf

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

Re: [Xen-devel] [PATCH v2 4/5] xl: improve return and exit codes of cpupool related functions

[Dario Faggioli]

On Sat, 2015-10-24 at 11:01 +0530, Harmandeep Kaur wrote:
> turning  cpupools related functions xl exit codes towards using the
> EXIT_[SUCCESS|FAILURE] macros, instead of instead of arbitrary
> numbers
> or libxl return codes.
> 
Ditch "xl" :-)

With that:

> Signed-off-by: Harmandeep Kaur 
>
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&D Ltd., Cambridge (UK)



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

Re: [Xen-devel] ovmf fails to build in stagin-4.6

[Wei Liu]

On Mon, Oct 26, 2015 at 10:43:15AM +0100, Olaf Hering wrote:
> For me ovmf fails to build in staging-4.6:
> 
> ...
> [  541s] + ./configure --host=x86_64-suse-linux-gnu 
> --build=x86_64-suse-linux-gnu --program-prefix= --disable-dependency-tracking 
> --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin 
> --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include 
> --libdir=/usr/lib64 --libexecdir=/usr/lib --localstatedir=/var 
> --sharedstatedir=/usr/com --mandir=/usr/share/man --infodir=/usr/share/info 
> --disable-dependency-tracking --enable-debug --disable-stubdom --disable-xen 
> --docdir=/usr/share/doc/packages/xen --enable-ovmf --enable-docs 
> --enable-tools --enable-systemd --with-systemd=/usr/lib/systemd/system 
> --with-systemd-modules-load=/usr/lib/modules-load.d --enable-stubdom 
> --enable-ioemu-stubdom --disable-c-stubdom --disable-caml-stubdom 
> --disable-vtpm-stubdom --enable-vtpmmgr-stubdom --with-initddir=/etc/init.d
> ...
> [  561s] + make -j2 debug_symbols=n -k
> ...
> [  727s] Building ... 
> /home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiHobLib/PeiHobLib.inf
>  [X64]
> [  727s] Building ... 
> /home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
>  [X64]
> [  727s] make[7]: Entering directory 
> '/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/Build/OvmfX64/RELEASE_GCC44/X64/MdePkg/Library/PeiHobLib/PeiHobLib'
> [  727s] "gcc" -g -fshort-wchar -fno-strict-aliasing -Wall -Werror 
> -Wno-array-bounds -ffunction-sections -fdata-sections -c -include AutoGen.h 
> -DSTRING_ARRAY_NAME=PeiHobLibStrings -m64 -fno-stack-protector 
> "-DEFIAPI=__attribute__((ms_abi))" -DNO_BUILTIN_VA_FUNCS -mno-red-zone 
> -Wno-address -mcmodel=large -DMDEPKG_NDEBUG -mno-mmx -mno-sse -o 
> /home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/Build/OvmfX64/RELEASE_GCC44/X64/MdePkg/Library/PeiHobLib/PeiHobLib/OUTPUT/./HobLib.obj
>  
> -I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiHobLib
>  
> -I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/Build/OvmfX64/RELEASE_GCC44/X64/MdePkg/Library/PeiHobLib/PeiHobLib/DEBUG
>  
> -I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg
>  -I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dc
 b/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Include 
-I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Include/X64
 
/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiHobLib/HobLib.c
> [  727s] make[7]: Entering directory 
> '/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/Build/OvmfX64/RELEASE_GCC44/X64/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt'
> [  727s] "gcc" -g -fshort-wchar -fno-strict-aliasing -Wall -Werror 
> -Wno-array-bounds -ffunction-sections -fdata-sections -c -include AutoGen.h 
> -DSTRING_ARRAY_NAME=PeiServicesTablePointerLibIdtStrings -m64 
> -fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))" -DNO_BUILTIN_VA_FUNCS 
> -mno-red-zone -Wno-address -mcmodel=large -DMDEPKG_NDEBUG -mno-mmx -mno-sse 
> -o 
> /home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/Build/OvmfX64/RELEASE_GCC44/X64/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt/OUTPUT/./PeiServicesTablePointer.obj
>  
> -I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiServicesTablePointerLibIdt
>  
> -I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/Build/OvmfX64/RELEASE_GCC44/X64/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt/DEBUG
>  -I/home/abuild/rpmbuild/BUILD/x
 en-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg 
-I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Include
 
-I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Include/X64
 
/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointer.c
> [  727s] 
> /home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointer.c:
>  In function 'MigratePeiServicesTablePointer':
> [  727s] 
> /h

Re: [Xen-devel] [PATCH v2 3/5] xl: improve return and exit codes of vcpu related functions

[Dario Faggioli]

On Sat, 2015-10-24 at 11:01 +0530, Harmandeep Kaur wrote:
> turning vcpu manipulation functions xl exit codes toward using the
> EXIT_[SUCCESS|FAILURE] macros, instead of instead of arbitrary
> numbers
> or libxl return codes.
> 
Again, distch "xl" from the sentence above.

Again, just one small comment:

> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c

> @@ -5508,7 +5506,9 @@ int main_vcpuset(int argc, char **argv)
>  break;
>  }
>  
> -return vcpuset(find_domain(argv[optind]), argv[optind + 1],
> check_host);
> +if (vcpuset(find_domain(argv[optind]), argv[optind + 1],
> check_host))
> +return EXIT_FAILURE;
> +else return EXIT_SUCCESS;
>
There's no need of this 'else'.

With these things fixed, this patch is:

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&D Ltd., Cambridge (UK)



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

Re: [Xen-devel] [PATCH v2 2/5] xl: improve return and exit codes of scheduling related functions

[Dario Faggioli]

On Sat, 2015-10-24 at 11:01 +0530, Harmandeep Kaur wrote:
> turning scheduling related functions xl exit codes towards using the
> EXIT_[SUCCESS|FAILURE] macros, instead of instead of arbitrary
> numbers
> or libxl return codes.
> 
"turning scheduling related xl functions exit codes..."

However, it is already clear from the "xl:" tag in the subject that
you'll be dealing with xl code, so I think you can just remove "xl"
from here.

Also, I'd (quickly) mention the fact that you are actually doing two
conversions in this patch:
 - for main_*: arbitrary --> EXIT_SUCCESS|EXIT_FAILURe
 - for internal fucntion: arbitrary --> 0/1

Finally, when sending new versions of a patch, it is rather useful that
you include:
 - in the cover letter, a summary of what changed, overall, in that
particular new version of the series (big-ish changes, and/or changes
related on the series structure, like new patch being added, etc.);
 - in each patch, a summary of what changed in that particular patch.
That's helpful for people doing the reviews, as it make quite easi to
check whether you actually considered all points that where raised
during previous iteration.

The latter, you can put (I usually do it in bulleted list form) in...
> Signed-off-by: Harmandeep Kaur 
> ---
>
...here, i.e., after the "---" mark just below the Signed-off-by: tag.
This way, git will throw it away when committing the patch, which is a
good thing (in fact, we want the summary to be there during review, but
we don't want it in `git log').

About the patch, this is really good, and we're almost there (AFAICT).
I think I found an issue, though, here:

> @@ -5975,13 +5965,12 @@ static int
> sched_domain_output(libxl_scheduler sched, int (*output)(int),
>  libxl_cpupoolinfo *poolinfo = NULL;
>  uint32_t poolid;
>  int nb_domain, n_pools = 0, i, p;
> -int rc = 0;
>  
>  if (cpupool) {
>  if (libxl_cpupool_qualifier_to_cpupoolid(ctx, cpupool,
> &poolid, NULL) ||
>  !libxl_cpupoolid_is_valid(ctx, poolid)) {
>  fprintf(stderr, "unknown cpupool \'%s\'\n", cpupool);
> -return -ERROR_FAIL;
> +return 1;
>  }
>  }
>  
> @@ -5994,10 +5983,10 @@ static int
> sched_domain_output(libxl_scheduler sched, int (*output)(int),
>  if (!poolinfo) {
>  fprintf(stderr, "error getting cpupool info\n");
>  libxl_dominfo_list_free(info, nb_domain);
> -return -ERROR_NOMEM;
> +return 1;
>  }
>  
> -for (p = 0; !rc && (p < n_pools); p++) {
> +for (p = 0; p < n_pools; p++) {
>  if ((poolinfo[p].sched != sched) ||
>  (cpupool && (poolid != poolinfo[p].poolid)))
>  continue;
>
I don't think we can just get rid of rc in this way. In fact...


> @@ -6008,8 +5997,7 @@ static int sched_domain_output(libxl_scheduler
> sched, int (*output)(int),
>  for (i = 0; i < nb_domain; i++) {
>  if (info[i].cpupool != poolinfo[p].poolid)
>  continue;
> -rc = output(info[i].domid);
> -if (rc)
> +if (output(info[i].domid))
>  break;
>
... if the call to whatever output points fails, we break out from the
inner loop, but not from the outer one, while the original code wanted
us to leave that one too.

So, I think in this case you should keep rc, or use another strategy,
involving 'return'-s or 'goto'-s (but I think just keeping rc is the
easiest and better solution).

It is quite possible that I suggested you to do this during v1's
review. If yes, that was because I must have missed the "!rc" part of
the outer loop condition... Sorry for that. :-P

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



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

Re: [Xen-devel] [PATCH v5 8/9] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Jan Beulich]

>>> On 26.10.15 at 10:48,  wrote:
> On 26/10/2015 17:42,  Jan Beulich wrote:
>> >>> On 26.10.15 at 08:59,  wrote:
>> > On 26/10/2015 15:03,  Jan Beulich wrote:
>> >> >>> "Wang, Wei W"  10/26/15 7:27 AM >>>
>> >> >On 08/10/2015 12:11,  Jan Beulich wrote:
>> >> >> >>> On 14.09.15 at 04:32,  wrote:
>> >> >> > @@ -309,23 +326,13 @@ struct xen_get_cpufreq_para {
>> >> >> >  XEN_GUEST_HANDLE_64(uint32) scaling_available_frequencies;
>> >> >> >  XEN_GUEST_HANDLE_64(char)   scaling_available_governors;
>> >> >> >  char scaling_driver[CPUFREQ_NAME_LEN];
>> >> >> > -
>> >> >> > -uint32_t cpuinfo_cur_freq;
>> >> >> > -uint32_t cpuinfo_max_freq;
>> >> >> > -uint32_t cpuinfo_min_freq;
>> >> >> > -uint32_t scaling_cur_freq;
>> >> >> > -
>> >> >> >  char scaling_governor[CPUFREQ_NAME_LEN];
>> >> >> > -uint32_t scaling_max_freq;
>> >> >> > -uint32_t scaling_min_freq;
>> >> >> >
>> >> >> >  /* for specific governor */
>> >> >> >  union {
>> >> >> >  struct  xen_userspace userspace;
>> >> >> >  struct  xen_ondemand ondemand;
>> >> >> >  } u;
>> >> >> > -
>> >> >> > -int32_t turbo_enabled;
>> >> >> >  };
>> >> >>
>> >> >> Is all of this re-arrangement really needed? Also can't
>> >> >> turbo_enabled and scaling_turbo_pct be combined into a single field?
>> >> >
>> >> >Personally, we should not combine the two.
>> >> > turbo_enabled is used by both the old pstate driver and
>> >> >intel_pstate, but  scaling_turbo_pct is only used in intel_pstate.
>> >> >If we use
>> >> "scaling_turbo_pct=0"
>> >> > to represent "turbo_enabled=0", and "scaling_turbo_pct>0" to
>> >> >represent " turbo_enabled=1", then we will need to modify the old
>> >> >driver to use scaling_turbo_pct, i.e. changing the old driver to be 
>> >> >aware 
> of
>> the "percentage"
>> >> > concept, which is proposed in intel_pstate. On the other side, I
>> >> >think keeping  turbo_enabled and scaling_turbo_pct separated makes
>> >> >the code
>> >> easier to read.
>> >>
>> >> Note that "combine" doesn't necessarily mean "eliminate the old one"
>> >> - they could as well become field of a union. The basic question you
>> >> should ask yourself in such cases is: "Do both fields have a meaning
>> >> at the same time?" If the answer is yes, then of course they should
>> >> remain separate. If the answer is no _and_ their purpose is
>> >> reasonably similar, then combining them should at least be considered.
>> >
>> > Ok. I will keep the two separated, since they do have their own
>> > meaning at the same time.
>> 
>> Being which?
> 
> Keeping both of the two there. Just as what they are now - two independent 
> fields.

That wasn't the question; I rather inquired what "meaning at the same
time" both fields have.

Jan


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

Re: [Xen-devel] [PATCH v5 8/9] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Wang, Wei W]

On 26/10/2015 17:42,  Jan Beulich wrote:
> >>> On 26.10.15 at 08:59,  wrote:
> > On 26/10/2015 15:03,  Jan Beulich wrote:
> >> >>> "Wang, Wei W"  10/26/15 7:27 AM >>>
> >> >On 08/10/2015 12:11,  Jan Beulich wrote:
> >> >> >>> On 14.09.15 at 04:32,  wrote:
> >> >> > @@ -309,23 +326,13 @@ struct xen_get_cpufreq_para {
> >> >> >  XEN_GUEST_HANDLE_64(uint32) scaling_available_frequencies;
> >> >> >  XEN_GUEST_HANDLE_64(char)   scaling_available_governors;
> >> >> >  char scaling_driver[CPUFREQ_NAME_LEN];
> >> >> > -
> >> >> > -uint32_t cpuinfo_cur_freq;
> >> >> > -uint32_t cpuinfo_max_freq;
> >> >> > -uint32_t cpuinfo_min_freq;
> >> >> > -uint32_t scaling_cur_freq;
> >> >> > -
> >> >> >  char scaling_governor[CPUFREQ_NAME_LEN];
> >> >> > -uint32_t scaling_max_freq;
> >> >> > -uint32_t scaling_min_freq;
> >> >> >
> >> >> >  /* for specific governor */
> >> >> >  union {
> >> >> >  struct  xen_userspace userspace;
> >> >> >  struct  xen_ondemand ondemand;
> >> >> >  } u;
> >> >> > -
> >> >> > -int32_t turbo_enabled;
> >> >> >  };
> >> >>
> >> >> Is all of this re-arrangement really needed? Also can't
> >> >> turbo_enabled and scaling_turbo_pct be combined into a single field?
> >> >
> >> >Personally, we should not combine the two.
> >> > turbo_enabled is used by both the old pstate driver and
> >> >intel_pstate, but  scaling_turbo_pct is only used in intel_pstate.
> >> >If we use
> >> "scaling_turbo_pct=0"
> >> > to represent "turbo_enabled=0", and "scaling_turbo_pct>0" to
> >> >represent " turbo_enabled=1", then we will need to modify the old
> >> >driver to use scaling_turbo_pct, i.e. changing the old driver to be aware 
> >> >of
> the "percentage"
> >> > concept, which is proposed in intel_pstate. On the other side, I
> >> >think keeping  turbo_enabled and scaling_turbo_pct separated makes
> >> >the code
> >> easier to read.
> >>
> >> Note that "combine" doesn't necessarily mean "eliminate the old one"
> >> - they could as well become field of a union. The basic question you
> >> should ask yourself in such cases is: "Do both fields have a meaning
> >> at the same time?" If the answer is yes, then of course they should
> >> remain separate. If the answer is no _and_ their purpose is
> >> reasonably similar, then combining them should at least be considered.
> >
> > Ok. I will keep the two separated, since they do have their own
> > meaning at the same time.
> 
> Being which?

Keeping both of the two there. Just as what they are now - two independent 
fields.

Best,
Wei



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

[Xen-devel] [qemu-mainline test] 63304: tolerable trouble: broken/fail/pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-libvirt  3 host-install(3)  broken in 63275 pass in 63304
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 3 host-install(3) broken in 
63275 pass in 63304
 test-amd64-i386-xl-qemuu-debianhvm-amd64 3 host-install(3) broken in 63275 
pass in 63304
 test-amd64-amd64-xl-xsm   3 host-install(3)   broken pass in 63275
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 3 host-install(3) broken pass in 
63275
 test-amd64-amd64-amd64-pvgrub  3 host-install(3)  broken pass in 63275
 test-armhf-armhf-xl-rtds 11 guest-start fail pass in 63275

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 15 guest-start.2 fail in 63275 like 63117

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

version targeted for testing:
 qemuubc79082e4cd12c1241fa03b0abceacf45f537740
baseline version:
 qemuu426c0df9e3e6e64c7ea489092c57088ca4d227d0

Last test of basis63117  2015-10-21 13:12:41 Z4 days
Failing since 63202  2015-10-22 01:09:34 Z4 days4 attempts
Testing same since63275  2015-10-23 21:54:57 Z2 days2 attempts


People who touched revisions under test:
  Alexey Kardashevskiy 
  Andreas Färber 
  Andrew Jones 
  Aurelien Jarno 
  Benjamin Herrenschmidt 
  Bharata B Rao 
  Christian Borntraeger 
  Cornelia Huck 
  Daniel P. Berrange 
  David Gibson 
  David Hildenbrand 
  Eduardo Habkost 
  Eduardo Otubo 
  Igor Mammedov 
  James Hogan 
  Knut Omang 
  Laszlo Ersek 
  Laurent Vivier 
  Leon Alrae 
  Marc-André Lureau 
  Max Filippov 
  Michael Roth 
  Michael S. Tsirkin 
  Michael Walle 
  Paolo Bonzini 
  Peter Crosthwaite 
  Peter Crosthwaite 
  Peter Maydell 
  Richard Henderson 
  Thibaut Collet 
  Thomas Huth 
  Tony Krowiak 
  zhanghailiang 
  Zhu Guihua 

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

[Xen-devel] [PATCH MINI-OS] xenbus: notify the other end when necessary

[Wei Liu]

The xenbus thread didn't send notification to other end when it expected
more data or consumed responses, which led to stalling the ring from
time to time.

This is the culprit that guest was less responsive when using stubdom
because the device model was stalled.

Fix this by sending notification to the other end at the right places.

Signed-off-by: Wei Liu 
---
Cc: Ian Campbell 
Cc: Ian Jackson 
Cc: Samuel Thibault 

With this path I can migrate a guest with stubdom a few thousand times
without any issue, while before I could easily trigger time out
within a few iterations. This should make OSSTest stubdom test case more
reliable.

Ian J, this is a patch suitable for backporting to 4.6. It's good time
to branch mini-os now.
---
 xenbus/xenbus.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
index 4613ed6..7451161 100644
--- a/xenbus/xenbus.c
+++ b/xenbus/xenbus.c
@@ -205,8 +205,10 @@ static void xenbus_thread_func(void *ign)
 prod = xenstore_buf->rsp_prod;
 DEBUG("Rsp_cons %d, rsp_prod %d.\n", xenstore_buf->rsp_cons,
 xenstore_buf->rsp_prod);
-if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < sizeof(msg))
+if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < sizeof(msg)) 
{
+notify_remote_via_evtchn(start_info.store_evtchn);
 break;
+}
 rmb();
 memcpy_from_ring(xenstore_buf->rsp,
 &msg,
@@ -217,8 +219,10 @@ static void xenbus_thread_func(void *ign)
 xenstore_buf->rsp_prod - xenstore_buf->rsp_cons,
 msg.req_id);
 if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons <
-sizeof(msg) + msg.len)
+sizeof(msg) + msg.len) {
+notify_remote_via_evtchn(start_info.store_evtchn);
 break;
+}
 
 DEBUG("Message is good.\n");
 
@@ -265,6 +269,9 @@ static void xenbus_thread_func(void *ign)
 xenstore_buf->rsp_cons += msg.len + sizeof(msg);
 wake_up(&req_info[msg.req_id].waitq);
 }
+
+wmb();
+notify_remote_via_evtchn(start_info.store_evtchn);
 }
 }
 }
-- 
2.1.4


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

Re: [Xen-devel] [OSSTEST PATCH 26/26] ts-xen-install: networking: Rename `nodhcp' to `ensurebridge'

[Hu, Robert]

> -Original Message-
> From: Hu, Robert
> Sent: Monday, October 26, 2015 3:05 PM
> To: 'Ian Campbell' ; 'Ian Jackson'
> ; 'xen-de...@lists.xenproject.org'
> 
> Subject: RE: [OSSTEST PATCH 26/26] ts-xen-install: networking: Rename
> `nodhcp' to `ensurebridge'
> 
> > -Original Message-
> > From: Hu, Robert
> > Sent: Sunday, October 25, 2015 10:46 AM
> > To: Ian Campbell ; 'Ian Jackson'
> > ; 'xen-de...@lists.xenproject.org'
> > 
> > Subject: RE: [OSSTEST PATCH 26/26] ts-xen-install: networking: Rename
> > `nodhcp' to `ensurebridge'
> >
> > > -Original Message-
> > > From: Ian Campbell [mailto:ian.campb...@citrix.com]
> > > Sent: Friday, October 23, 2015 9:38 PM
> > > To: Hu, Robert ; 'Ian Jackson'
> > > ; 'xen-de...@lists.xenproject.org'
> > > 
> > > Subject: Re: [OSSTEST PATCH 26/26] ts-xen-install: networking: Rename
> > > `nodhcp' to `ensurebridge'
> > >
> > > On Fri, 2015-10-23 at 13:25 +, Hu, Robert wrote:
> > > > > -Original Message-
> > > > > From: Ian Campbell [mailto:ian.campb...@citrix.com]
> > > > > Sent: Friday, October 23, 2015 4:15 PM
> > > > > To: Hu, Robert ; 'Ian Jackson'
> > > > > ; 'xen-de...@lists.xenproject.org'
> > > > > 
> > > > > Subject: Re: [OSSTEST PATCH 26/26] ts-xen-install: networking:
> Rename
> > > > > `nodhcp' to `ensurebridge'
> > > > >
> > > > > On Fri, 2015-10-23 at 06:16 +, Hu, Robert wrote:
> > > > >
> > > > > > [Hu, Robert]
> > > > > > Seems the failure log shall be this, any idea? I've spent days on
> > > > > > debugging this:(
> > > > > > (XEN) traps.c:3290: GPF (): 82d080195082 ->
> > 82d080243d9d
> > > > > > (XEN) PCI add device :00:00.0
> > > > > > (XEN) PCI add device :00:01.0
> > > > > > (XEN) PCI add device :00:01.1
> > > > > > (XEN) PCI add device :00:01.2
> > > > > > (XEN) PCI add device :00:01.3
> > > > > > (XEN) PCI add device :00:02.0
> > > > > > (XEN) PCI add device :00:03.0
> > > > > > (XEN) d0: Forcing read-only access to MFN fed00
> > > > > > (XEN) Hardware Dom0 crashed: rebooting machine in 5 seconds.
> > > > > > Issued domain 3 reboot
> > > > > > qemu: terminating on signal 1 from pid 4322
> > > > >
> > > > > Please can you report this as a regular bug in a fresh thread, that 
> > > > > way
> > > > > the relevant Xen maintainers are likely to see and react to it, rather
> > > > > than just us osstest folks.
> > > > [Hu, Robert]
> > > >
> > > > It shall be in that way after I confirm it is a bug.
> > > > Currently I'm just still not certain it is a nested bug or because of 
> > > > the
> > > > latest
> > > > osstest code change.
> > > > I was just asking for if you can recall some hint on what changes (of
> > > > osstest)
> > > > might causing this.
> > > > I'm to restore to my v12 code, with current Xen and qemu selection to
> try
> > > > again. I think by this way, I can confirm it is an actual nested bug or
> > > > not.
> > > > Then I would report this in a fresh thread.
> > >
> > [Hu, Robert]
> >
> > With v12 code, on same L1 Dom0 kernel, L1 Xen and Qemu selection,
> nested
> > test
> > passes.
> > I've saved l1 guest configuration, l1 network configuration, and l1 Dom0
> > kernel
> > config for further comparison. Anything else possibly related you can think
> > of?
> >
> > > A dom0 crash of this sort is pretty certainly a bug somewhere in Xen,
> > > whether it is exposed by a new osstest case or not. The people who are
> > best
> > > placed to figure out where the bug is are certainly not reading this
> > > osstest thread.
> > >
> > > So please just report it as a bug as it stands, with the relevant
> > > information.
> > [Hu, Robert]
> >
> > Nested Xen is in tech preview phase, not that production level matured.
> > It is so nit-picking that any configuration change not meeting its appetite
> will
> > induce its naughtiness, i.e. crash, I think. :)
> [Hu, Robert]
> 
> Root cause found: Dom0 kernel boot cmd line: console=xvc0 matters, shall
> be
> hvc0 in nested environment.
[Hu, Robert] 

A patch for this: in ts-xen-install, after exact kernel and xen, check if 
'kernkind'
for this host exist, if not, set it with existing runvar.
The hvc0 --> xvc0 happens in debian_boot_setup() --> target_kernkind_check().
If 'kernkind' runvar for host is missing, the existing code will set it to xvc0.
For our pvops kernel, it shall be hvc0.

diff --git a/ts-xen-install b/ts-xen-install
index 3d0f394..eb40c1e 100755
--- a/ts-xen-install
+++ b/ts-xen-install
@@ -79,6 +79,15 @@ sub extract () {
$r{"$ho->{Ident}_${part}buildjob"} // $r{"${part}buildjob"},
\%distpath);
 }
+
+if (!target_var($ho, 'kernkind'))
+{
+   my $pfx=target_var_prefix($ho);
+
+   store_runvar($pfx."kernkind", $r{'kernkind'});
+}
+
+
 if (target_file_exists($ho, "/usr/lib64/efi/xen.efi")) {
target_cmd_root($ho,< 
> > >
> > > Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.o

Re: [Xen-devel] does cpu_down makes brings core to deepest sleep state?

[Jan Beulich]

>>> On 25.10.15 at 09:59,  wrote:
> I modified Xen kernel to turn off the core number 1 from 80th second to
> 100th.
> In 80th second I use cpu_down which is implemented in /xen/common/cpu.c to
> turn off the 1st core. But, as you can see in below, the core does not be
> in C4 idle state shile have almost 20 second CC6 state residency.
>  Was the core in this 20 second period turned off ?
> C0 residency demonstrate that in the period CPU 1 was in C0 all the time!
> How it is possible to be in C0 and being in CC6 simultaneously?!

But the C0 numbers didn't change - how did you conclude it was in
C0? Fact is - once offlined, no software statistics are being kept for
a CPU; CC6 statistics are being maintained by (and hence read from
hardware), and hence that number changing seems quite natural.

Jan

> sudo xenpm get-cpuidle-state:
> Max possible C-state: C7
> 
> cpu id   : 0
> total C-states   : 5
> idle time(ms): 67008
> C0   : transition [   0]
>residency  [  100722 ms]
> C1   : transition [   0]
>residency  [   0 ms]
> C2   : transition [   0]
>residency  [   0 ms]
> C3   : transition [   0]
>residency  [   0 ms]
> C4   : transition [   0]
>residency  [   0 ms]
> pc2  : [   0 ms]
> pc3  : [   0 ms]
> pc6  : [   0 ms]
> pc7  : [   0 ms]
> cc3  : [   0 ms]
> cc6  : [   0 ms]
> cc7  : [   0 ms]
> 
> cpu id   : 1
> total C-states   : 5
> idle time(ms): 100722
> C0   : transition [   0]
>residency  [  100722 ms]
> C1   : transition [   0]
>residency  [   0 ms]
> C2   : transition [   0]
>residency  [   0 ms]
> C3   : transition [   0]
>residency  [   0 ms]
> C4   : transition [   0]
>residency  [   0 ms]
> pc2  : [   0 ms]
> pc3  : [   0 ms]
> pc6  : [   0 ms]
> pc7  : [   0 ms]
> cc3  : [ 630 ms]
> cc6  : [   20113 ms]
> cc7  : [   0 ms]




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

[Xen-devel] ovmf fails to build in stagin-4.6

[Olaf Hering]

For me ovmf fails to build in staging-4.6:

...
[  541s] + ./configure --host=x86_64-suse-linux-gnu 
--build=x86_64-suse-linux-gnu --program-prefix= --disable-dependency-tracking 
--prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin 
--sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include 
--libdir=/usr/lib64 --libexecdir=/usr/lib --localstatedir=/var 
--sharedstatedir=/usr/com --mandir=/usr/share/man --infodir=/usr/share/info 
--disable-dependency-tracking --enable-debug --disable-stubdom --disable-xen 
--docdir=/usr/share/doc/packages/xen --enable-ovmf --enable-docs --enable-tools 
--enable-systemd --with-systemd=/usr/lib/systemd/system 
--with-systemd-modules-load=/usr/lib/modules-load.d --enable-stubdom 
--enable-ioemu-stubdom --disable-c-stubdom --disable-caml-stubdom 
--disable-vtpm-stubdom --enable-vtpmmgr-stubdom --with-initddir=/etc/init.d
...
[  561s] + make -j2 debug_symbols=n -k
...
[  727s] Building ... 
/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiHobLib/PeiHobLib.inf
 [X64]
[  727s] Building ... 
/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
 [X64]
[  727s] make[7]: Entering directory 
'/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/Build/OvmfX64/RELEASE_GCC44/X64/MdePkg/Library/PeiHobLib/PeiHobLib'
[  727s] "gcc" -g -fshort-wchar -fno-strict-aliasing -Wall -Werror 
-Wno-array-bounds -ffunction-sections -fdata-sections -c -include AutoGen.h 
-DSTRING_ARRAY_NAME=PeiHobLibStrings -m64 -fno-stack-protector 
"-DEFIAPI=__attribute__((ms_abi))" -DNO_BUILTIN_VA_FUNCS -mno-red-zone 
-Wno-address -mcmodel=large -DMDEPKG_NDEBUG -mno-mmx -mno-sse -o 
/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/Build/OvmfX64/RELEASE_GCC44/X64/MdePkg/Library/PeiHobLib/PeiHobLib/OUTPUT/./HobLib.obj
 
-I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiHobLib
 
-I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/Build/OvmfX64/RELEASE_GCC44/X64/MdePkg/Library/PeiHobLib/PeiHobLib/DEBUG
 
-I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg
 -I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/
 non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Include 
-I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Include/X64
 
/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiHobLib/HobLib.c
[  727s] make[7]: Entering directory 
'/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/Build/OvmfX64/RELEASE_GCC44/X64/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt'
[  727s] "gcc" -g -fshort-wchar -fno-strict-aliasing -Wall -Werror 
-Wno-array-bounds -ffunction-sections -fdata-sections -c -include AutoGen.h 
-DSTRING_ARRAY_NAME=PeiServicesTablePointerLibIdtStrings -m64 
-fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))" -DNO_BUILTIN_VA_FUNCS 
-mno-red-zone -Wno-address -mcmodel=large -DMDEPKG_NDEBUG -mno-mmx -mno-sse -o 
/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/Build/OvmfX64/RELEASE_GCC44/X64/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt/OUTPUT/./PeiServicesTablePointer.obj
 
-I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiServicesTablePointerLibIdt
 
-I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/Build/OvmfX64/RELEASE_GCC44/X64/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt/DEBUG
 -I/home/abuild/rpmbuild/BUILD/xen
 -4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg 
-I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Include
 
-I/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Include/X64
 
/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointer.c
[  727s] 
/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointer.c:
 In function 'MigratePeiServicesTablePointer':
[  727s] 
/home/abuild/rpmbuild/BUILD/xen-4.6.20151022T160310.e4a1dcb/non-dbg/tools/firmware/ovmf-dir-remote/MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointer.c:100:26:
 e

Re: [Xen-devel] [PATCH v5 8/9] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Jan Beulich]

>>> On 26.10.15 at 08:59,  wrote:
> On 26/10/2015 15:03,  Jan Beulich wrote:
>> >>> "Wang, Wei W"  10/26/15 7:27 AM >>>
>> >On 08/10/2015 12:11,  Jan Beulich wrote:
>> >> >>> On 14.09.15 at 04:32,  wrote:
>> >> > -new_policy.governor = __find_governor(op-
>> >u.set_gov.scaling_governor);
>> >> > -if (new_policy.governor == NULL)
>> >> > -return -EINVAL;
>> >> > +if ( internal_gov && internal_gov->cur_gov )
>> >> > +{
>> >> > +if ( !strnicmp(op->u.set_gov.scaling_governor,
>> >> > +   "performance", CPUFREQ_NAME_LEN) )
>> >> > +internal_gov->cur_gov = INTERNAL_GOV_PERFORMANCE;
>> >> > +else if ( !strnicmp(op->u.set_gov.scaling_governor,
>> >> > +   "powersave", CPUFREQ_NAME_LEN) )
>> >> > +internal_gov->cur_gov = INTERNAL_GOV_POWERSAVE;
>> >> > +else if ( !strnicmp(op->u.set_gov.scaling_governor,
>> >> > +   "userspace", CPUFREQ_NAME_LEN) )
>> >> > +internal_gov->cur_gov = INTERNAL_GOV_USERSPACE;
>> >> > +else if ( !strnicmp(op->u.set_gov.scaling_governor,
>> >> > +   "ondemand", CPUFREQ_NAME_LEN) )
>> >> > +internal_gov->cur_gov = INTERNAL_GOV_ONDEMAND;
>> >>
>> >> Wouldn't this better be done by the internal governor's code, so it
>> >> can also for itself decide which of the kinds it may not want to support?
>> >
>> >I think it should be pmstat.c-'s job here to set "
>> >internal_gov->cur_gov", which  is later passed to the internal
>> >governor's implementation code, who then  decides how to deal with the
>> requested governor in "internal_gov->cur_gov".
>> 
>> Which it then is able to communicate in which way? Without itself adjusting
>> ->cur_gov again?
>  
> In this way: 
> pmstat.c gets the string-represented governor adjusting request from the 
> upper layer, parses it to the number-represented value (INTERNAL_GOV_), and 
> feeds the number-represented one to the driver's internal governor 
> implementation. If the internal governor implementation doesn't support the 
> passed INTERNAL_GOV_XX, i.e. INTERNAL_GOV_XX goes to the "default:" section 
> of the "switch()", which adjusts the internal_gov->cur_gov to the default 
> one, 
> e.g. INTERNAL_GOV_ONDEMAND.

Well, okay, I'm tired of discussing issues like this.

>> >> > @@ -309,23 +326,13 @@ struct xen_get_cpufreq_para {
>> >> >  XEN_GUEST_HANDLE_64(uint32) scaling_available_frequencies;
>> >> >  XEN_GUEST_HANDLE_64(char)   scaling_available_governors;
>> >> >  char scaling_driver[CPUFREQ_NAME_LEN];
>> >> > -
>> >> > -uint32_t cpuinfo_cur_freq;
>> >> > -uint32_t cpuinfo_max_freq;
>> >> > -uint32_t cpuinfo_min_freq;
>> >> > -uint32_t scaling_cur_freq;
>> >> > -
>> >> >  char scaling_governor[CPUFREQ_NAME_LEN];
>> >> > -uint32_t scaling_max_freq;
>> >> > -uint32_t scaling_min_freq;
>> >> >
>> >> >  /* for specific governor */
>> >> >  union {
>> >> >  struct  xen_userspace userspace;
>> >> >  struct  xen_ondemand ondemand;
>> >> >  } u;
>> >> > -
>> >> > -int32_t turbo_enabled;
>> >> >  };
>> >>
>> >> Is all of this re-arrangement really needed? Also can't turbo_enabled
>> >> and scaling_turbo_pct be combined into a single field?
>> >
>> >Personally, we should not combine the two.
>> > turbo_enabled is used by both the old pstate driver and intel_pstate,
>> >but  scaling_turbo_pct is only used in intel_pstate. If we use
>> "scaling_turbo_pct=0"
>> > to represent "turbo_enabled=0", and "scaling_turbo_pct>0" to represent
>> >" turbo_enabled=1", then we will need to modify the old driver to use
>> >scaling_turbo_pct, i.e. changing the old driver to be aware of the 
>> >"percentage"
>> > concept, which is proposed in intel_pstate. On the other side, I think
>> >keeping  turbo_enabled and scaling_turbo_pct separated makes the code
>> easier to read.
>> 
>> Note that "combine" doesn't necessarily mean "eliminate the old one" - they
>> could as well become field of a union. The basic question you should ask
>> yourself in such cases is: "Do both fields have a meaning at the same time?" 
>> If
>> the answer is yes, then of course they should remain separate. If the answer 
>> is
>> no _and_ their purpose is reasonably similar, then combining them should at
>> least be considered.
> 
> Ok. I will keep the two separated, since they do have their own meaning at 
> the same time. 

Being which?

Jan

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

Re: [Xen-devel] [PATCH v2 1/5] xl: convert exit codes to EXIT_[SUCCESS|FAILURE]

[Dario Faggioli]

On Sat, 2015-10-24 at 11:01 +0530, Harmandeep Kaur wrote:
> turning main function xl exit codes towards using the
> EXIT_[SUCCESS|FAILURE] macros, instead of instead of arbitrary
> numbers
> or libxl return codes.
> 
I'd say "turning xl main() function exit codes..."

Also in the subject "xl: convert main() exit codes to..."

> also includes a document comment in xl.h stating xl process should
> always return EXIT_FOO and main_* can be treated as main() as if
> they are returning a process exit status and not a function return
> value)
> 
> Signed-off-by: Harmandeep Kaur 
>
I also have a (minor) comment to the code but, with the subject,
changelog, and comment (see below) fixed, this patch can have:

Reviewed-by: Dario Faggioli 

> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -30,6 +30,12 @@ struct cmd_spec {
>  char *cmd_option;
>  };
>  
> +/*
> +*xl process should always return EXIT_FOO and main_* can be treated
> +*as main() as if they are returning a process exit status and not a
> +*function return value.
> +*/
> +
I think it is important to be a bit more precise, here. After all,
we're explaining people how they should do things, so let's do it
properly. :-)

"The xl process should always return either EXIT_SUCCESS or
EXIT_FAILURE. main_* functions, implementing the various xl commands,
can be treated..."

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



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

Re: [Xen-devel] [xen-4.5-testing test] 63303: regressions - trouble: blocked/broken/fail/pass

[Jan Beulich]

>>> On 26.10.15 at 08:07,  wrote:
> flight 63303 xen-4.5-testing real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/63303/ 
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  build-armhf   5 xen-build fail REGR. vs. 
> 63099

This being

libxl_arm.c: In function 'libxl__arch_domain_init_hw_description':
libxl_arm.c:591:76: error: 'state' undeclared (first use in this function)
libxl_arm.c:591:76: note: each undeclared identifier is reported only once for 
each function it appears in
make[3]: *** [libxl_arm.o] Error 1

is 9befcd335c perhaps missing some prereq backport?

Since 4.5.2 is supposed to go out asap, can you please fix this or
revert?

Thanks, Jan


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

Re: [Xen-devel] [PATCH] Fixing coverity bug CID1311511

[Lasya Venneti]

Thank you for pointing out the changelog errors to me, I will definitely
keep those in mind and be careful next time.

Thanks
Lasya V

On 26 October 2015 at 14:48, Lasya Venneti  wrote:

> Hello,
>
> I just wanted to submit this as one of the bugs, the one which George
> assigned to me is still pending(it's a patch series), I will submit that
> ASAP. As for this bug, if I have incorrectly handled it, can you please
> point out my mistake, I will correct it and re-submit.
>
> Thanks
> Lasya V
>
>
>
> On 26 October 2015 at 14:39, Dario Faggioli 
> wrote:
>
>> On Sun, 2015-10-25 at 15:32 +0530, Lasya Venneti wrote:
>> > *This is part of my 'bite sized contribution' to Xen for the
>> > OutreachY program.
>> >
>> > *The change handles the return value of the function xc_dom_allocate,
>> > if the function returns NULL the function returns -1. It would not be
>> > useful to jump to err as err would check !dom for NULL.
>> >
>> But then you're not closing xs_fd, is that ok? (I'm asking, because I
>> am not at all a xenstore expert, but, FWIW, it does not feel right to
>> me).
>>
>> > *Changes have been made in the build function in init-xenstore
>> > -domain.c
>> >
>> > *I have taken these discussions for reference:
>> > https://www.choon.net/forum/read.php?22,3805351,3805351
>> >
>> > Signed-off: Lasya Venneti 
>> >
>> Most of this (except the first bullet point, perhaps), and especially
>> the Signed-off-by (it's 'Signed-off-by:', not 'Signed-off') tag goes in
>> the patch changelog.
>>
>> In fact:
>>  - this looks like a cover letter for a patch series, but there is
>>only one patch in this case. Usually, when there is only one patch,
>>you don't need a cover letter (there are exceptions, but I don't
>>think this qualifies);
>>  - cover letters, no matter whether for series or single patches, do
>>not become part of the source tree, when the patch (series) is
>>committed. That is why, information about the patch
>>content/design/etc. and the tags must live in the changelog. If you
>>do like this, someone looking at `git log' wouldn't see it.
>>
>> Regards,
>> Dario
>> --
>> <> (Raistlin Majere)
>> -
>> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
>> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
>>
>>
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] Fixing coverity bug CID1311511

[Lasya Venneti]

Hello,

I just wanted to submit this as one of the bugs, the one which George
assigned to me is still pending(it's a patch series), I will submit that
ASAP. As for this bug, if I have incorrectly handled it, can you please
point out my mistake, I will correct it and re-submit.

Thanks
Lasya V



On 26 October 2015 at 14:39, Dario Faggioli 
wrote:

> On Sun, 2015-10-25 at 15:32 +0530, Lasya Venneti wrote:
> > *This is part of my 'bite sized contribution' to Xen for the
> > OutreachY program.
> >
> > *The change handles the return value of the function xc_dom_allocate,
> > if the function returns NULL the function returns -1. It would not be
> > useful to jump to err as err would check !dom for NULL.
> >
> But then you're not closing xs_fd, is that ok? (I'm asking, because I
> am not at all a xenstore expert, but, FWIW, it does not feel right to
> me).
>
> > *Changes have been made in the build function in init-xenstore
> > -domain.c
> >
> > *I have taken these discussions for reference:
> > https://www.choon.net/forum/read.php?22,3805351,3805351
> >
> > Signed-off: Lasya Venneti 
> >
> Most of this (except the first bullet point, perhaps), and especially
> the Signed-off-by (it's 'Signed-off-by:', not 'Signed-off') tag goes in
> the patch changelog.
>
> In fact:
>  - this looks like a cover letter for a patch series, but there is
>only one patch in this case. Usually, when there is only one patch,
>you don't need a cover letter (there are exceptions, but I don't
>think this qualifies);
>  - cover letters, no matter whether for series or single patches, do
>not become part of the source tree, when the patch (series) is
>committed. That is why, information about the patch
>content/design/etc. and the tags must live in the changelog. If you
>do like this, someone looking at `git log' wouldn't see it.
>
> Regards,
> Dario
> --
> <> (Raistlin Majere)
> -
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
>
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] Fixing coverity bug CID1311511

[Dario Faggioli]

On Sun, 2015-10-25 at 15:32 +0530, Lasya Venneti wrote:
> *This is part of my 'bite sized contribution' to Xen for the
> OutreachY program. 
> 
> *The change handles the return value of the function xc_dom_allocate,
> if the function returns NULL the function returns -1. It would not be
> useful to jump to err as err would check !dom for NULL. 
> 
But then you're not closing xs_fd, is that ok? (I'm asking, because I
am not at all a xenstore expert, but, FWIW, it does not feel right to
me).

> *Changes have been made in the build function in init-xenstore
> -domain.c
> 
> *I have taken these discussions for reference:
> https://www.choon.net/forum/read.php?22,3805351,3805351
> 
> Signed-off: Lasya Venneti 
>
Most of this (except the first bullet point, perhaps), and especially
the Signed-off-by (it's 'Signed-off-by:', not 'Signed-off') tag goes in
the patch changelog.

In fact:
 - this looks like a cover letter for a patch series, but there is 
   only one patch in this case. Usually, when there is only one patch, 
   you don't need a cover letter (there are exceptions, but I don't 
   think this qualifies);
 - cover letters, no matter whether for series or single patches, do 
   not become part of the source tree, when the patch (series) is 
   committed. That is why, information about the patch 
   content/design/etc. and the tags must live in the changelog. If you 
   do like this, someone looking at `git log' wouldn't see it.

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



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

Re: [Xen-devel] xSplice prototype

[Ross Lagerwall]

On 10/23/2015 05:23 PM, Konrad Rzeszutek Wilk wrote:

On Fri, Oct 23, 2015 at 04:28:25PM +0100, Ross Lagerwall wrote:

Limitations
===
The above is enough to fully implement an update system where multiple
source patches are combined (using combinediff) and built into a single
binary which then atomically replaces any existing loaded patches (this is
why I added a REPLACE operation). This is the approach used by kPatch and
kGraft. Multiple completely independent patches can also be loaded but
unexpected interactions may occur.

As it stands, the patches are statically linked which means that independent
patches cannot be linked against one another (e.g. if one introduces a new
symbol). Using the combinediff approach above fixes this.

Backtraces containing functions from a patch module do not show the symbol
name.

There is no checking that a patch which is loaded is built for the correct
hypervisor.


Hehe.. bugs? What bugs!?


Of course there are no bugs :-)



Binary patching works at the function level.


Design thoughts
===
Combining patches at the source level is relatively easy. Multiple binary
patches applied at runtime is tricky. I'm not convinced that it is
necessarily a good idea. Based on the discussion so far, the sanest way of
doing this that I can think of is:
* Each hypervisor has an embedded build id.
* Each binary patch has an embedded build id.
* The hypervisor should expose its build id and the build id of every loaded
binary patch.
* Each binary patch specifies one or more build ids on which it depends.
These build ids can be either a hypervisor build id or another patch build
id. The dependencies could either identified automatically or by a
developer.
* The userspace tool enforces dependency management (user can optionally
force patch apply). I don't see any reason to involve the
hypervisor for dependency management.
Implementing this scheme will require dynamically linking the binary
patches.

The CHECK phase seems unnecessary to me. I would think that any safety
checking that needs to be done would be done atomically at the point of
patch apply (or revert). Given the implemented system of applying patches,
I'm not sure if any safety checking need be done at all.


It was added as a way to do signature checking and any other type
of checking that needed to be done. And which may take quite a while
to get done - hence doing it asynchronously.


OK. There are many things that need to be done to load an xSplice 
module, almost all of which are dependent on the size of the module and 
may also fail (e.g. resolving symbols, performing relocations, copying 
allocated sections, etc). I think signature checking should be as part 
of the load procedure, and if that needs to be done asynchronously, then 
so be it. The nice thing about doing signature checking at load time is 
that (if it's implemented as per Linux's signature checking) once the 
load phase is complete, the original uploaded payload can be freed from 
memory. It might be handy to think of the load procedure as equivalent 
to a basic version of the Linux kernel module loader (which is pretty 
much what I did when implementing it).


And while I remember, I think the REVERTED state is unnecessary. It 
seems exactly equivalent to the LOADED state, which is just confusing.





We have a meeting this Monday (Oct 26th) @ 10AM on the #xsplice channel.

And in which we can disucss the 'combining patches' part, the build-id, the
logging mechanism, and other things you had encountered and have thoughts on.



Sure, we can discuss these items.

--
Ross Lagerwall

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

[Xen-devel] Understanding Xen event channel bit operation

[amin.fall...@gmail.com]

Hi everybody
I need to understand when these pending and mask bits are set and cleared.
It seems pending bits are set by evtchn_set_pending method in
event_channel.c but I don't understand where pending bit is cleared by the
guest and where mask bit is set and reset?
Can anybody help me with understanding this?
Thanks all
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] xen-blkback: clear PF_NOFREEZE for xen_blkif_schedule()

[Jiri Kosina]

From: Jiri Kosina 

xen_blkif_schedule() kthread calls try_to_freeze() at the beginning of 
every attempt to purge the LRU. This operation can't ever succeed though, 
as the kthread hasn't marked itself as freezable.

Before (hopefully eventually) kthread freezing gets converted to fileystem 
freezing, we'd rather mark xen_blkif_schedule() freezable (as it can 
generate I/O during suspend).

Signed-off-by: Jiri Kosina 
---
 drivers/block/xen-blkback/blkback.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/xen-blkback/blkback.c 
b/drivers/block/xen-blkback/blkback.c
index af3caa3..bb65f7c 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -597,6 +597,7 @@ int xen_blkif_schedule(void *arg)
 
xen_blkif_get(blkif);
 
+   set_freezable();
while (!kthread_should_stop()) {
if (unlikely(vbd->size != vbd_sz(vbd)))
xen_vbd_resize(blkif);

-- 
Jiri Kosina
SUSE Labs

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

Re: [Xen-devel] [PATCH v5 8/9] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Wang, Wei W]

On 26/10/2015 15:03,  Jan Beulich wrote:
> >>> "Wang, Wei W"  10/26/15 7:27 AM >>>
> >On 08/10/2015 12:11,  Jan Beulich wrote:
> >> >>> On 14.09.15 at 04:32,  wrote:
> >> > -new_policy.governor = __find_governor(op-
> >u.set_gov.scaling_governor);
> >> > -if (new_policy.governor == NULL)
> >> > -return -EINVAL;
> >> > +if ( internal_gov && internal_gov->cur_gov )
> >> > +{
> >> > +if ( !strnicmp(op->u.set_gov.scaling_governor,
> >> > +   "performance", CPUFREQ_NAME_LEN) )
> >> > +internal_gov->cur_gov = INTERNAL_GOV_PERFORMANCE;
> >> > +else if ( !strnicmp(op->u.set_gov.scaling_governor,
> >> > +   "powersave", CPUFREQ_NAME_LEN) )
> >> > +internal_gov->cur_gov = INTERNAL_GOV_POWERSAVE;
> >> > +else if ( !strnicmp(op->u.set_gov.scaling_governor,
> >> > +   "userspace", CPUFREQ_NAME_LEN) )
> >> > +internal_gov->cur_gov = INTERNAL_GOV_USERSPACE;
> >> > +else if ( !strnicmp(op->u.set_gov.scaling_governor,
> >> > +   "ondemand", CPUFREQ_NAME_LEN) )
> >> > +internal_gov->cur_gov = INTERNAL_GOV_ONDEMAND;
> >>
> >> Wouldn't this better be done by the internal governor's code, so it
> >> can also for itself decide which of the kinds it may not want to support?
> >
> >I think it should be pmstat.c-'s job here to set "
> >internal_gov->cur_gov", which  is later passed to the internal
> >governor's implementation code, who then  decides how to deal with the
> requested governor in "internal_gov->cur_gov".
> 
> Which it then is able to communicate in which way? Without itself adjusting
> ->cur_gov again?
 
In this way: 
pmstat.c gets the string-represented governor adjusting request from the upper 
layer, parses it to the number-represented value (INTERNAL_GOV_), and feeds the 
number-represented one to the driver's internal governor implementation. If the 
internal governor implementation doesn't support the passed INTERNAL_GOV_XX, 
i.e. INTERNAL_GOV_XX goes to the "default:" section of the "switch()", which 
adjusts the internal_gov->cur_gov to the default one, e.g. 
INTERNAL_GOV_ONDEMAND.

> >> >  struct xen_get_cpufreq_para {
> >> >  /* IN/OUT variable */
> >> >  uint32_t cpu_num;
> >> >  uint32_t freq_num;
> >> >  uint32_t gov_num;
> >> > +int32_t turbo_enabled;
> >> > +
> >> > +uint32_t cpuinfo_cur_freq;
> >> > +uint32_t cpuinfo_max_freq;
> >> > +uint32_t cpuinfo_min_freq;
> >> > +uint32_t scaling_cur_freq;
> >> > +
> >> > +uint32_t scaling_turbo_pct;
> >> > +uint32_t scaling_max_perf;
> >> > +uint32_t scaling_min_perf;
> >> > +enum perf_alias perf_alias;
> >> >
> >> >  /* for all governors */
> >> >  /* OUT variable */
> >> > @@ -309,23 +326,13 @@ struct xen_get_cpufreq_para {
> >> >  XEN_GUEST_HANDLE_64(uint32) scaling_available_frequencies;
> >> >  XEN_GUEST_HANDLE_64(char)   scaling_available_governors;
> >> >  char scaling_driver[CPUFREQ_NAME_LEN];
> >> > -
> >> > -uint32_t cpuinfo_cur_freq;
> >> > -uint32_t cpuinfo_max_freq;
> >> > -uint32_t cpuinfo_min_freq;
> >> > -uint32_t scaling_cur_freq;
> >> > -
> >> >  char scaling_governor[CPUFREQ_NAME_LEN];
> >> > -uint32_t scaling_max_freq;
> >> > -uint32_t scaling_min_freq;
> >> >
> >> >  /* for specific governor */
> >> >  union {
> >> >  struct  xen_userspace userspace;
> >> >  struct  xen_ondemand ondemand;
> >> >  } u;
> >> > -
> >> > -int32_t turbo_enabled;
> >> >  };
> >>
> >> Is all of this re-arrangement really needed? Also can't turbo_enabled
> >> and scaling_turbo_pct be combined into a single field?
> >
> >Personally, we should not combine the two.
> > turbo_enabled is used by both the old pstate driver and intel_pstate,
> >but  scaling_turbo_pct is only used in intel_pstate. If we use
> "scaling_turbo_pct=0"
> > to represent "turbo_enabled=0", and "scaling_turbo_pct>0" to represent
> >" turbo_enabled=1", then we will need to modify the old driver to use
> >scaling_turbo_pct, i.e. changing the old driver to be aware of the 
> >"percentage"
> > concept, which is proposed in intel_pstate. On the other side, I think
> >keeping  turbo_enabled and scaling_turbo_pct separated makes the code
> easier to read.
> 
> Note that "combine" doesn't necessarily mean "eliminate the old one" - they
> could as well become field of a union. The basic question you should ask
> yourself in such cases is: "Do both fields have a meaning at the same time?" 
> If
> the answer is yes, then of course they should remain separate. If the answer 
> is
> no _and_ their purpose is reasonably similar, then combining them should at
> least be considered.

Ok. I will keep the two separated, since they do have their own meaning at the 
same time. 

Best,
Wei

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

Re: [Xen-devel] [OSSTEST PATCH 26/26] ts-xen-install: networking: Rename `nodhcp' to `ensurebridge'

[Hu, Robert]

> -Original Message-
> From: Hu, Robert
> Sent: Sunday, October 25, 2015 10:46 AM
> To: Ian Campbell ; 'Ian Jackson'
> ; 'xen-de...@lists.xenproject.org'
> 
> Subject: RE: [OSSTEST PATCH 26/26] ts-xen-install: networking: Rename
> `nodhcp' to `ensurebridge'
> 
> > -Original Message-
> > From: Ian Campbell [mailto:ian.campb...@citrix.com]
> > Sent: Friday, October 23, 2015 9:38 PM
> > To: Hu, Robert ; 'Ian Jackson'
> > ; 'xen-de...@lists.xenproject.org'
> > 
> > Subject: Re: [OSSTEST PATCH 26/26] ts-xen-install: networking: Rename
> > `nodhcp' to `ensurebridge'
> >
> > On Fri, 2015-10-23 at 13:25 +, Hu, Robert wrote:
> > > > -Original Message-
> > > > From: Ian Campbell [mailto:ian.campb...@citrix.com]
> > > > Sent: Friday, October 23, 2015 4:15 PM
> > > > To: Hu, Robert ; 'Ian Jackson'
> > > > ; 'xen-de...@lists.xenproject.org'
> > > > 
> > > > Subject: Re: [OSSTEST PATCH 26/26] ts-xen-install: networking: Rename
> > > > `nodhcp' to `ensurebridge'
> > > >
> > > > On Fri, 2015-10-23 at 06:16 +, Hu, Robert wrote:
> > > >
> > > > > [Hu, Robert]
> > > > > Seems the failure log shall be this, any idea? I've spent days on
> > > > > debugging this:(
> > > > > (XEN) traps.c:3290: GPF (): 82d080195082 ->
> 82d080243d9d
> > > > > (XEN) PCI add device :00:00.0
> > > > > (XEN) PCI add device :00:01.0
> > > > > (XEN) PCI add device :00:01.1
> > > > > (XEN) PCI add device :00:01.2
> > > > > (XEN) PCI add device :00:01.3
> > > > > (XEN) PCI add device :00:02.0
> > > > > (XEN) PCI add device :00:03.0
> > > > > (XEN) d0: Forcing read-only access to MFN fed00
> > > > > (XEN) Hardware Dom0 crashed: rebooting machine in 5 seconds.
> > > > > Issued domain 3 reboot
> > > > > qemu: terminating on signal 1 from pid 4322
> > > >
> > > > Please can you report this as a regular bug in a fresh thread, that way
> > > > the relevant Xen maintainers are likely to see and react to it, rather
> > > > than just us osstest folks.
> > > [Hu, Robert]
> > >
> > > It shall be in that way after I confirm it is a bug.
> > > Currently I'm just still not certain it is a nested bug or because of the
> > > latest
> > > osstest code change.
> > > I was just asking for if you can recall some hint on what changes (of
> > > osstest)
> > > might causing this.
> > > I'm to restore to my v12 code, with current Xen and qemu selection to try
> > > again. I think by this way, I can confirm it is an actual nested bug or
> > > not.
> > > Then I would report this in a fresh thread.
> >
> [Hu, Robert]
> 
> With v12 code, on same L1 Dom0 kernel, L1 Xen and Qemu selection, nested
> test
> passes.
> I've saved l1 guest configuration, l1 network configuration, and l1 Dom0
> kernel
> config for further comparison. Anything else possibly related you can think
> of?
> 
> > A dom0 crash of this sort is pretty certainly a bug somewhere in Xen,
> > whether it is exposed by a new osstest case or not. The people who are
> best
> > placed to figure out where the bug is are certainly not reading this
> > osstest thread.
> >
> > So please just report it as a bug as it stands, with the relevant
> > information.
> [Hu, Robert]
> 
> Nested Xen is in tech preview phase, not that production level matured.
> It is so nit-picking that any configuration change not meeting its appetite 
> will
> induce its naughtiness, i.e. crash, I think. :)
[Hu, Robert] 

Root cause found: Dom0 kernel boot cmd line: console=xvc0 matters, shall be
hvc0 in nested environment.

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

[Xen-devel] [xen-4.5-testing test] 63303: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf   5 xen-build fail REGR. vs. 63099
 build-armhf   4 host-build-prep  fail in 63214 REGR. vs. 63099

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl3 host-install(3)  broken in 63268 pass in 63303
 test-amd64-i386-qemut-rhel6hvm-amd 3 host-install(3) broken in 63268 pass in 
63303
 test-amd64-amd64-libvirt-vhd  3 host-install(3)  broken in 63268 pass in 63303
 test-amd64-i386-libvirt-pair  4 host-install/dst_host(4)  broken pass in 63214
 test-amd64-i386-xl-qemuu-win7-amd64  3 host-install(3)broken pass in 63214
 test-amd64-i386-rumpuserxen-i386  3 host-install(3)   broken pass in 63268
 test-amd64-i386-libvirt  14 guest-saverestore  fail in 63214 pass in 63303
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail in 63214 
pass in 63303
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 16 guest-localmigrate/x10 fail in 
63268 pass in 63303
 test-amd64-i386-xl-qemuu-winxpsp3 16 guest-localmigrate/x10 fail pass in 63268

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds  6 xen-boot  fail REGR. vs. 63099
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail in 63214 blocked in 
63099
 test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail in 63214 
like 63099
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail like 63099
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail like 63099

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 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

version targeted for testing:
 xen  a5d0480d9c5b36b5f47459ce92ed3c67b7bed51d
baseline version:
 xen  80e9f5624f9d1ef2e7bbd9b9b185e96b45e1bb17

Last test of basis63099  2015-10-20 17:40:41 Z5 days
Failing since 63155  2015-10-21 15:43:44 Z4 days4 attempts
Testing same since63214  2015-10-22 13:24:43 Z3 days3 attempts


People who touched revisions under test:
  Ian Campbell 
  Jan Beulich 
  Julien Grall 
  Wei Liu 
  Yang Zhang 

jobs:
 build-amd64  pass
 build-armhf  fail
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  blocked 
 build-i386-libvirt   pass
 build-amd64-prev pass
 build-i386-prev  pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 build-amd64-rumpuserxen  pass
 build-i386-rumpuserxen   pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  blocked 
 test-amd64-i386-xl   pass
 test-amd64-amd64-xl-pvh-amd  fail
 test-amd64-i386-qemut-rhel6hvm-amd   pass
 test-amd64-i386-qemuu-rhel6hvm-amd   p

Re: [Xen-devel] [PATCH v5 8/9] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Jan Beulich]

>>> "Wang, Wei W"  10/26/15 7:27 AM >>>
>On 08/10/2015 12:11,  Jan Beulich wrote:
>> >>> On 14.09.15 at 04:32,  wrote:
>> > --- a/tools/libxc/xc_pm.c
>> > +++ b/tools/libxc/xc_pm.c
>> > @@ -260,13 +260,14 @@ int xc_get_cpufreq_para(xc_interface *xch, int
>> cpuid,
>> >  }
>> >  else
>> >  {
>> > -user_para->cpuinfo_cur_freq = sys_para->cpuinfo_cur_freq;
>> > -user_para->cpuinfo_max_freq = sys_para->cpuinfo_max_freq;
>> > -user_para->cpuinfo_min_freq = sys_para->cpuinfo_min_freq;
>> > -user_para->scaling_cur_freq = sys_para->scaling_cur_freq;
>> > -user_para->scaling_max_freq = sys_para->scaling_max_freq;
>> > -user_para->scaling_min_freq = sys_para->scaling_min_freq;
>> > -user_para->turbo_enabled= sys_para->turbo_enabled;
>> > +user_para->cpuinfo_cur_freq  = sys_para->cpuinfo_cur_freq;
>> > +user_para->cpuinfo_max_freq  = sys_para->cpuinfo_max_freq;
>> > +user_para->cpuinfo_min_freq  = sys_para->cpuinfo_min_freq;
>> > +user_para->scaling_cur_freq  = sys_para->scaling_cur_freq;
>> > +user_para->scaling_max.pct   = sys_para->scaling_max_perf;
>> > +user_para->scaling_min.pct   = sys_para->scaling_min_perf;
>> > +user_para->scaling_turbo_pct = sys_para->scaling_turbo_pct;
>> > +user_para->turbo_enabled = sys_para->turbo_enabled;
>> 
>> You fail to communicate perf_alias here. How will the caller know?
>
>"user_para->perf_alias = sys_para->perf_alias" is added in the next patch 
>"tools:...", because the the "perf_alias " field is added to 
>xc_get_cpufreq_para there.

But the public interface structure field is already there at this point. I.e. 
at this
point in the series you're hiding information - another sign for improper 
breakup
of the series.

>> >  op->u.get_para.cpuinfo_cur_freq =
>> >  cpufreq_driver->get ? cpufreq_driver->get(op->cpuid) : 
>> > policy->cur;
>> >  op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
>> >  op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
>> >  op->u.get_para.scaling_cur_freq = policy->cur;
>> > -op->u.get_para.scaling_max_freq = policy->max;
>> > -op->u.get_para.scaling_min_freq = policy->min;
>> > +if ( internal_gov )
>> > +{
>> > +op->u.get_para.scaling_max_perf = limits->max_perf_pct;
>> > +op->u.get_para.scaling_min_perf = limits->min_perf_pct;
>> > +op->u.get_para.scaling_turbo_pct = limits->turbo_pct;
>> > +}
>> > +else
>> > +{
>> > +op->u.get_para.scaling_max_perf = policy->max;
>> > +op->u.get_para.scaling_min_perf = policy->min;
>> > +}
>> >
>> >  if ( cpufreq_driver->name[0] )
>> > +{
>> >  strlcpy(op->u.get_para.scaling_driver,
>> >  cpufreq_driver->name, CPUFREQ_NAME_LEN);
>> > +if ( !strncmp(cpufreq_driver->name,
>> > + "intel_pstate", CPUFREQ_NAME_LEN) )
>> > +op->u.get_para.perf_alias = PERCENTAGE;
>> > +else
>> > +op->u.get_para.perf_alias = FREQUENCY;
>> 
>> This should be done together with the other things in the if/else right 
>> above.
>
>The code here is in a different file from the if/else above.

Huh? I don't think I trimmed the original mail this badly. From the context
above, both are directly adjacent in the same source file.

>> > -new_policy.governor = __find_governor(op->u.set_gov.scaling_governor);
>> > -if (new_policy.governor == NULL)
>> > -return -EINVAL;
>> > +if ( internal_gov && internal_gov->cur_gov )
>> > +{
>> > +if ( !strnicmp(op->u.set_gov.scaling_governor,
>> > +   "performance", CPUFREQ_NAME_LEN) )
>> > +internal_gov->cur_gov = INTERNAL_GOV_PERFORMANCE;
>> > +else if ( !strnicmp(op->u.set_gov.scaling_governor,
>> > +   "powersave", CPUFREQ_NAME_LEN) )
>> > +internal_gov->cur_gov = INTERNAL_GOV_POWERSAVE;
>> > +else if ( !strnicmp(op->u.set_gov.scaling_governor,
>> > +   "userspace", CPUFREQ_NAME_LEN) )
>> > +internal_gov->cur_gov = INTERNAL_GOV_USERSPACE;
>> > +else if ( !strnicmp(op->u.set_gov.scaling_governor,
>> > +   "ondemand", CPUFREQ_NAME_LEN) )
>> > +internal_gov->cur_gov = INTERNAL_GOV_ONDEMAND;
>> 
>> Wouldn't this better be done by the internal governor's code, so it can also 
>> for
>> itself decide which of the kinds it may not want to support?
>
>I think it should be pmstat.c-'s job here to set " internal_gov->cur_gov", 
>which
> is later passed to the internal governor's implementation code, who then
> decides how to deal with the requested governor in "internal_gov->cur_gov".

Which it then is able to communicate in which way? Without itself adjusting
->cur_gov again?

>> > --- a/xen/include/public/sysctl.h
>> > +++ b/xen/include/public/sysctl.h
>> > @@ -297,11 +297,28 @@ typedef struct xen_