[Xen-devel] [PATCH] xen/domctl: lower loglevel of XEN_DOMCTL_memory_mapping

[Tiejun Chen]

We should lower loglevel to XENLOG_G_DEBUG while mapping or
unmapping memory via XEN_DOMCTL_memory_mapping since its
fair enough to check this info just while debugging.

CC: Ian Campbell 
CC: Ian Jackson 
CC: Jan Beulich 
CC: Keir Fraser 
CC: Tim Deegan 
Signed-off-by: Tiejun Chen 
---
 xen/common/domctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 7f959f3..3bf39f1 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -1049,7 +1049,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
 
 if ( add )
 {
-printk(XENLOG_G_INFO
+printk(XENLOG_G_DEBUG
"memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
d->domain_id, gfn, mfn, nr_mfns);
 
@@ -1061,7 +1061,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
 }
 else
 {
-printk(XENLOG_G_INFO
+printk(XENLOG_G_DEBUG
"memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
d->domain_id, gfn, mfn, nr_mfns);
 
-- 
1.9.1


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

Re: [Xen-devel] [Xen-users] Xen bridging issue.

[johnny Strom]

On 09/08/2015 02:45 PM, Wei Liu wrote:

On Tue, Sep 08, 2015 at 02:33:53PM +0300, johnny Strom wrote:

On 09/08/2015 02:12 PM, Wei Liu wrote:

On Tue, Sep 08, 2015 at 02:07:21PM +0300, johnny Strom wrote:

On 09/08/2015 01:06 PM, Wei Liu wrote:

On Tue, Sep 08, 2015 at 12:59:39PM +0300, johnny Strom wrote:

On 09/08/2015 12:13 PM, Wei Liu wrote:

  xenstore-ls/local/domain/$DOMID/

Here is the output of xenstore-ls only one network card is working.

xenstore-ls  /local/domain/1


[...]

  vif = ""
   0 = ""
backend = "/local/domain/0/backend/vif/1/0"
backend-id = "0"
state = "4"
handle = "0"
mac = "00:16:3e:ee:aa:aa"
multi-queue-num-queues = "17"

OK so the number of queues is 17. You probably don't need that many
queues.

Set module parameter "xenvif_max_queues" of netback to something like 4
should work around the problem for you.

Hello

I tried to set it to 4 in  /etc/modprobe.d/xen_netback.conf

rmmod xen_netback

modprobe -v xen_netback
insmod
/lib/modules/3.16.0-4-amd64/kernel/drivers/net/xen-netback/xen-netback.ko
xenvif_max_queues=4



But it is still the same issue..

Is xenvif_max_queues supported in Linux kernel 3.16?

modinfo -p xen_netback

separate_tx_rx_irq: (bool)
rx_drain_timeout_msecs: (uint)
rx_stall_timeout_msecs: (uint)
max_queues:Maximum number of queues per virtual interface (uint)

Oh, right, the parameter name should be "max_queues".

Sorry about that!

Wei.


It's still the same issue:

modprobe -v xen_netback
insmod
/lib/modules/3.16.0-4-amd64/kernel/drivers/net/xen-netback/xen-netback.ko
max_queues=4


If there are the precise steps you took, isn't modprobe -v already
inserted the module without parameter set? I.e. the later insmod had no
effect.


Hello again

I am not sure but there might be another issue with the xen_netback 
module in Debian jessie.


I am not able to set the  max_queues options so that it is set at load time.

I have tested with the loop kernel module where it works to set an value 
but doing the same for

the xen_netback driver dose not work:

This works with loop module like this:
options loop max_loop=50

And then doing the same with the xen_netback that dose not work:
options xen_netback max_queues=4

Or is there some other way it should be set in 
/etc/modprobe.d/xen_netback.conf?



Best regards Johnny




But what could be reason for this?


Make sure that parameter is correctly set. You can look at
/sys/module/xen_netback/parameter/max_queues for the actual number.

You can even just echo a number to that file to set the value on the
fly.


Could it be problems with one of the CPUS? since if I boot dom0 with just 14
cpu cores then it works


No, it can't be related to CPUs.  That's because DomU doesn't exhaust
resources anymore.

Wei.

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



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

[Xen-devel] [PATCH] x86/hvm: fix saved pmtimer value

[Kouya Shimura]

The ACPI PM timer is sometimes broken on live migration.
Since vcpu->arch.hvm_vcpu.guest_time is zero in most cases.

Without this patch, the clock of windows server 2012R2 without HPET
might leap forward several minutes on live migration.

Signed-off-by: Kouya Shimura 
---
 xen/arch/x86/hvm/pmtimer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/pmtimer.c b/xen/arch/x86/hvm/pmtimer.c
index 07594e1..282efd0 100644
--- a/xen/arch/x86/hvm/pmtimer.c
+++ b/xen/arch/x86/hvm/pmtimer.c
@@ -252,7 +252,7 @@ static int pmtimer_save(struct domain *d, 
hvm_domain_context_t *h)
 /* Update the counter to the guest's current time.  We always save
  * with the domain paused, so the saved time should be after the
  * last_gtime, but just in case, make sure we only go forwards */
-x = ((s->vcpu->arch.hvm_vcpu.guest_time - s->last_gtime) * s->scale) >> 32;
+x = ((hvm_get_guest_time(s->vcpu) - s->last_gtime) * s->scale) >> 32;
 if ( x < 1UL<<31 )
 s->pm.tmr_val += x;
 if ( (s->pm.tmr_val & TMR_VAL_MSB) != msb )
-- 
1.9.1


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

Re: [Xen-devel] [PATCH v4 10/11] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Wang, Wei W]

On 24/07/2015 22:16,  Jan Beulich wrote:
>>> On 25.06.15 at 13:17,  wrote:
> --- a/xen/drivers/acpi/pmstat.c
> +++ b/xen/drivers/acpi/pmstat.c
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -315,8 +315,18 @@ struct xen_get_cpufreq_para {
>  uint32_t scaling_cur_freq;
>  
>  char scaling_governor[CPUFREQ_NAME_LEN];
> -uint32_t scaling_max_freq;
> -uint32_t scaling_min_freq;
> +
> +union {
> +uint32_t freq;
> +uint32_t pct;
> +} scaling_max;
> +
> +union {
> +uint32_t freq;
> +uint32_t pct;
> +} scaling_min;

>scaling_min and scaling_max should really be of the same type, so that someone 
>wanting to introduce helper functions 
>or pointers to them can hand both interchangeably.

>Also I'm starting to get tired of repeating that it is still unclear how a 
>consumer of the structure will know which of the 
>two fields of the unions are applicable.

Hi Jan,

Probably we don't need a union here. I plan to simply change them to 
uint32_t scaling_max_perf;
uint32_t scaling_max_perf;

Then it's up to the driver to put what kind of value to it. It's like we simply 
provide a drinking vessel, and it depends on the user to put water or milk into 
it. In our case, the intel_pstate driver assigns a percentage vale to it (in 
the "uint32_t" type), and the legacy driver assigns the absolute value to it 
(in the "uint32_t" type, too).

I will finish this round of revision soon.

Best,
Wei





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

Re: [Xen-devel] [PATCH v2 3/4] tools: add tools support for Intel CDP

[Jan Beulich]

>>> On 09.09.15 at 10:10,  wrote:
> On Wed, Sep 09, 2015 at 03:32:11PM +0800, Chao Peng wrote:
>> On Wed, Sep 09, 2015 at 01:16:47PM +0800, He Chen wrote:
>> > @@ -304,14 +305,41 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t 
>> > domid,
>> >  goto out;
>> >  }
>> >  
>> > +rc = libxl_psr_cat_get_l3_info(ctx, , _sockets);
>> > +if (rc) {
>> > +LOGE(ERROR, "Failed to get cat info");
>> > +goto out;
>> > +}
>> > +
>> > +if (!info->cdp_enabled &&
>> > +   (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
>> > +type == LIBXL_PSR_CBM_TYPE_L3_DATA))
>> > +{
>> > +LOGE(ERROR, "Unable to set Code/Data CBM with CDP disabled");
>> > +rc = EINVAL;
>> > +free(info);
>> > +goto out;
>> > +}
>> > +
>> >  libxl_for_each_set_bit(socketid, *target_map) {
>> >  if (socketid >= nr_sockets)
>> >  break;
>> > -if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, 
>> > cbm)) {
>> > +if (info->cdp_enabled && type == LIBXL_PSR_CBM_TYPE_L3_CBM)
>> > +{
>> > +if(xc_psr_cat_set_domain_data(ctx->xch, domid,
>> > +   LIBXL_PSR_CBM_TYPE_L3_CODE, socketid, cbm) ||
>> > +   xc_psr_cat_set_domain_data(ctx->xch, domid,
>> > +   LIBXL_PSR_CBM_TYPE_L3_DATA, socketid, cbm))
>> > +{
>> > +libxl__psr_cat_log_err_msg(gc, errno);
>> > +rc = ERROR_FAIL;
>> > +}
>> 
>> Will you merge the two if's?
> 
> Surely.

Surely not you mean, or else how will this ...

>> > +} else if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, 
>> > socketid, cbm)) {

... work?

Looking at this I'm surprised though that consumers is required to do
two calls (and know whether CDP is enabled) when they want to set
things up without caring for the code/data distinction. I think this
should be taken care of in the hypervisor.

Jan


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

Re: [Xen-devel] [PATCH v6 16/18] vmx: Add some scheduler hooks for VT-d posted interrupts

[Wu, Feng]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Monday, September 07, 2015 8:55 PM
> To: Wu, Feng
> Cc: Andrew Cooper; Dario Faggioli; George Dunlap; Tian, Kevin;
> xen-devel@lists.xen.org; Keir Fraser
> Subject: Re: [PATCH v6 16/18] vmx: Add some scheduler hooks for VT-d posted
> interrupts
> 
> >>> On 25.08.15 at 03:57,  wrote:
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -1573,6 +1573,22 @@ static void __context_switch(void)
> >  per_cpu(curr_vcpu, cpu) = n;
> >  }
> >
> > +static inline void pi_ctxt_switch_from(struct vcpu *prev)
> > +{
> > +/*
> > + * When switching from non-idle to idle, we only do a lazy context
> switch.
> > + * However, in order for posted interrupt (if available and enabled) to
> > + * work properly, we at least need to update the descriptors.
> > + */
> > +if ( prev->arch.pi_ctxt_switch_from && !is_idle_vcpu(prev) )
> > +prev->arch.pi_ctxt_switch_from(prev);
> > +}
> > +
> > +static inline void pi_ctxt_switch_to(struct vcpu *next)
> > +{
> > +if ( next->arch.pi_ctxt_switch_to && !is_idle_vcpu(next) )
> > +next->arch.pi_ctxt_switch_to(next);
> > +}
> >
> >  void context_switch(struct vcpu *prev, struct vcpu *next)
> >  {
> > @@ -1605,9 +1621,12 @@ void context_switch(struct vcpu *prev, struct
> vcpu *next)
> >
> >  set_current(next);
> >
> > +pi_ctxt_switch_from(prev);
> > +
> >  if ( (per_cpu(curr_vcpu, cpu) == next) ||
> >   (is_idle_domain(nextd) && cpu_online(cpu)) )
> >  {
> > +pi_ctxt_switch_to(next);
> >  local_irq_enable();
> 
> This placement, if really intended that way, needs explanation (in a
> comment) and perhaps even renaming of the involved symbols, as

I think maybe removing the static wrapper function can make thing
clearer. What is your opinion?

> looking at it from a general perspective it seems wrong (with
> pi_ctxt_switch_to() excluding idle vCPU-s it effectively means you
> want this only when switching back to what got switched out lazily
> before, i.e. this would be not something to take place on an arbitrary
> context switch). As to possible alternative names - maybe make the
> hooks ctxt_switch_prepare() and ctxt_switch_cancel()?
> 
> > @@ -117,10 +119,20 @@ static int vmx_vcpu_initialise(struct vcpu *v)
> >  INIT_LIST_HEAD(>arch.hvm_vmx.pi_blocked_vcpu_list);
> >  INIT_LIST_HEAD(>arch.hvm_vmx.pi_vcpu_on_set_list);
> >
> > +v->arch.hvm_vmx.pi_block_cpu = -1;
> > +
> > +spin_lock_init(>arch.hvm_vmx.pi_lock);
> > +
> >  v->arch.schedule_tail= vmx_do_resume;
> >  v->arch.ctxt_switch_from = vmx_ctxt_switch_from;
> >  v->arch.ctxt_switch_to   = vmx_ctxt_switch_to;
> >
> > +if ( iommu_intpost && is_hvm_vcpu(v) )
> > +{
> > +v->arch.pi_ctxt_switch_from = vmx_pre_ctx_switch_pi;
> > +v->arch.pi_ctxt_switch_to = vmx_post_ctx_switch_pi;
> > +}
> 
> Why conditional upon is_hvm_vcpu()?

I only think we need to add these hooks for PV guests, right?
Or you mean I should use has_hvm_container_vcpu() instead?

> 
> > @@ -718,6 +730,140 @@ static void vmx_fpu_leave(struct vcpu *v)
> >  }
> >  }
> >
> > +void arch_vcpu_wake_prepare(struct vcpu *v)
> 
> A non-static function with this name should not live in vmx.c.
> 
> > +{
> > +unsigned long gflags;
> 
> Just "flags" please.
> 
> > +if ( !iommu_intpost || !is_hvm_vcpu(v)
> || !has_arch_pdevs(v->domain) )
> 
> DYM !has_hvm_container_vcpu()?
> 
> > +return;
> > +
> > +spin_lock_irqsave(>arch.hvm_vmx.pi_lock, gflags);
> > +
> > +if ( likely(vcpu_runnable(v)) ||
> > + !test_bit(_VPF_blocked, >pause_flags) )
> 
> _VPF_blocked set implies !vcpu_runnable(), i.e. afaict just the
> latter check would suffice if this is really what you mean.
> 
> > +{
> > +struct pi_desc *pi_desc = >arch.hvm_vmx.pi_desc;
> > +unsigned long flags;
> 
> You don't need this - you already know IRQs are off for the lock
> acquire below.
> 
> > +/*
> > + * We don't need to send notification event to a non-running
> > + * vcpu, the interrupt information will be delivered to it before
> > + * VM-ENTRY when the vcpu is scheduled to run next time.
> > + */
> > +pi_set_sn(pi_desc);
> > +
> > +/*
> > + * Set 'NV' field back to posted_intr_vector, so the
> > + * Posted-Interrupts can be delivered to the vCPU by
> > + * VT-d HW after it is scheduled to run.
> > + */
> > +write_atomic((uint8_t*)_desc->nv, posted_intr_vector);
> 
> Bogus cast.
> 
> > +
> > +/*
> > + * Delete the vCPU from the related block list
> > + * if we are resuming from blocked state.
> > + */
> > +if ( v->arch.hvm_vmx.pi_block_cpu != -1 )
> > +{
> > +spin_lock_irqsave(_cpu(pi_blocked_vcpu_lock,
> > +  

Re: [Xen-devel] [PATCH v2 1/4] x86: Support enable CDP by boot parameter and add get CDP status

[Chao Peng]

On Wed, Sep 09, 2015 at 01:16:45PM +0800, He Chen wrote:
> Intel Code/Data Prioritization(CDP) feature is based on CAT. cdp_enabled
> is added to CAT socket info to indicate CDP is on or off on the socket,
> note that cos_max would be half when CDP is on. struct psr_cat_cbm is
> extended to support CDP operation. Extend psr_get_cat_l3_info sysctl to
> get CDP status.
> 
> Signed-off-by: He Chen 
> ---
>  xen/arch/x86/psr.c  | 84 
> ++---
>  xen/arch/x86/sysctl.c   |  5 ++-
>  xen/include/asm-x86/msr-index.h |  3 ++
>  xen/include/asm-x86/psr.h   | 11 +-
>  xen/include/public/sysctl.h |  2 +
>  5 files changed, 89 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index c0daa2e..ba0f726 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -17,13 +17,21 @@
>  #include 
>  #include 
>  #include 
> +#include 

Is this still needed?

>  #include 
>  
>  #define PSR_CMT(1<<0)
>  #define PSR_CAT(1<<1)
> +#define PSR_CDP(1<<2)
>  
>  struct psr_cat_cbm {
> -uint64_t cbm;
> +union {
> +uint64_t cbm;
> +struct {
> +uint64_t code;
> +uint64_t data;
> +} cdp;
> +} u;
>  unsigned int ref;
>  };
>  
> @@ -32,6 +40,7 @@ struct psr_cat_socket_info {
>  unsigned int cos_max;
>  struct psr_cat_cbm *cos_to_cbm;
>  spinlock_t cbm_lock;

As you defined the cdp stauts for each socket here ...
> +bool_t cdp_enabled;
>  };
>  
>  struct psr_assoc {
> @@ -43,6 +52,7 @@ struct psr_cmt *__read_mostly psr_cmt;
>  
>  static unsigned long *__read_mostly cat_socket_enable;
>  static struct psr_cat_socket_info *__read_mostly cat_socket_info;
> +static unsigned long *__read_mostly cdp_socket_enable;

... this one then perhaps is redundency. If only one is need then I really
like to keep the latter one.

> @@ -470,6 +500,7 @@ static void cat_cpu_init(void)
>  struct psr_cat_socket_info *info;
>  unsigned int socket;
>  unsigned int cpu = smp_processor_id();
> +uint64_t val;
>  const struct cpuinfo_x86 *c = cpu_data + cpu;
>  
>  if ( !cpu_has(c, X86_FEATURE_CAT) || c->cpuid_level < 
> PSR_CPUID_LEVEL_CAT )
> @@ -490,13 +521,34 @@ static void cat_cpu_init(void)
>  info->cos_to_cbm = temp_cos_to_cbm;
>  temp_cos_to_cbm = NULL;
>  /* cos=0 is reserved as default cbm(all ones). */
> -info->cos_to_cbm[0].cbm = (1ull << info->cbm_len) - 1;
> +info->cos_to_cbm[0].u.cbm = (1ull << info->cbm_len) - 1;
>  
>  spin_lock_init(>cbm_lock);
>  
>  set_bit(socket, cat_socket_enable);
>  printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, 
> cbm_len:%u\n",
> socket, info->cos_max, info->cbm_len);

If CDP is enable below, then this information is quite misleading.

> +
> +if ( (ecx & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
> +{
> +if ( test_bit(socket, cdp_socket_enable) )
> +return;
> +
> +rdmsrl(MSR_IA32_PSR_L3_QOS_CFG, val);
> +wrmsrl(MSR_IA32_PSR_L3_QOS_CFG, val | 1 << 
> PSR_L3_QOS_CDP_ENABLE_BIT);
> +
> +/* No need to write register since CBMs are fully open as 
> default by HW */
> +info->cos_to_cbm[0].u.cdp.code = (1ull << info->cbm_len) - 1;
> +info->cos_to_cbm[0].u.cdp.data = (1ull << info->cbm_len) - 1;

If I remember correctly, HW is supposed to boot as CAT mode by default and
only IA32_L3_QOS_Mask_0 is all ones, other mask msrs however are not mentioned
in the spec which then may contain arbitrary value. So I guesss as
least you need write all ones to IA32_L3_QOS_Mask_1 explicitly and that
should be done before you enabled CDP.

> +
> +/* Cut half of cos_max when CDP enabled */
> +info->cos_max = info->cos_max / 2;
> +
> +info->cdp_enabled = 1;
> +set_bit(socket, cdp_socket_enable);
> +printk(XENLOG_INFO "CDP: enabled on socket %u, cos_max:%u, 
> cbm_len:%u\n",
> +   socket, info->cos_max, info->cbm_len);
> +}
>  }
>  }
>  
> diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
> index e9c4723..402e7a7 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -328,7 +328,10 @@
>  #define MSR_IA32_CMT_EVTSEL  0x0c8d
>  #define MSR_IA32_CMT_CTR 0x0c8e
>  #define MSR_IA32_PSR_ASSOC   0x0c8f
> +#define MSR_IA32_PSR_L3_QOS_CFG  0x0c81
>  #define MSR_IA32_PSR_L3_MASK(n)  (0x0c90 + (n))
> +#define MSR_IA32_PSR_L3_MASK_CBM_CODE(n) (0x0c90 + (n * 2 + 1))
> +#define MSR_IA32_PSR_L3_MASK_CBM_DATA(n) (0x0c90 + (n * 2))

I guess 'CBM' can be removed from these two macros, e.g.
MSR_IA32_PSR_L3_MASK_CODE & MSR_IA32_PSR_L3_MASK_DATA

Chao

Re: [Xen-devel] [PATCH v4 10/11] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Wang, Wei W]

On 09/09/2015 16:32,  Jan Beulich wrote:
>>> On 09.09.15 at 10:11,  wrote:
> On 24/07/2015 22:16,  Jan Beulich wrote:
 On 25.06.15 at 13:17,  wrote:
>> --- a/xen/drivers/acpi/pmstat.c
>> +++ b/xen/drivers/acpi/pmstat.c
>> --- a/xen/include/public/sysctl.h
>> +++ b/xen/include/public/sysctl.h
>> @@ -315,8 +315,18 @@ struct xen_get_cpufreq_para {
>>  uint32_t scaling_cur_freq;
>>  
>>  char scaling_governor[CPUFREQ_NAME_LEN];
>> -uint32_t scaling_max_freq;
>> -uint32_t scaling_min_freq;
>> +
>> +union {
>> +uint32_t freq;
>> +uint32_t pct;
>> +} scaling_max;
>> +
>> +union {
>> +uint32_t freq;
>> +uint32_t pct;
>> +} scaling_min;
> 
>>scaling_min and scaling_max should really be of the same type, so that
> someone wanting to introduce helper functions
>>or pointers to them can hand both interchangeably.
> 
>>Also I'm starting to get tired of repeating that it is still unclear 
>>how a
> consumer of the structure will know which of the
>>two fields of the unions are applicable.
> 
>> Probably we don't need a union here. I plan to simply change them to 
>> uint32_t scaling_max_perf; uint32_t scaling_max_perf;
> 
>> Then it's up to the driver to put what kind of value to it. It's like 
>> we simply provide a drinking vessel, and it depends on the user to put 
>> water or milk into it. In our case, the intel_pstate driver assigns a 
>> percentage vale to it (in the "uint32_t" type), and the legacy driver 
>> assigns the absolute value to it (in the "uint32_t" type, too).

>I don't see how this will solve the problem of the consumer not knowing what 
>kind of value it has to deal with.

The consumer is inside the print_cpufreq_para() function. I have put the code 
below:

+if (!strncmp(p_cpufreq->scaling_driver,
+  "intel_pstate", CPUFREQ_NAME_LEN) )
+{
+printf("max_perf_pct : %d\n", p_cpufreq->scaling_max.pct);
+printf("min_perf_pct : %d\n", p_cpufreq->scaling_min.pct);
+printf("turbo_pct: %d\n", p_cpufreq->scaling_turbo_pct);
+}
+else
+{
+printf("scaling_avail_freq   :");
+for ( i = 0; i < p_cpufreq->freq_num; i++ )
+if ( p_cpufreq->scaling_available_frequencies[i] ==
+ p_cpufreq->scaling_cur_freq )
+printf(" *%d", p_cpufreq->scaling_available_frequencies[i]);
+else
+printf(" %d", p_cpufreq->scaling_available_frequencies[i]);
+printf("\n");
+printf("scaling frequency: max [%u] min [%u] cur [%u]\n",
+   p_cpufreq->scaling_max.freq,
+   p_cpufreq->scaling_min.freq,
+   p_cpufreq->scaling_cur_freq);

"p_cpufreq->scaling_driver" is the flag which distinguishes the usage of this 
"scaling_max_perf" field.


Best,
Wei


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

Re: [Xen-devel] [PATCH v2 3/4] tools: add tools support for Intel CDP

[Chao Peng]

On Wed, Sep 09, 2015 at 02:37:36AM -0600, Jan Beulich wrote:
> >>> On 09.09.15 at 10:10,  wrote:
> > On Wed, Sep 09, 2015 at 03:32:11PM +0800, Chao Peng wrote:
> >> On Wed, Sep 09, 2015 at 01:16:47PM +0800, He Chen wrote:
> >> > @@ -304,14 +305,41 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t 
> >> > domid,
> >> >  goto out;
> >> >  }
> >> >  
> >> > +rc = libxl_psr_cat_get_l3_info(ctx, , _sockets);
> >> > +if (rc) {
> >> > +LOGE(ERROR, "Failed to get cat info");
> >> > +goto out;
> >> > +}
> >> > +
> >> > +if (!info->cdp_enabled &&
> >> > +   (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
> >> > +type == LIBXL_PSR_CBM_TYPE_L3_DATA))
> >> > +{
> >> > +LOGE(ERROR, "Unable to set Code/Data CBM with CDP 
> >> > disabled");
> >> > +rc = EINVAL;
> >> > +free(info);
> >> > +goto out;
> >> > +}
> >> > +
> >> >  libxl_for_each_set_bit(socketid, *target_map) {
> >> >  if (socketid >= nr_sockets)
> >> >  break;
> >> > -if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, 
> >> > cbm)) {
> >> > +if (info->cdp_enabled && type == LIBXL_PSR_CBM_TYPE_L3_CBM)
> >> > +{
> >> > +if(xc_psr_cat_set_domain_data(ctx->xch, domid,
> >> > +   LIBXL_PSR_CBM_TYPE_L3_CODE, socketid, cbm) ||
> >> > +   xc_psr_cat_set_domain_data(ctx->xch, domid,
> >> > +   LIBXL_PSR_CBM_TYPE_L3_DATA, socketid, cbm))
> >> > +{
> >> > +libxl__psr_cat_log_err_msg(gc, errno);
> >> > +rc = ERROR_FAIL;
> >> > +}
> >> 
> >> Will you merge the two if's?
> > 
> > Surely.
> 
> Surely not you mean, or else how will this ...
> 
> >> > +} else if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, 
> >> > socketid, cbm)) {
> 
> ... work?

Indeed. Then just ignore it.

> 
> Looking at this I'm surprised though that consumers is required to do
> two calls (and know whether CDP is enabled) when they want to set
> things up without caring for the code/data distinction. I think this
> should be taken care of in the hypervisor.

Agreed, hypervisor should take care of it for this case.

Chao


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

Re: [Xen-devel] [PATCH for 4.6] x86/VPMU: Set VPMU context pointer to NULL when freeing it

[Jan Beulich]

>>> On 09.09.15 at 04:55,  wrote:
> Otherwise we may hit assertion in vpmu_initialise() if vcpu is offlined
> and then onlined again.
> 
> For tidyness, set priv_context to NULL as well.
> 
> Signed-off-by: Boris Ostrovsky 

Acked-by: Jan Beulich 


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

Re: [Xen-devel] [PATCH v2 2/4] x86: add domctl cmd to set/get CDP code/data CBM

[Chao Peng]

On Wed, Sep 09, 2015 at 01:16:46PM +0800, He Chen wrote:
> CDP extends CAT and provides the capacity to control L3 code & data
> cache. With CDP, one COS correspond to two CMBs(code & data). cbm_type
> is added to support distinguish different CBM operation. Besides, new
> domctl cmds are introdunced to support set/get CDP CBM. Some CAT
> functions to operation CBMs are extended to support CDP.
> 
> Signed-off-by: He Chen 
> ---
>  xen/arch/x86/domctl.c   |  32 -
>  xen/arch/x86/psr.c  | 163 
> ++--
>  xen/include/asm-x86/psr.h   |  12 +++-
>  xen/include/public/domctl.h |   4 ++
>  4 files changed, 170 insertions(+), 41 deletions(-)
> 
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index bf62a88..734fddb 100644
> --- a/xen/arch/x86/domctl.c
>  
> -int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
> +int psr_get_l3_cbm(struct domain *d, unsigned int socket,
> +   uint64_t *cbm, enum cbm_type type)
>  {
>  struct psr_cat_socket_info *info = get_cat_socket_info(socket);
>  
>  if ( IS_ERR(info) )
>  return PTR_ERR(info);
>  
> -*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
> +if ( type == PSR_CBM_TYPE_L3 )
> +{
> +if ( info->cdp_enabled )
> +return -EXDEV;
> +*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
> +}
> +else if ( type == PSR_CBM_TYPE_L3_CODE )
> +{
> +if ( !info->cdp_enabled )
> +return -EXDEV;
> +*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.code;
> +}
> +else
> +{
> +if ( !info->cdp_enabled )
> +return -EXDEV;
> +*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.data;
> +}

Could I suggest to use switch? And the check can be done in advance.

>  
>  return 0;
>  }
> @@ -371,65 +389,136 @@ static int write_l3_cbm(unsigned int socket, unsigned 
> int cos,
>  return 0;
>  }
>  
> -int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
> +static int find_cos(struct psr_cat_cbm *map, int cos_max,
> +uint64_t cbm_code, uint64_t cbm_data, bool_t cdp_enabled)
>  {
> -unsigned int old_cos, cos;
> -struct psr_cat_cbm *map, *found = NULL;
> +unsigned int cos;
> +
> +if ( !cdp_enabled )
> +{
> +for ( cos = 0; cos <= cos_max; cos++ )
> +if ( map[cos].ref && map[cos].u.cbm == cbm_code )
> +return cos;
> +}
> +else
> +{
> +for ( cos = 0; cos <= cos_max; cos++ )
> +if ( map[cos].ref && map[cos].u.cdp.code == cbm_code &&
> + map[cos].u.cdp.data == cbm_data )
> +return cos;
> +}

I guess the '{' and '}' can be omitted if the body contains only one
sentence.

> +
> +return -ENOENT;
> +}
> +
> +static int pick_avail_cos(struct psr_cat_cbm *map, int cos_max, int old_cos)
> +{
> +int cos;
> +
> +/* If old cos is referred only by the domain, then use it. */
> +if ( map[old_cos].ref == 1 )
> +return old_cos;
> +
> +/* Then we pick an unused one, never pick 0 */

Find an unused one other than cos0.

> +for ( cos = 1; cos <= cos_max; cos++ )
> +if ( map[cos].ref == 0 )
> +return cos;
> +
> +return -ENOENT;
> +}
> +
> +int psr_set_l3_cbm(struct domain *d, unsigned int socket,
> +   uint64_t cbm, enum cbm_type type)
> +{
> +unsigned int old_cos, cos_max;
> +int cos, ret;
> +uint64_t cbm_data, cbm_code;
> +bool_t need_write = 1;
> +struct psr_cat_cbm *map;
>  struct psr_cat_socket_info *info = get_cat_socket_info(socket);
>  
>  if ( IS_ERR(info) )
>  return PTR_ERR(info);
>  
> +cbm_code = (1ull << info->cbm_len) - 1;
> +cbm_data = (1ull << info->cbm_len) - 1;

Why this is needed, as you will give them the value eventually.

>  if ( !psr_check_cbm(info->cbm_len, cbm) )
>  return -EINVAL;
>  
> +cos_max = info->cos_max;
>  old_cos = d->arch.psr_cos_ids[socket];
>  map = info->cos_to_cbm;
>  
> -spin_lock(>cbm_lock);
> -
> -for ( cos = 0; cos <= info->cos_max; cos++ )
> +switch( type )

Coding style.

Chao
>  {
> -/* If still not found, then keep unused one. */
> -if ( !found && cos != 0 && map[cos].ref == 0 )
> -found = map + cos;
> -else if ( map[cos].u.cbm == cbm )
> -{
> -if ( unlikely(cos == old_cos) )
> -{
> -ASSERT(cos == 0 || map[cos].ref != 0);
> -spin_unlock(>cbm_lock);
> -return 0;
> -}
> -found = map + cos;
> +case PSR_CBM_TYPE_L3:
> +if ( info->cdp_enabled )
> +return -EINVAL;
> +cbm_code = cbm;
> +cbm_data = cbm;
>  break;
> -}
> -}
>  
> -/* If 

Re: [Xen-devel] [PATCH v2 0/4] Intel Code/Data Prioritization(CDP) feature enabling

[Chao Peng]

On Wed, Sep 09, 2015 at 01:16:44PM +0800, He Chen wrote:
> Changes in v2:
> - x86: Enable CDP by boot parameter instead of enabling/disabling CDP at
> runtime (suggested by Andrew)

As you added a new boot option, you also need a patch for
docs/misc/xen-command-line.markdown.

Chao
> - tools: remove psr-cat-cdp-enable/disable xl commands
> - code style
> 
> Code/Data Prioritization(CDP) is offered in Intel Broadwell and later server
> platforms, which is an extension of CAT. CDP enables isolation and separate
> prioritization of code and data fetches to the L3 cache in a software
> configurable manner, which can enable workload prioritization and tuning of
> cache capacity to the characteristics of the workload. CDP extends Cache
> Allocation Technology (CAT) by providing separate code and data capacity bit
> masks(CBM) per Class of Service (COS). CDP is used on VM basis in the Xen
> implementation.
> 
> More information about CDP, please refer to Intel SDM, Volumn 3, section 17.16
> http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf
> 
> This patch series enables CDP feature in Xen based on CAT code, and extends
> CBM operation functions to support CDP. For all the changes, please see in
> each patch.
> 
> This patchset has been tested on Intel Broadwell server platform.
> 
> To make this patchset better, any comment or suggestion is welcomed, I would
> really appreciate it.
> 
> Thanks
> 
> He Chen (4):
>   x86: Support enable CDP by boot parameter and add get CDP status
>   x86: add domctl cmd to set/get CDP code/data CBM
>   tools: add tools support for Intel CDP
>   docs: add document to introduce CDP command
> 
>  docs/man/xl.pod.1   |  14 +++
>  docs/misc/xl-psr.markdown   |  44 +++-
>  tools/libxc/include/xenctrl.h   |   7 +-
>  tools/libxc/xc_psr.c|  17 ++-
>  tools/libxl/libxl.h |  10 ++
>  tools/libxl/libxl_psr.c |  32 +-
>  tools/libxl/libxl_types.idl |   3 +
>  tools/libxl/xl.h|   2 +
>  tools/libxl/xl_cmdimpl.c|  52 +++--
>  tools/libxl/xl_cmdtable.c   |   5 +
>  xen/arch/x86/domctl.c   |  32 +-
>  xen/arch/x86/psr.c  | 237 
> 
>  xen/arch/x86/sysctl.c   |   5 +-
>  xen/include/asm-x86/msr-index.h |   3 +
>  xen/include/asm-x86/psr.h   |  23 +++-
>  xen/include/public/domctl.h |   4 +
>  xen/include/public/sysctl.h |   2 +
>  17 files changed, 420 insertions(+), 72 deletions(-)
> 
> -- 
> 1.9.1
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

Re: [Xen-devel] [PATCH for 4.6] x86/VPMU: Set VPMU context pointer to NULL when freeing it

[Wei Liu]

On Tue, Sep 08, 2015 at 10:55:52PM -0400, Boris Ostrovsky wrote:
> Otherwise we may hit assertion in vpmu_initialise() if vcpu is offlined
> and then onlined again.
> 
> For tidyness, set priv_context to NULL as well.
> 
> Signed-off-by: Boris Ostrovsky 

Release-acked-by: Wei Liu 

> ---
>  xen/arch/x86/cpu/vpmu_amd.c   | 2 ++
>  xen/arch/x86/cpu/vpmu_intel.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
> index 825be72..04da81a 100644
> --- a/xen/arch/x86/cpu/vpmu_amd.c
> +++ b/xen/arch/x86/cpu/vpmu_amd.c
> @@ -438,6 +438,8 @@ static void amd_vpmu_destroy(struct vcpu *v)
>  amd_vpmu_unset_msr_bitmap(v);
>  
>  xfree(vpmu->context);
> +vpmu->context = NULL;
> +vpmu->priv_context = NULL;
>  
>  if ( vpmu_is_set(vpmu, VPMU_RUNNING) )
>  release_pmu_ownship(PMU_OWNER_HVM);
> diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
> index b3750d7..12f80ae 100644
> --- a/xen/arch/x86/cpu/vpmu_intel.c
> +++ b/xen/arch/x86/cpu/vpmu_intel.c
> @@ -828,7 +828,9 @@ static void core2_vpmu_destroy(struct vcpu *v)
>  struct vpmu_struct *vpmu = vcpu_vpmu(v);
>  
>  xfree(vpmu->context);
> +vpmu->context = NULL;
>  xfree(vpmu->priv_context);
> +vpmu->priv_context = NULL;
>  if ( has_hvm_container_vcpu(v) && cpu_has_vmx_msr_bitmap )
>  core2_vpmu_unset_msr_bitmap(v->arch.hvm_vmx.msr_bitmap);
>  release_pmu_ownship(PMU_OWNER_HVM);
> -- 
> 1.8.1.4

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

Re: [Xen-devel] [PATCH for 4.6] x86/VPMU: Set VPMU context pointer to NULL when freeing it

[Dietmar Hahn]

Am Dienstag 08 September 2015, 22:55:52 schrieb Boris Ostrovsky:
> Otherwise we may hit assertion in vpmu_initialise() if vcpu is offlined
> and then onlined again.
> 
> For tidyness, set priv_context to NULL as well.

Reviewed-by: Dietmar Hahn 

> 
> Signed-off-by: Boris Ostrovsky 
> ---
>  xen/arch/x86/cpu/vpmu_amd.c   | 2 ++
>  xen/arch/x86/cpu/vpmu_intel.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
> index 825be72..04da81a 100644
> --- a/xen/arch/x86/cpu/vpmu_amd.c
> +++ b/xen/arch/x86/cpu/vpmu_amd.c
> @@ -438,6 +438,8 @@ static void amd_vpmu_destroy(struct vcpu *v)
>  amd_vpmu_unset_msr_bitmap(v);
>  
>  xfree(vpmu->context);
> +vpmu->context = NULL;
> +vpmu->priv_context = NULL;
>  
>  if ( vpmu_is_set(vpmu, VPMU_RUNNING) )
>  release_pmu_ownship(PMU_OWNER_HVM);
> diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
> index b3750d7..12f80ae 100644
> --- a/xen/arch/x86/cpu/vpmu_intel.c
> +++ b/xen/arch/x86/cpu/vpmu_intel.c
> @@ -828,7 +828,9 @@ static void core2_vpmu_destroy(struct vcpu *v)
>  struct vpmu_struct *vpmu = vcpu_vpmu(v);
>  
>  xfree(vpmu->context);
> +vpmu->context = NULL;
>  xfree(vpmu->priv_context);
> +vpmu->priv_context = NULL;
>  if ( has_hvm_container_vcpu(v) && cpu_has_vmx_msr_bitmap )
>  core2_vpmu_unset_msr_bitmap(v->arch.hvm_vmx.msr_bitmap);
>  release_pmu_ownship(PMU_OWNER_HVM);
> 

-- 
Company details: http://ts.fujitsu.com/imprint.html

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

Re: [Xen-devel] [PATCH] x86/hvm: fix saved pmtimer value

[Jan Beulich]

>>> On 09.09.15 at 09:31,  wrote:
> The ACPI PM timer is sometimes broken on live migration.
> Since vcpu->arch.hvm_vcpu.guest_time is zero in most cases.

I.e. in other than "delay for missed ticks mode". Would have been
nice if you had spelled this out explicitly. With that the question
then is - should the field perhaps be used when non-zero, and the
function only be called otherwise?

Wei - regardless of the answer to the above (i.e. regardless of
whether a v2 is going to be needed) I think we want this in 4.6.

Jan


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

Re: [Xen-devel] Question to Xen log level in the case of PT

[Chen, Tiejun]

So can Xen change log level dynamically like Linux? If yes, we might
change this level temporarily while passing through IGD. If not, any
suggestion?


First of all you could boot without lowering the log level (non-debug
builds) or raising the log level ("loglvl=warning"; debug builds). But


Sorry I don't know how to build "non-debug" here. Could you give me this 
detail? Or where I can get this info?



that would change the log level for the entire session, which may
not be what you're after. I too realized that having a way to
dynamically adjust the log level would be useful occasionally. For
post-4.6 I have a patch (attached) ready allowing to do so in a
limited way from the serial console (and hence also via "xl debug-key").
As you'll see in there I also took note of it probably being desirable
to have a sysctl (and then a wrapping xl command) to full control the
log level. I didn't get around to implement that yet.


Good to know this.



Otoh the specific messages you cite are of quite questionable use
in the first place. I certainly would welcome a patch lowering their
priority to XENLOG_G_DEBUG (which however would still not


Done.

Thanks
Tiejun



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

Re: [Xen-devel] [v2][PATCH] xen/vtd/iommu: permit group devices to passthrough in relaxed mode

[Jan Beulich]

>>> On 09.09.15 at 03:59,  wrote:
> @@ -2310,12 +2312,16 @@ static int intel_iommu_assign_device(
>   PCI_DEVFN2(bdf) == devfn &&
>   rmrr->scope.devices_cnt > 1 )
>  {
> -printk(XENLOG_G_ERR VTDPREFIX
> -   " cannot assign %04x:%02x:%02x.%u"
> +bool_t relaxed = !!(flag & XEN_DOMCTL_DEV_RDM_RELAXED);
> +
> +printk(XENLOG_G_WARNING VTDPREFIX

Well, I can live with this always being a warning, but it's not what I
had asked for. The VT-d maintainers will have to judge.

Jan


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

Re: [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Jan Beulich]

>>> On 09.09.15 at 00:23,  wrote:
> On 09/08/15 19:26, Anthony PERARD wrote:
>> And I get this on the console:
>> Welcome to GRUB!
>> 
>>  X64 Exception Type - 0E(#PF - Page-Fault)  CPU Apic ID -  
>> RIP  - 0F5F8918, CS  - 0028, RFLAGS - 00210206
>> ExceptionData - 0011
>> RAX  - , RCX - 07FCE000, RDX - 
>> RBX  - 0B6092C0, RSP - 0F5F8590, RBP - 0B608EA0
>> RSI  - 0F5F8838, RDI - 0B608EA0
>> R8   - , R9  - 0B609200, R10 - 
>> R11  - 000A, R12 - , R13 - 001B
>> R14  - 0B609360, R15 - 
>> DS   - 0008, ES  - 0008, FS  - 0008
>> GS   - 0008, SS  - 0008
>> CR0  - 8033, CR2 - 0F5F8918, CR3 - 0F597000
>> CR4  - 0668, CR8 - 
>> DR0  - , DR1 - , DR2 - 
>> DR3  - , DR6 - 0FF0, DR7 - 0400
>> GDTR - 0F57BF18 003F, LDTR - 
>> IDTR - 0EEA5018 0FFF,   TR - 
>> FXSAVE_STATE - 0F5F81F0
>>  Find PE image 
> /build/xen-unstable/src/xen-unstable/tools/firmware/ovmf-dir-remote/Build
> /OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/Runtime
> Dxe/StatusCodeRuntimeDxe/DEBUG/StatusCodeRuntimeDxe.dll 
> (ImageBase=0F556000, EntryPoint=0F55628F) 
>> 
>> I did check with other guest (Windows, Ubuntu, Debian Jessie), and they are
>> working correctly. Debian Wheezy is the only one that fail.
> 
> I don't have an environment to reproduce this in. I think we should try
> to understand this problem better, before deciding how to make it go away.
> 
> Please locate the "StatusCodeRuntimeDxe.debug" file in your Build
> directory (ie. under the location listed in the error report). Then,
> please disassemble it with "objdump -S". The fault location in the
> disassembly can be found based on RIP, ImageBase and EntryPoint;

I don't think the exact instruction at that address really matters. The
main question appears to be why RIP and RSP both point into the
same page (see also the subject of Anthony's mail). I.e. we need to
spot the entity setting the stack to a page that also contains code,
or placing code on the stack. That's unlikely to be found by identifying
the instruction RIP points to, but rather (sadly not part of the state
dump) something higher up the call chain.

Jan


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

Re: [Xen-devel] [PATCH v2 3/4] tools: add tools support for Intel CDP

[Chao Peng]

On Wed, Sep 09, 2015 at 01:16:47PM +0800, He Chen wrote:
> This is the xl/xc changes to support Intel Code/Data Prioritization.
> CAT xl commands to set/get CBMs are extended to support CDP.
> 
> Signed-off-by: He Chen 
> ---
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 5f9047c..e4eb4df 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -796,6 +796,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, 
> libxl_mac *src);
>   * If this is defined, the Cache Allocation Technology feature is supported.
>   */
>  #define LIBXL_HAVE_PSR_CAT 1
> +
> +/*
> + * LIBXL_HAVE_PSR_CDP
> + *
> + * If this is defined, the Code/Data Prioritization feature is supported.
> + */
> +#define LIBXL_HAVE_PSR_CDP 1
>  #endif
>  
>  /*
> @@ -1729,6 +1736,9 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, 
> libxl_psr_cat_info **info,
>  void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr);
>  #endif
>  
> +#ifdef LIBXL_HAVE_PSR_CDP
> +#endif
> +

Why this? There are several of them in this patch.

>  /* misc */
>  
>  /* Each of these sets or clears the flag according to whether the
> diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> index 3378239..26939a5 100644
> --- a/tools/libxl/libxl_psr.c
> +++ b/tools/libxl/libxl_psr.c
> @@ -297,6 +297,7 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
>  GC_INIT(ctx);
>  int rc;
>  int socketid, nr_sockets;
> +libxl_psr_cat_info *info;
>  
>  rc = libxl__count_physical_sockets(gc, _sockets);
>  if (rc) {
> @@ -304,14 +305,41 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t 
> domid,
>  goto out;
>  }
>  
> +rc = libxl_psr_cat_get_l3_info(ctx, , _sockets);
> +if (rc) {
> +LOGE(ERROR, "Failed to get cat info");
> +goto out;
> +}
> +
> +if (!info->cdp_enabled &&
> +   (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
> +type == LIBXL_PSR_CBM_TYPE_L3_DATA))
> +{
> +LOGE(ERROR, "Unable to set Code/Data CBM with CDP disabled");
> +rc = EINVAL;
> +free(info);
> +goto out;
> +}
> +
>  libxl_for_each_set_bit(socketid, *target_map) {
>  if (socketid >= nr_sockets)
>  break;
> -if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, 
> cbm)) {
> +if (info->cdp_enabled && type == LIBXL_PSR_CBM_TYPE_L3_CBM)
> +{
> +if(xc_psr_cat_set_domain_data(ctx->xch, domid,
> +   LIBXL_PSR_CBM_TYPE_L3_CODE, socketid, cbm) ||
> +   xc_psr_cat_set_domain_data(ctx->xch, domid,
> +   LIBXL_PSR_CBM_TYPE_L3_DATA, socketid, cbm))
> +{
> +libxl__psr_cat_log_err_msg(gc, errno);
> +rc = ERROR_FAIL;
> +}

Will you merge the two if's?

Chao
> +} else if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, 
> socketid, cbm)) {
>  libxl__psr_cat_log_err_msg(gc, errno);
>  rc = ERROR_FAIL;
>  }
>  }
> +free(info);
>  
>  out:
>  GC_FREE;

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

Re: [Xen-devel] [PATCH v2 2/4] x86: add domctl cmd to set/get CDP code/data CBM

[Jan Beulich]

>>> On 09.09.15 at 09:24,  wrote:
> On Wed, Sep 09, 2015 at 01:16:46PM +0800, He Chen wrote:
>> -int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
>> +int psr_get_l3_cbm(struct domain *d, unsigned int socket,
>> +   uint64_t *cbm, enum cbm_type type)
>>  {
>>  struct psr_cat_socket_info *info = get_cat_socket_info(socket);
>>  
>>  if ( IS_ERR(info) )
>>  return PTR_ERR(info);
>>  
>> -*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
>> +if ( type == PSR_CBM_TYPE_L3 )
>> +{
>> +if ( info->cdp_enabled )
>> +return -EXDEV;
>> +*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
>> +}
>> +else if ( type == PSR_CBM_TYPE_L3_CODE )
>> +{
>> +if ( !info->cdp_enabled )
>> +return -EXDEV;
>> +*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.code;
>> +}
>> +else
>> +{
>> +if ( !info->cdp_enabled )
>> +return -EXDEV;
>> +*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.data;
>> +}
> 
> Could I suggest to use switch? And the check can be done in advance.

Indeed. And please consider carefully what the default case ought
to be.

>> @@ -371,65 +389,136 @@ static int write_l3_cbm(unsigned int socket, unsigned 
>> int cos,
>>  return 0;
>>  }
>>  
>> -int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
>> +static int find_cos(struct psr_cat_cbm *map, int cos_max,
>> +uint64_t cbm_code, uint64_t cbm_data, bool_t 
>> cdp_enabled)
>>  {
>> -unsigned int old_cos, cos;
>> -struct psr_cat_cbm *map, *found = NULL;
>> +unsigned int cos;
>> +
>> +if ( !cdp_enabled )
>> +{
>> +for ( cos = 0; cos <= cos_max; cos++ )
>> +if ( map[cos].ref && map[cos].u.cbm == cbm_code )
>> +return cos;
>> +}
>> +else
>> +{
>> +for ( cos = 0; cos <= cos_max; cos++ )
>> +if ( map[cos].ref && map[cos].u.cdp.code == cbm_code &&
>> + map[cos].u.cdp.data == cbm_data )
>> +return cos;
>> +}
> 
> I guess the '{' and '}' can be omitted if the body contains only one
> sentence.

Not really: They are required on the outer if(), and hence for
consistency they should be retained on the corresponding else.

Jan


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

Re: [Xen-devel] [PATCH v2 1/4] x86: Support enable CDP by boot parameter and add get CDP status

[He Chen]

On Wed, Sep 09, 2015 at 03:04:35PM +0800, Chao Peng wrote:
> On Wed, Sep 09, 2015 at 01:16:45PM +0800, He Chen wrote:
> > Intel Code/Data Prioritization(CDP) feature is based on CAT. cdp_enabled
> > is added to CAT socket info to indicate CDP is on or off on the socket,
> > note that cos_max would be half when CDP is on. struct psr_cat_cbm is
> > extended to support CDP operation. Extend psr_get_cat_l3_info sysctl to
> > get CDP status.
> > 
> > Signed-off-by: He Chen 
> > ---
> >  xen/arch/x86/psr.c  | 84 
> > ++---
> >  xen/arch/x86/sysctl.c   |  5 ++-
> >  xen/include/asm-x86/msr-index.h |  3 ++
> >  xen/include/asm-x86/psr.h   | 11 +-
> >  xen/include/public/sysctl.h |  2 +
> >  5 files changed, 89 insertions(+), 16 deletions(-)
> > 
> > diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> > index c0daa2e..ba0f726 100644
> > --- a/xen/arch/x86/psr.c
> > +++ b/xen/arch/x86/psr.c
> > @@ -17,13 +17,21 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> 
> Is this still needed?
> 

Obviously not, my mistake.

> >  #include 
> >  
> >  #define PSR_CMT(1<<0)
> >  #define PSR_CAT(1<<1)
> > +#define PSR_CDP(1<<2)
> >  
> >  struct psr_cat_cbm {
> > -uint64_t cbm;
> > +union {
> > +uint64_t cbm;
> > +struct {
> > +uint64_t code;
> > +uint64_t data;
> > +} cdp;
> > +} u;
> >  unsigned int ref;
> >  };
> >  
> > @@ -32,6 +40,7 @@ struct psr_cat_socket_info {
> >  unsigned int cos_max;
> >  struct psr_cat_cbm *cos_to_cbm;
> >  spinlock_t cbm_lock;
> 
> As you defined the cdp stauts for each socket here ...
> > +bool_t cdp_enabled;
> >  };
> >  
> >  struct psr_assoc {
> > @@ -43,6 +52,7 @@ struct psr_cmt *__read_mostly psr_cmt;
> >  
> >  static unsigned long *__read_mostly cat_socket_enable;
> >  static struct psr_cat_socket_info *__read_mostly cat_socket_info;
> > +static unsigned long *__read_mostly cdp_socket_enable;
> 
> ... this one then perhaps is redundency. If only one is need then I really
> like to keep the latter one.
> 

You're right. I would refine this.

> > @@ -470,6 +500,7 @@ static void cat_cpu_init(void)
> >  struct psr_cat_socket_info *info;
> >  unsigned int socket;
> >  unsigned int cpu = smp_processor_id();
> > +uint64_t val;
> >  const struct cpuinfo_x86 *c = cpu_data + cpu;
> >  
> >  if ( !cpu_has(c, X86_FEATURE_CAT) || c->cpuid_level < 
> > PSR_CPUID_LEVEL_CAT )
> > @@ -490,13 +521,34 @@ static void cat_cpu_init(void)
> >  info->cos_to_cbm = temp_cos_to_cbm;
> >  temp_cos_to_cbm = NULL;
> >  /* cos=0 is reserved as default cbm(all ones). */
> > -info->cos_to_cbm[0].cbm = (1ull << info->cbm_len) - 1;
> > +info->cos_to_cbm[0].u.cbm = (1ull << info->cbm_len) - 1;
> >  
> >  spin_lock_init(>cbm_lock);
> >  
> >  set_bit(socket, cat_socket_enable);
> >  printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, 
> > cbm_len:%u\n",
> > socket, info->cos_max, info->cbm_len);
> 
> If CDP is enable below, then this information is quite misleading.
> 

Is it proper to remove CAT printk info?

> > +
> > +if ( (ecx & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
> > +{
> > +if ( test_bit(socket, cdp_socket_enable) )
> > +return;
> > +
> > +rdmsrl(MSR_IA32_PSR_L3_QOS_CFG, val);
> > +wrmsrl(MSR_IA32_PSR_L3_QOS_CFG, val | 1 << 
> > PSR_L3_QOS_CDP_ENABLE_BIT);
> > +
> > +/* No need to write register since CBMs are fully open as 
> > default by HW */
> > +info->cos_to_cbm[0].u.cdp.code = (1ull << info->cbm_len) - 1;
> > +info->cos_to_cbm[0].u.cdp.data = (1ull << info->cbm_len) - 1;
> 
> If I remember correctly, HW is supposed to boot as CAT mode by default and
> only IA32_L3_QOS_Mask_0 is all ones, other mask msrs however are not mentioned
> in the spec which then may contain arbitrary value. So I guesss as
> least you need write all ones to IA32_L3_QOS_Mask_1 explicitly and that
> should be done before you enabled CDP.
> 

I have tested it at Broadwell server. I mean, I have used rdmsr to get mask0
and mask1 during booting and then check them, I found that they all ones by
default.
But since spec doesn't mention it as you said, I would write all ones to mask1
for better compatibility.

> > +
> > +/* Cut half of cos_max when CDP enabled */
> > +info->cos_max = info->cos_max / 2;
> > +
> > +info->cdp_enabled = 1;
> > +set_bit(socket, cdp_socket_enable);
> > +printk(XENLOG_INFO "CDP: enabled on socket %u, cos_max:%u, 
> > cbm_len:%u\n",
> > +   socket, info->cos_max, info->cbm_len);
> > +}
> >  }
> >  }
> >  
> > diff --git a/xen/include/asm-x86/msr-index.h 
> > b/xen/include/asm-x86/msr-index.h
> > 

Re: [Xen-devel] [PATCH v4 10/11] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Jan Beulich]

>>> On 09.09.15 at 10:49,  wrote:
> On 09/09/2015 16:32,  Jan Beulich wrote:
 On 09.09.15 at 10:11,  wrote:
>> On 24/07/2015 22:16,  Jan Beulich wrote:
> On 25.06.15 at 13:17,  wrote:
>>> --- a/xen/drivers/acpi/pmstat.c
>>> +++ b/xen/drivers/acpi/pmstat.c
>>> --- a/xen/include/public/sysctl.h
>>> +++ b/xen/include/public/sysctl.h
>>> @@ -315,8 +315,18 @@ struct xen_get_cpufreq_para {
>>>  uint32_t scaling_cur_freq;
>>>  
>>>  char scaling_governor[CPUFREQ_NAME_LEN];
>>> -uint32_t scaling_max_freq;
>>> -uint32_t scaling_min_freq;
>>> +
>>> +union {
>>> +uint32_t freq;
>>> +uint32_t pct;
>>> +} scaling_max;
>>> +
>>> +union {
>>> +uint32_t freq;
>>> +uint32_t pct;
>>> +} scaling_min;
>> 
>>>scaling_min and scaling_max should really be of the same type, so that
>> someone wanting to introduce helper functions
>>>or pointers to them can hand both interchangeably.
>> 
>>>Also I'm starting to get tired of repeating that it is still unclear 
>>>how a
>> consumer of the structure will know which of the
>>>two fields of the unions are applicable.
>> 
>>> Probably we don't need a union here. I plan to simply change them to 
>>> uint32_t scaling_max_perf; uint32_t scaling_max_perf;
>> 
>>> Then it's up to the driver to put what kind of value to it. It's like 
>>> we simply provide a drinking vessel, and it depends on the user to put 
>>> water or milk into it. In our case, the intel_pstate driver assigns a 
>>> percentage vale to it (in the "uint32_t" type), and the legacy driver 
>>> assigns the absolute value to it (in the "uint32_t" type, too).
> 
>>I don't see how this will solve the problem of the consumer not knowing what 
> kind of value it has to deal with.
> 
> The consumer is inside the print_cpufreq_para() function. I have put the 
> code below:

No, this is not "the" consumer but just one out of potentially many.

> +if (!strncmp(p_cpufreq->scaling_driver,
> +  "intel_pstate", CPUFREQ_NAME_LEN) )

And this is not really a proper way to distinguish which of an output
structure's sub-union is to be used. Just consider what happens to
the code when we end up gaining a few more drivers providing
percentage values, and perhaps another one providing a third
variant of output representation.

Jan


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

Re: [Xen-devel] xhci_hcd intterrupt affinity in Dom0/DomU limited to single interrupt

[Jan Beulich]

>>> On 08.09.15 at 18:02,  wrote:
> I believe the driver does support use of multiple interrupts based on
> the previous explanation of the lspci output where it was established
> that the device could use up to 8 interrupts which is what I see on bare
> metal.

Where is the proof of that? All I've seen is output like this

Capabilities: [80] MSI: Enable+ Count=1/8 Maskable- 64bit+

which says that one out of eight interrupts is being used. And
if in the native case this would indeed be the case, I don't think
you've provided complete hypervisor and kernel logs for the
Xen case so far, which would allow us to look for respective error
indications. And this (ignoring the line wrapping, which makes
things hard to read - it would be appreciated if you could fix
your mail client)...

> Bare metal:
> 
> cat /proc/interrupts 
>CPU0   CPU1   CPU2   CPU3   CPU4   CPU5
> CPU6   CPU7   
>   0: 36  0  0  0  0  0
> 0  0  IR-IO-APIC-edge  timer
>[...]
>  27: 337125  47893 708965   4049   53940667 263303
> 87847   4958  IR-PCI-MSI-edge  xhci_hcd

... also shows just a single interrupt being in use.

Jan


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

Re: [Xen-devel] Question to Xen log level in the case of PT

[Jan Beulich]

>>> On 09.09.15 at 08:53,  wrote:
>> > So can Xen change log level dynamically like Linux? If yes, we might
>>> change this level temporarily while passing through IGD. If not, any
>>> suggestion?
>>
>> First of all you could boot without lowering the log level (non-debug
>> builds) or raising the log level ("loglvl=warning"; debug builds). But
> 
> Sorry I don't know how to build "non-debug" here. Could you give me this 
> detail? Or where I can get this info?

Config.mk has

# A debug build of Xen and tools?
debug ?= y
debug_symbols ?= $(debug)

which you can alter or override on the make command line or in
.config. But for development purposes you really want to build
debug mode binaries, and rather adjust the log level on the
command line.

Jan


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

Re: [Xen-devel] [PATCH v2 3/4] tools: add tools support for Intel CDP

[He Chen]

On Wed, Sep 09, 2015 at 03:32:11PM +0800, Chao Peng wrote:
> On Wed, Sep 09, 2015 at 01:16:47PM +0800, He Chen wrote:
> > This is the xl/xc changes to support Intel Code/Data Prioritization.
> > CAT xl commands to set/get CBMs are extended to support CDP.
> > 
> > Signed-off-by: He Chen 
> > ---
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index 5f9047c..e4eb4df 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -796,6 +796,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, 
> > libxl_mac *src);
> >   * If this is defined, the Cache Allocation Technology feature is 
> > supported.
> >   */
> >  #define LIBXL_HAVE_PSR_CAT 1
> > +
> > +/*
> > + * LIBXL_HAVE_PSR_CDP
> > + *
> > + * If this is defined, the Code/Data Prioritization feature is supported.
> > + */
> > +#define LIBXL_HAVE_PSR_CDP 1
> >  #endif
> >  
> >  /*
> > @@ -1729,6 +1736,9 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, 
> > libxl_psr_cat_info **info,
> >  void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr);
> >  #endif
> >  
> > +#ifdef LIBXL_HAVE_PSR_CDP
> > +#endif
> > +
> 
> Why this? There are several of them in this patch.
> 

My carelessness. I would remove the redundant code lines.

> >  /* misc */
> >  
> >  /* Each of these sets or clears the flag according to whether the
> > diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> > index 3378239..26939a5 100644
> > --- a/tools/libxl/libxl_psr.c
> > +++ b/tools/libxl/libxl_psr.c
> > @@ -297,6 +297,7 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t 
> > domid,
> >  GC_INIT(ctx);
> >  int rc;
> >  int socketid, nr_sockets;
> > +libxl_psr_cat_info *info;
> >  
> >  rc = libxl__count_physical_sockets(gc, _sockets);
> >  if (rc) {
> > @@ -304,14 +305,41 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t 
> > domid,
> >  goto out;
> >  }
> >  
> > +rc = libxl_psr_cat_get_l3_info(ctx, , _sockets);
> > +if (rc) {
> > +LOGE(ERROR, "Failed to get cat info");
> > +goto out;
> > +}
> > +
> > +if (!info->cdp_enabled &&
> > +   (type == LIBXL_PSR_CBM_TYPE_L3_CODE ||
> > +type == LIBXL_PSR_CBM_TYPE_L3_DATA))
> > +{
> > +LOGE(ERROR, "Unable to set Code/Data CBM with CDP disabled");
> > +rc = EINVAL;
> > +free(info);
> > +goto out;
> > +}
> > +
> >  libxl_for_each_set_bit(socketid, *target_map) {
> >  if (socketid >= nr_sockets)
> >  break;
> > -if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, 
> > cbm)) {
> > +if (info->cdp_enabled && type == LIBXL_PSR_CBM_TYPE_L3_CBM)
> > +{
> > +if(xc_psr_cat_set_domain_data(ctx->xch, domid,
> > +   LIBXL_PSR_CBM_TYPE_L3_CODE, socketid, cbm) ||
> > +   xc_psr_cat_set_domain_data(ctx->xch, domid,
> > +   LIBXL_PSR_CBM_TYPE_L3_DATA, socketid, cbm))
> > +{
> > +libxl__psr_cat_log_err_msg(gc, errno);
> > +rc = ERROR_FAIL;
> > +}
> 
> Will you merge the two if's?
> 
> Chao

Surely.

> > +} else if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, 
> > socketid, cbm)) {
> >  libxl__psr_cat_log_err_msg(gc, errno);
> >  rc = ERROR_FAIL;
> >  }
> >  }
> > +free(info);
> >  
> >  out:
> >  GC_FREE;
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

Re: [Xen-devel] [PATCH] x86/hvm: fix saved pmtimer value

[Kouya Shimura]

Jan Beulich  writes:

 On 09.09.15 at 09:31,  wrote:
>> The ACPI PM timer is sometimes broken on live migration.
>> Since vcpu->arch.hvm_vcpu.guest_time is zero in most cases.
>
> I.e. in other than "delay for missed ticks mode". Would have been
> nice if you had spelled this out explicitly. 

Actually, I tried "delay for missed ticks mode" (timer_mode=0). 
Even in this case, the pmtimer was broken. I don't know why.

> With that the question then is - 
> should the field perhaps be used when non-zero, and the
> function only be called otherwise?

The adjustment of timer value in pmtimer_save() was introduced
before other timer_modes were implemented.

I'm not sure (skeptical) the small adjustment is really necessary 
in pmtimer_save.

Thanks,
Kouya

>
> Wei - regardless of the answer to the above (i.e. regardless of
> whether a v2 is going to be needed) I think we want this in 4.6.
>
> Jan

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

Re: [Xen-devel] [PATCH v4 10/11] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Jan Beulich]

>>> On 09.09.15 at 10:11,  wrote:
> On 24/07/2015 22:16,  Jan Beulich wrote:
 On 25.06.15 at 13:17,  wrote:
>> --- a/xen/drivers/acpi/pmstat.c
>> +++ b/xen/drivers/acpi/pmstat.c
>> --- a/xen/include/public/sysctl.h
>> +++ b/xen/include/public/sysctl.h
>> @@ -315,8 +315,18 @@ struct xen_get_cpufreq_para {
>>  uint32_t scaling_cur_freq;
>>  
>>  char scaling_governor[CPUFREQ_NAME_LEN];
>> -uint32_t scaling_max_freq;
>> -uint32_t scaling_min_freq;
>> +
>> +union {
>> +uint32_t freq;
>> +uint32_t pct;
>> +} scaling_max;
>> +
>> +union {
>> +uint32_t freq;
>> +uint32_t pct;
>> +} scaling_min;
> 
>>scaling_min and scaling_max should really be of the same type, so that 
> someone wanting to introduce helper functions 
>>or pointers to them can hand both interchangeably.
> 
>>Also I'm starting to get tired of repeating that it is still unclear how a 
> consumer of the structure will know which of the 
>>two fields of the unions are applicable.
> 
> Probably we don't need a union here. I plan to simply change them to 
> uint32_t scaling_max_perf;
> uint32_t scaling_max_perf;
> 
> Then it's up to the driver to put what kind of value to it. It's like we 
> simply provide a drinking vessel, and it depends on the user to put water or 
> milk into it. In our case, the intel_pstate driver assigns a percentage vale 
> to it (in the "uint32_t" type), and the legacy driver assigns the absolute 
> value to it (in the "uint32_t" type, too).

I don't see how this will solve the problem of the consumer not
knowing what kind of value it has to deal with.

Jan


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

Re: [Xen-devel] [PATCH] x86/hvm: fix saved pmtimer value

[Jan Beulich]

>>> On 09.09.15 at 10:31,  wrote:
> Jan Beulich  writes:
> 
> On 09.09.15 at 09:31,  wrote:
>>> The ACPI PM timer is sometimes broken on live migration.
>>> Since vcpu->arch.hvm_vcpu.guest_time is zero in most cases.
>>
>> I.e. in other than "delay for missed ticks mode". Would have been
>> nice if you had spelled this out explicitly. 
> 
> Actually, I tried "delay for missed ticks mode" (timer_mode=0). 
> Even in this case, the pmtimer was broken. I don't know why.

It would of course be helpful to understand why.

>> With that the question then is - 
>> should the field perhaps be used when non-zero, and the
>> function only be called otherwise?
> 
> The adjustment of timer value in pmtimer_save() was introduced
> before other timer_modes were implemented.
> 
> I'm not sure (skeptical) the small adjustment is really necessary 
> in pmtimer_save.

Together with the above, and with or without code adjustment, I'd
then like to ask for a v2 with an improved description. And please
don't forget to Cc maintainers of the code as well as Wei (the 4.6
release manager).

Jan


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

Re: [Xen-devel] Asus X99-A VT-d problems

[Jan Beulich]

>>> On 08.09.15 at 15:21,  wrote:
> I got Xen booting now with VT-d enabled, lot's of errors and the system is
> unstable. xl dmesg output:

Lots of errors? I don't see much - can you point out the errors you see?
There are two interesting things:

> (XEN) [VT-D]iommu.c:1149: drhd->address = fbffd000 iommu->reg = 
> 82c000201000
> (XEN) [VT-D]iommu.c:1151: cap = d2008c10ef0466 ecap = f0205b
>[...]
> (XEN) [VT-D]iommu.c:1149: drhd->address = fbffc000 iommu->reg = 
> 82c000203000
> (XEN) [VT-D]iommu.c:1151: cap = d2078c106f0466 ecap = f020df

Two IOMMUs with different capabilities. I'll have to look at what
specifically one supports which the other doesn't, but I wouldn't
be surprised if code wouldn't fully cope with that.

> (XEN) Failed to enable Interrupt Remapping: Will not enable x2APIC.

But later ...

> (XEN) Intel VT-d iommu 0 supported page sizes: 4kB, 2MB, 1GB.
> (XEN) Intel VT-d iommu 1 supported page sizes: 4kB, 2MB, 1GB.
> (XEN) Intel VT-d Snoop Control not enabled.
> (XEN) Intel VT-d Dom0 DMA Passthrough not enabled.
> (XEN) Intel VT-d Queued Invalidation enabled.
> (XEN) Intel VT-d Interrupt Remapping enabled.

... and ...

> (XEN) I/O virtualisation enabled
> (XEN)  - Dom0 mode: Relaxed
> (XEN) Interrupt remapping enabled

I'll have to check what this inconsistency means.

Apart from that the request for a native kernel log with IOMMU
enabled still stands (for comparison as well as quirk identification
purposes).

Jan


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

Re: [Xen-devel] [PATCH V6 3/7] libxl: add pvusb API

[Chun Yan Liu]

>>> On 9/9/2015 at 12:52 AM, in message <55ef1244@citrix.com>, George Dunlap
 wrote: 
> On 09/08/2015 03:17 PM, Ian Campbell wrote: 
> > On Mon, 2015-08-10 at 18:35 +0800, Chunyan Liu wrote: 
> >  
> > Sorry for the delay, between 4.6 freeze crunch, conference and vacation 
> > I've been a bit swamped. 
> >  
> > I'm just going to comment on the APIs (mainly public libxl.h and .idl) in 
> > this pass. 
> >  
> >> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h 
> >> index 5f9047c..05b6331 100644 
> >> --- a/tools/libxl/libxl.h 
> >> +++ b/tools/libxl/libxl.h 
> >> @@ -123,6 +123,23 @@ 
> >>  #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1 
> >>   
> >>  /* 
> >> + * LIBXL_HAVE_PVUSB indicates the functions for doing hot-plug of 
> >  
> > And cold-plug, no? 
>  
> So you should probably say something like "indicates functions for 
> plugging in USB devices through pvusb -- both hotplug and at domain 
> creation time." 

Thanks. Will clarify.

>  
> >> +libxl_usbctrl_type = Enumeration("usbctrl_type", [ 
> >> +(0, "AUTO"), 
> >  
> > What are the proposed semantics of using LIBXL_USBCTRL_TYPE_AUTO? 
>  
> Generally "DTRT".  Meaning: 
> 1. If your domain has no devicemodel, use PV. 
> 2. If your device has a devicemodel, and no PV drivers have peen 
> detected, use the devicemodel. 
> 3. If your device has a devicemodel, but PV drivers have been detected, 
> use PV. 
>  
> At the moment we don't have a way to check for PV drivers, so this just 
> collapses down to "PV for domains without a DM and DM for domains with a 
> DM." 

Better to be: by default, PV for PV guest and DM for HVM guest.

Thanks,
Chunyan

>  
> >  
> >> +(1, "PV"), 
> >> +(2, "QEMU"), 
> >  
> > Is "QEMU" what we want here, as opposed to, say, "EMU" (similar to NICs)? 
>  
> I had this as "DEVICEMODEL", since what we mean is that we want the 
> device model to provide access (and in theory in the future we may use a 
> different device model).  But "EMU" works for me too. 
>  
> > I think we probably don't want to go as fine grained as "XHCI" and "EHCI" 
> > etc, do we? I see we have a version field below, is it intended that there 
> > be some way to select between e.g. UHCI and OHCI (which IIRC are different 
> > USB 1.0 controllers). 
> >  
> > Maybe these questions should all be left aside for when QMEU support is 
> > actually added (AFAICT this field is just a placeholder)? In fact I glanced 
> > at the code and was surprised to find nothing checking for 
> > LIBXL_USBCTRL_TYPE at all, did I miss something? 
> >  
> > I think the two choices are: 
> >  
> > We can decide quickly and easily what the option(s) other than PV should be 
> > here and you include it in the IDL, but you would then need to check 
> > usbctrl->type == PV at various points, not silently treat all options as 
> > PV. 
> >  
> > Or this becomes a long conversation in which case I think your best bet 
> > would be to leave the enum with just the PV (and maybe AUTO) entries and 
> > leave the decision on the name for the emulated option to the series which 
> > implements that. 
>  
> I think the idea was to simply offer 1, 2, and 3 as options, and for the 
> devicemodel version, choose a suitable controller (or set of 
> controllers) for each option; similar to what usbversion= does now. 
>  
> >  
> >> +]) 
> >> + 
> >> +libxl_usbdev_type = Enumeration("usbdev_type", [ 
> >> +(0, "invalid"), 
> >> +(1, "hostdev"), 
> >> +]) 
> >> + 
> >> +libxl_device_usbctrl = Struct("device_usbctrl", [ 
> >> +("type", libxl_usbctrl_type), 
> >> +("devid", libxl_devid), 
> >> +("version", integer), 
> >> +("ports", integer), 
> >> +("backend_domid", libxl_domid), 
> >> +("backend_domname", string), 
> >> +   ]) 
> >> + 
> >> +libxl_device_usb = Struct("device_usb", [ 
> >> +("ctrl", libxl_devid), 
> >> +("port", integer), 
> >> +("u", KeyedUnion(None, libxl_usbdev_type, "devtype", 
> >> +   [("hostdev", Struct(None, [ 
> >> + ("hostbus",   integer), 
> >> + ("hostaddr",  integer)])), 
> >> +("invalid", None), 
> >  
> > AIUI this is what was agreed to, i.e. an enum with only one real option, in 
> > order to leave a space for new devtypes without major API overhaul. 
> >  
> > Please can you confirm that hostbus and hostaddr are both flat integer 
> > namespaces (i.e. there is no structure to the bits within either, they are 
> > just a number). 
>  
> I can confirm this. 
>  
>  -George 
>  
>  
>  


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

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

[Ian Campbell]

On Tue, 2015-09-08 at 17:13 +0100, Ian Jackson wrote:
> Jan Beulich writes ("Re: [Xen-devel] [xen-unstable test] 61521:
> regressions - FAIL"):
> > That's what I was about to say in a reply to your earlier mail. To me
> > this still means the guest booted up successfully (or else it wouldn't
> > have accepted the TCP connection). I agree its "ssh server" service
> > is unavailable (or unreliable), which is bad, but which then again we
> > have no real handle to deal with without someone running into it
> > outside of osstest.
> 
> Right.
> 
> > Whether to use this as justification for a force push I'm not sure
> > anyway.
> 
> I have looked a the histories of various `debianhvm' tests in many
> other osstest `branches' and there do seem to be occasional similar
> failures elsewhere.  So Ian C wasn't right to remember that this test

was ???

> step might be unreliable.
> 
> I think based on this, and Wei's analysis, a force push is justified.

>From the original report:
> version targeted for testing:
>  xen  a7b39c8bd6cba3fe1c8012987b9e28bdbac7e92d
> baseline version:
>  xen  801ab48e5556cb54f67e3cb57f077f47e8663ced

Therefore I have force pushed a7b39c8bd6cba3fe1c8012987b9e28bdbac7e92d.

Ian.

$ OSSTEST_CONFIG=production-config ./ap-push xen-unstable 
a7b39c8bd6cba3fe1c8012987b9e28bdbac7e92d
+ branch=xen-unstable
+ revision=a7b39c8bd6cba3fe1c8012987b9e28bdbac7e92d
+ . cri-lock-repos
++ . cri-common
+++ . cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable 
a7b39c8bd6cba3fe1c8012987b9e28bdbac7e92d
+ branch=xen-unstable
+ revision=a7b39c8bd6cba3fe1c8012987b9e28bdbac7e92d
+ . cri-lock-repos
++ . cri-common
+++ . cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . cri-common
++ . cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
+ local b
+ local p
++ ./mg-list-all-branches
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ '[' xxen-4.0-testing = xxen-unstable ']'
+ p=xen-4.0-testing
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ '[' xxen-4.1-testing = xxen-unstable ']'
+ p=xen-4.1-testing
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ '[' xxen-4.2-testing = xxen-unstable ']'
+ p=xen-4.2-testing
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ '[' xxen-4.3-testing = xxen-unstable ']'
+ p=xen-4.3-testing
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ '[' xxen-4.4-testing = xxen-unstable ']'
+ p=xen-4.4-testing
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ '[' xxen-4.5-testing = xxen-unstable ']'
+ p=xen-4.5-testing
+ for b in '$(./mg-list-all-branches)'
+ case "$b" in
+ '[' xxen-unstable 

Re: [Xen-devel] OVMF BoF @ KVM Forum 2015

[Laszlo Ersek]

On 08/10/15 18:24, Laszlo Ersek wrote:
> Hi.
> 
> Let's do an OVMF BoF at this year's KVM Forum too.

Here's a preliminary task list, after some off-list discussion (I tried
to incorporate comments):

- create GPL'd fork called "ovmf" for expediting virt development
  (OvmfPkg, ArmVirtPkg)
  - maybe leverage the feature under
 for
setting up a separate "tianocore/edk2-gpl" repo, for GPL'd
contributions [Jordan]
- repo separation by license could make things harder for packagers
  and QEMU bundling [Laszlo]
- document the rules / justification for "ovmf" (licensing
  conflicts, non-technical blockage on edk2 etc).
- No new mailing list needed
- push RH's downstream-only patches to "ovmf", wherever that makes
  sense
- remove encumbered FAT driver
- import Peter Batard's GPL r/o FAT driver port of GRUB's
- secure OpenSSL linking exception for the former from the copyright
  holders (Peter Batard, GRUB project)
- "ovmf" should be periodically rebased / should fetch+merge edk2 as
  master (arguments both for and against merging); distros should
  then track "ovmf" as their upstream, not edk2
- get OVMF into Fedora (as pkg) and QEMU (as bundled binary)
- do OVMF releases, maybe in sync with QEMU's releases
  - we can probably build from known good revisions from git [Alex]
- revive Q35 SATA driver work / poke Reza
  - Hannes and Gabriel have refreshed patches, but their versions differ


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

Re: [Xen-devel] [linux-linus test] 61295: regressions - FAIL

[Ian Campbell]

On Thu, 2015-09-03 at 23:29 +, osstest service owner wrote:
> flight 61295 linux-linus real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/61295/
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> [...]
>  test-armhf-armhf-xl-cubietruck  6 xen-bootfail REGR. vs. 
> 59254
> [...]

The bisector has fingered[0] a merge commit, f36fc04e4cdd, from the clk tree.

The serial log[1] shows silence from dom0, and the debug keys then report:

Sep  3 05:38:14.774354 (XEN) [ Xen-4.6.0-rc  arm32  debug=y  Not tainted 
]
Sep  3 05:38:14.779727 (XEN) CPU:0
Sep  3 05:38:14.781104 (XEN) PC: c024a9dc
Sep  3 05:38:14.783225 (XEN) CPSR:   61d3 MODE:32-bit Guest SVC
Sep  3 05:38:14.787230 (XEN)  R0:  R1: 0001 R2:  R3: 

Sep  3 05:38:14.792737 (XEN)  R4: c0ea5ec4 R5: c0dabda0 R6: 000b R7: 
c0db107c
Sep  3 05:38:14.798352 (XEN)  R8: c02bfeb2 R9: 61d3 R10:c0dabc82 
R11: R12:c0dabc38
Sep  3 05:38:14.804979 (XEN) USR: SP:  LR: 
Sep  3 05:38:14.808228 (XEN) SVC: SP: c0dabc00 LR: c091ff44 SPSR:01d3
Sep  3 05:38:14.812858 (XEN) ABT: SP: c0ea5a8c LR: c02147e0 SPSR:a1d3
Sep  3 05:38:14.817358 (XEN) UND: SP: c0ea5a98 LR: c0ea5a98 SPSR:
Sep  3 05:38:14.821857 (XEN) IRQ: SP: c0ea5a80 LR: c0214840 SPSR:a1d3
Sep  3 05:38:14.826361 (XEN) FIQ: SP: c0ea5aa4 LR: c0ea5aa4 SPSR:
Sep  3 05:38:14.830855 (XEN) FIQ: R8:  R9:  R10: 
R11: R12:
Sep  3 05:38:14.837482 (XEN) 
Sep  3 05:38:14.838229 (XEN)  SCTLR: 10c5387d
Sep  3 05:38:14.840605 (XEN)TCR: 
Sep  3 05:38:14.842990 (XEN)  TTBR0: 6020406a
Sep  3 05:38:14.846108 (XEN)  TTBR1: 6020406a
Sep  3 05:38:14.849232 (XEN)   IFAR: , IFSR: 
Sep  3 05:38:14.853107 (XEN)   DFAR: , DFSR: 0005
Sep  3 05:38:14.856975 (XEN) 
Sep  3 05:38:14.857608 (XEN)   VTCR_EL2: 80003558
Sep  3 05:38:14.859986 (XEN)  VTTBR_EL2: 0001bfefe000
Sep  3 05:38:14.863108 (XEN) 
Sep  3 05:38:14.863855 (XEN)  SCTLR_EL2: 30cd187f
Sep  3 05:38:14.866236 (XEN)HCR_EL2: 0038643f
Sep  3 05:38:14.869357 (XEN)  TTBR0_EL2: bfef
Sep  3 05:38:14.872481 (XEN) 
Sep  3 05:38:14.873236 (XEN)ESR_EL2: 93810007
Sep  3 05:38:14.875610 (XEN)  HPFAR_EL2: 0001c810
Sep  3 05:38:14.878735 (XEN)  HDFAR: e0800c1c
Sep  3 05:38:14.881236 (XEN)  HIFAR: b0108114
Sep  3 05:38:14.883611 (XEN) 
Sep  3 05:38:14.884236 (XEN) Guest stack trace from sp=c0dabc00:
Sep  3 05:38:14.887987 (XEN)c0b600b8 c0dabc24  c0ea5ec4 c0dabda0 
000b c0db107c c02bfeb2
Sep  3 05:38:14.895110 (XEN)c0dabc82 c0213f80 c0b4e034 61d3 0001 
0001 c0daa220 000b
Sep  3 05:38:14.902236 (XEN)c0ecb290 c0dabda0  0008 6500 
38373937 20343030 64393565
Sep  3 05:38:14.909364 (XEN)30313063 38356520 31303064 31652034 30306335 
28203830 38313465 30323039
Sep  3 05:38:14.916489 (XEN)c0002029 c0dabca4 c02afa50   
0005 c0dabda0 0005
Sep  3 05:38:14.923617 (XEN)00989680 c0daff28 c0dabe4c c091f82c  
c021f1b0  c0dabcd1
Sep  3 05:38:14.930616 (XEN) c04810b0   367830c8 
38303230 00633730 0005
Sep  3 05:38:14.937769 (XEN) c0db1404 c0dabda0  00989680 
c0daff28 c0dabe4c c020a238
Sep  3 05:38:14.944981 (XEN)c0ea976b 000a c0ea976b 0030 c0ecd5c8 
0002  
Sep  3 05:38:14.951988 (XEN) 0002  c0ecd5f8 c0ecd630 
c029bdc4  7fff
Sep  3 05:38:14.959116 (XEN)32313030 0290 0250 0250 98968000 
 9aca c0ecd448
Sep  3 05:38:14.966240 (XEN)0290 c029dd28   8ad1 
 0010 000a
Sep  3 05:38:14.973238 (XEN)9aca 003b 0001 c02bfeb0 a1d3 
 c0dabdd4 c0214818
Sep  3 05:38:14.980364 (XEN)   1ae0a000 c0da5fc8 
c0da5fd0 1ae0a000 1ae0a000
Sep  3 05:38:14.987489 (XEN) 00989680 c0daff28 c0dabe4c dbbaffc8 
c0dabde8 c026ae1c c02bfeb0
Sep  3 05:38:14.994617 (XEN)a1d3  c0dabe28 0018 00989680 
 dbbaffc8 
Sep  3 05:38:15.001742 (XEN)dbbafc00 1ae0a000 1dcd6500 dbbb22c0 c0da82c0 
1ae0a000  c0dac6b4
Sep  3 05:38:15.008949 (XEN)00989680 c0daff28 c0dabe4c c026ae1c c0daff28 
 db0145c0 dbbb4e80
Sep  3 05:38:15.015988 (XEN)   c02965b8 a153 
dbbb4e80 c0de8a84 c02a360c
Sep  3 05:38:15.023115 (XEN) c0286db8 db003e40 c0de8a84 db0145c0 
dbbb4e80 0013 410fc074


Between 26f8b7edc9ea and f36fc04e4cdd there is:

 * no diff to arch/arm/mach-sunxi/ 
 * addition or changes of "clock-indices" property in many
   arch/arm/boot/dts/*sun?i*
 * a bunch of changes in drivers/clk/*sun?i*

Ian.
[0] 

Re: [Xen-devel] [PATCH v4 10/11] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Jan Beulich]

>>> On 09.09.15 at 12:35,  wrote:
> On 09/09/2015 18:10,  Jan Beulich wrote:
 On 09.09.15 at 11:35,  wrote:
>>> Using the drinking vessel analogy, we are not putting milk and water 
>>> into the vessel at the same time. If the producer puts water into the 
>>> vessel, then the consumer simply consumes water; If the producer puts 
>>> milk into the vessel, then the consumer simply consumes milk. I think 
>>> we don't need to worry about what type of drinking is put inside the 
>>> vessel, because the vessel is just an intermediate place holding the 
>>>liquid between the producer and consumer - the consumer has the 
>>> privity of contract with the producer and it has the right logic to deal 
> with what's inside the vessel.
> 
>> This analogy breaks for a blind: How would he know whether there's water or 
> milk in there? He'd have to try it. 
>>Now what if we use >venom instead of milk in your analogy?
> 
>>Bottom line - the consumer needs to know what kind of data it is to expect to 
> consume.
> 
> There is a contract between the consumer and the producer. In our case, the 
> contract is "p_cpufreq->scaling_driver". Before the right consumer consumes 
> the 
> value of " uint32_t scaling_max_perf ", it goes through the check:
>  +if (!strncmp(p_cpufreq->scaling_driver,
>  +  "intel_pstate", CPUFREQ_NAME_LEN) )
> , where "intel_pstate" can be changed to other new driver names (contract) 
> in the future.

Which is precisely what I already said doesn't scale.

Jan


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

Re: [Xen-devel] [Xen-users] Xen bridging issue.

[Wei Liu]

On Wed, Sep 09, 2015 at 10:09:09AM +0300, johnny Strom wrote:
[...]
> 
> Hello again
> 
> I am not sure but there might be another issue with the xen_netback module
> in Debian jessie.
> 
> I am not able to set the  max_queues options so that it is set at load time.
> 
> I have tested with the loop kernel module where it works to set an value but
> doing the same for
> the xen_netback driver dose not work:
> 
> This works with loop module like this:
> options loop max_loop=50
> 
> And then doing the same with the xen_netback that dose not work:
> options xen_netback max_queues=4
> 
> Or is there some other way it should be set in
> /etc/modprobe.d/xen_netback.conf?
> 
> 
> Best regards Johnny
> 

After looking at the code more carefully I think that's a bug.

I will send a patch to fix it when I get around to do it. In the mean
time (till the bug fix is propagated to Debian, that probably takes
quite a bit of time), you can use a script to echo the value you want to
the control knob during system startup.

I will put a Reported-by tag with your email address in my patch if you
don't mind. I will also CC you on that patch I'm going to send so that
you have an idea when it's merged upstreamed.

Wei.

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

[Xen-devel] [linux-linus test] 61594: regressions - trouble: broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl   14 guest-saverestore fail REGR. vs. 59254
 test-amd64-i386-xl-xsm   14 guest-saverestore fail REGR. vs. 59254
 test-armhf-armhf-xl-arndale   8 leak-check/basis(8)   fail REGR. vs. 59254
 test-armhf-armhf-xl-multivcpu  6 xen-boot fail REGR. vs. 59254
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 15 guest-localmigrate.2 
fail REGR. vs. 59254
 test-amd64-i386-pair   21 guest-migrate/src_host/dst_host fail REGR. vs. 59254
 test-armhf-armhf-xl-xsm   8 leak-check/basis(8)   fail REGR. vs. 59254
 test-armhf-armhf-xl   8 leak-check/basis(8)   fail REGR. vs. 59254
 test-armhf-armhf-xl-cubietruck  6 xen-bootfail REGR. vs. 59254
 test-armhf-armhf-xl-credit2   8 leak-check/basis(8)   fail REGR. vs. 59254
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 19 guest-start/debianhvm.repeat 
fail REGR. vs. 59254

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt  8 leak-check/basis(8)   fail REGR. vs. 59254
 test-armhf-armhf-libvirt-xsm  8 leak-check/basis(8)   fail REGR. vs. 59254
 test-armhf-armhf-xl-rtds  8 leak-check/basis(8)   fail REGR. vs. 59254
 test-armhf-armhf-xl-qcow2 8 leak-check/basis(8) fail baseline untested
 test-armhf-armhf-libvirt-vhd  8 leak-check/basis(8) fail baseline untested
 test-armhf-armhf-libvirt-raw  8 leak-check/basis(8) fail baseline untested
 test-armhf-armhf-libvirt-qcow2  6 xen-boot  fail baseline untested
 test-armhf-armhf-xl-vhd   8 leak-check/basis(8) fail baseline untested
 test-armhf-armhf-xl-raw   8 leak-check/basis(8) fail baseline untested
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail like 59254
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail like 59254

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  14 guest-saverestorefail   never pass
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-amd64-i386-libvirt  14 guest-saverestorefail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail never pass
 test-amd64-amd64-libvirt-pair 21 guest-migrate/src_host/dst_host fail never 
pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-amd64-i386-libvirt-qcow2 11 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt 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-vhd  11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  11 migrate-support-checkfail   never pass

version targeted for testing:
 linux4e4adb2f462889b9eac736dd06d60658beb091b6
baseline version:
 linux45820c294fe1b1a9df495d57f40585ef2d069a39

Last test of basis59254  2015-07-09 04:20:48 Z   62 days
Failing since 59348  2015-07-10 04:24:05 Z   61 days   37 attempts
Testing same since61594  2015-09-08 04:27:30 Z1 days1 attempts


2050 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvops  

Re: [Xen-devel] [Patch V1 1/3] xen: introduce dummy system device

[Juergen Gross]

On 09/07/2015 05:29 PM, Stefano Stabellini wrote:

On Thu, 3 Sep 2015, Juergen Gross wrote:

Introduce a new dummy system device serving as parent for virtual
buses. This will enable new pv backends to introduce virtual buses
which are removable again opposed to system buses which are meant
to stay once added.

Signed-off-by: Juergen Gross 
---
  hw/xenpv/xen_machine_pv.c| 39 +++
  include/hw/xen/xen_backend.h |  1 +
  2 files changed, 40 insertions(+)

diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c
index 2e545d2..57bc071 100644
--- a/hw/xenpv/xen_machine_pv.c
+++ b/hw/xenpv/xen_machine_pv.c
@@ -24,10 +24,15 @@

  #include "hw/hw.h"
  #include "hw/boards.h"
+#include "hw/sysbus.h"
  #include "hw/xen/xen_backend.h"
  #include "xen_domainbuild.h"
  #include "sysemu/block-backend.h"

+#define TYPE_XENSYSDEV "xensysdev"
+
+DeviceState *xen_sysdev;
+
  static void xen_init_pv(MachineState *machine)
  {
  const char *kernel_filename = machine->kernel_filename;
@@ -59,6 +64,9 @@ static void xen_init_pv(MachineState *machine)
  break;
  }

+xen_sysdev = qdev_create(NULL, TYPE_XENSYSDEV);
+qdev_init_nofail(xen_sysdev);
+
  xen_be_register("console", _console_ops);
  xen_be_register("vkbd", _kbdmouse_ops);
  xen_be_register("vfb", _framebuffer_ops);
@@ -93,6 +101,31 @@ static void xen_init_pv(MachineState *machine)
  xen_init_display(xen_domid);
  }

+static int xen_sysdev_init(SysBusDevice *dev)
+{
+return 0;
+}
+
+static Property xen_sysdev_properties[] = {
+{/* end of property list */},
+};
+
+static void xen_sysdev_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+k->init = xen_sysdev_init;
+dc->props = xen_sysdev_properties;
+}
+
+static const TypeInfo xensysdev_info = {
+.name  = TYPE_XENSYSDEV,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(SysBusDevice),
+.class_init= xen_sysdev_class_init,
+};
+
  static QEMUMachine xenpv_machine = {
  .name = "xenpv",
  .desc = "Xen Para-virtualized PC",
@@ -101,9 +134,15 @@ static QEMUMachine xenpv_machine = {
  .default_machine_opts = "accel=xen",
  };

+static void xenpv_register_types(void)
+{
+type_register_static(_info);
+}


Given that you need this just for usbback, I wonder if you could
move xen_sysdev and its initalization to usbback_init.


Sure I could. OTOH I think this system device could really be of common
interest.


Juergen

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

Re: [Xen-devel] On distro packaging of stub domains (Re: Notes from Xen BoF at Debconf15)

[Ian Campbell]

On Tue, 2015-09-08 at 18:38 +, Antti Kantee wrote:
> On 08/09/15 16:15, Ian Campbell wrote:
> > On Tue, 2015-09-08 at 15:03 +, Antti Kantee wrote:
> > 
> > > For unikernels, the rump kernel project provides Rumprun, which can
> > > provide you with a near-full POSIX'y interface.
> > 
> > I'm not 100% clear: Does rumprun _build_ or _run_ the application? It
> > sound
> > s like it builds but the name suggests otherwise.
> 
> For all practical purposes, Rumprun is an OS, except that you always 
> cross-compile for it.  So, I'd say "yes", but it depends on how you want 
> to interpret the situation.  We could spend days writing emails back and 
> forth, but there's really no substitute for an hour of hands-on 
> experimentation.
> 
> (nb. the launch tool for launching Rumprun instances is currently called 
> rumprun.  It's on my todo list to propose changing the name of the tool 
> to e.g. rumprunner or runrump or something which is distinct from the OS 
> name, since similarity causes some confusion)

Thanks, I think I get it...


> > Do these wrappers make a rump kernel build target look just like any
> > other
> > ross build target? (I've just got to the end and found my answer, which
> > was
> > yes. I've left this next section in since I think it's a nice summary
> > of
> > why it matters that the answer is yes)
> > 
> > e.g. I have aarch64-linux-gnu-{gcc,as,ld,ar,etc} which I can use to
> > build
> > aarch64 binaries on my x86_64 host, including picking up aarch64
> > libraries
> > and headers from the correct arch specific patch.
> > 
> > Do these rumprun-provided wrappers provide x86_64-rumpkernel
> > -{gcc,as,ld,ar,etc} ?
> 
> No, like I said and which you discovered later, 
> x86_64-rumprun-netbsd-{gcc,as,ld,ar,etc}.  aarch64 would be 
> aarch64-rumprun-netbsd-{...}.

Sorry, I used an explicit example when really I just meant "some triplet"
without saying "such as" or "e.g.".

So the answer to the question I wanted to ask (rather than the one I did)
is "yes", which is good!

> > > If the above didn't explain the grand scheme of things clearly, have a
> > > look at http://wiki.rumpkernel.org/Repo and especially the picture.  If
> > > things are still not clear after that, please point out matters of
> > > confusion and I will try to improve the explanations.
> > 
> > I think that wiki page is clear, but I think it's orthogonal to the issue
> > with distro packaging of rump kernels.
> 
> Sure, but I wanted to get the concepts right.  And they're still not 
> right.  We're talking about packaging for *Rumprun*, not rump kernels in 
> general.

Right.

> > >However, since a) nobody (else) ships applications as relocatable
> > > static objects b) Rumprun does not support shared libraries, I don't
> > > know how helpful the fact of ABI compatibility is.  IMO, adding
> > > shared
> > > library support would be a backwards way to go: increasing runtime
> > > processing and memory requirements to solve a build problem sounds
> > > plain
> > > weird.  So, I don't think you can leverage anything existing.
> > 
> > This is an interesting point, since not building a shared library is
> > already therefore requiring packaging changes which are going to be at
> > least a little bit rumpkernel specific.
> > 
> > Is it at all possible (even theoretically) to take a shared library
> > (which
> > is relocatable as required) and to do a compile time static linking
> > pass on
> > it? i.e. use libfoo.so but still do static linking?
> 
> But shared libraries aren't "relocatable", that's the whole point of 
> shared libraries! ;) ;)

Hrm, perhaps I'm confusing PIC with relocatable but AIUI a shared library
can be loaded at any address (subject to some constraints) in a process and
may be loaded at different addresses in different processes, which is what
you actually need to do...

> I guess you could theoretically link shared libs with a different ld, 

...this. (assuming you meant "link an app against shared libs")

> and I don't think it would be very different from prelinking shared 
> libs,

Indeed.

>  but as Samuel demonstrated, it won't work at least with an 
> out-of-the-box ld.

Right, I thought it probably wasn't, which is why I said "even
theoretically".

> I think it's easier to blame Solaris for the world going bonkers with 
> shared libs, bite the bullet, and start adding static linking back where 
> it's been ripped out from.  Shared libs make zero sense for unikernels 
> since you don't have anyone to share them with, so you're just paying 
> extra for PIC for absolutely no return.  (dynamically loadable code is a 
> separate issue, if you even want to go there ... I wouldn't)

The issue, and the reason I mentioned it, is that distros (at least Linux
distros) have, for better or worse, gone in heavily for the use of shared
libraries in their application packaging norms.

Actually distros might be (e.g. Debian is) quite good at always providing a
.a as well as the .so when packaging libraries 

Re: [Xen-devel] [PATCH v4 10/11] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Jan Beulich]

>>> On 09.09.15 at 11:35,  wrote:
> Using the drinking vessel analogy, we are not putting milk and water into 
> the vessel at the same time. If the producer puts water into the vessel, then 
> the consumer simply consumes water; If the producer puts milk into the 
> vessel, then the consumer simply consumes milk. I think we don't need to 
> worry about what type of drinking is put inside the vessel, because the 
> vessel is just an intermediate place holding the liquid between the producer 
> and consumer - the consumer has the privity of contract with the producer and 
> it has the right logic to deal with what's inside the vessel.

This analogy breaks for a blind: How would he know whether there's
water or milk in there? He'd have to try it. Now what if we use venom
instead of milk in your analogy?

Bottom line - the consumer needs to know what kind of data it is to
expect to consume.

Jan


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

Re: [Xen-devel] [PATCH net] xen-netback: respect user provided max_queues

[Wei Liu]

On Wed, Sep 09, 2015 at 11:12:44AM +0100, David Vrabel wrote:
> On 09/09/15 11:09, Wei Liu wrote:
> > Originally that parameter was always reset to num_online_cpus during
> > module initialisation, which renders it useless.
> > 
> > The fix is to only set max_queues to num_online_cpus when user has not
> > provided a value.
> [...]
> > --- a/drivers/net/xen-netback/netback.c
> > +++ b/drivers/net/xen-netback/netback.c
> > @@ -67,7 +67,7 @@ module_param(rx_drain_timeout_msecs, uint, 0444);
> >  unsigned int rx_stall_timeout_msecs = 6;
> >  module_param(rx_stall_timeout_msecs, uint, 0444);
> >  
> > -unsigned int xenvif_max_queues;
> > +unsigned int xenvif_max_queues = 0;
> 
> You don't need this.
> 
> Otherwise,
> 
> Reviewed-by: David Vrabel 
> 
> Is an equivalent fix needed in netfront?
> 

I think so.

I will address your comment and send both fixes (front and back) in a
series.

Wei.

> David
> 
> >  module_param_named(max_queues, xenvif_max_queues, uint, 0644);
> >  MODULE_PARM_DESC(max_queues,
> >  "Maximum number of queues per virtual interface");
> > @@ -2105,8 +2105,11 @@ static int __init netback_init(void)
> > if (!xen_domain())
> > return -ENODEV;
> >  
> > -   /* Allow as many queues as there are CPUs, by default */
> > -   xenvif_max_queues = num_online_cpus();
> > +   /* Allow as many queues as there are CPUs if user has not
> > +* specified a value.
> > +*/
> > +   if (xenvif_max_queues == 0)
> > +   xenvif_max_queues = num_online_cpus();
> >  
> > if (fatal_skb_slots < XEN_NETBK_LEGACY_SLOTS_MAX) {
> > pr_info("fatal_skb_slots too small (%d), bump it to 
> > XEN_NETBK_LEGACY_SLOTS_MAX (%d)\n",
> > 

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

[Xen-devel] [PATCH net v2 0/2] xen-net{front, back}: respect user provided max_queues

[Wei Liu]

Wei Liu (2):
  xen-netback: respect user provided max_queues
  xen-netfront: respect user provided max_queues

 drivers/net/xen-netback/netback.c | 7 +--
 drivers/net/xen-netfront.c| 7 +--
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.1.4


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

[Xen-devel] [PATCH net v2 2/2] xen-netfront: respect user provided max_queues

[Wei Liu]

Originally that parameter was always reset to num_online_cpus during
module initialisation, which renders it useless.

The fix is to only set max_queues to num_online_cpus when user has not
provided a value.

Signed-off-by: Wei Liu 
Cc: David Vrabel 
---
 drivers/net/xen-netfront.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index e27e6d2..0b53bd9 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -2132,8 +2132,11 @@ static int __init netif_init(void)
 
pr_info("Initialising Xen virtual ethernet driver\n");
 
-   /* Allow as many queues as there are CPUs, by default */
-   xennet_max_queues = num_online_cpus();
+   /* Allow as many queues as there are CPUs if user has not
+* specified a value.
+*/
+   if (xenvif_max_queues == 0)
+   xenvif_max_queues = num_online_cpus();
 
return xenbus_register_frontend(_driver);
 }
-- 
2.1.4


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

Re: [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Ian Campbell]

On Wed, 2015-09-09 at 01:06 -0600, Jan Beulich wrote:
> > > > On 09.09.15 at 00:23,  wrote:
> > On 09/08/15 19:26, Anthony PERARD wrote:
> > > And I get this on the console:
> > > Welcome to GRUB!
> > > 
> > >  X64 Exception Type - 0E(#PF - Page-Fault)  CPU Apic ID -
> > >  
> > > RIP  - 0F5F8918, CS  - 0028, RFLAGS -
> > > 00210206
> > > ExceptionData - 0011
> > > RAX  - , RCX - 07FCE000, RDX -
> > > 
> > > RBX  - 0B6092C0, RSP - 0F5F8590, RBP -
> > > 0B608EA0
> > > RSI  - 0F5F8838, RDI - 0B608EA0
> > > R8   - , R9  - 0B609200, R10 -
> > > 
> > > R11  - 000A, R12 - , R13 -
> > > 001B
> > > R14  - 0B609360, R15 - 
> > > DS   - 0008, ES  - 0008, FS  -
> > > 0008
> > > GS   - 0008, SS  - 0008
> > > CR0  - 8033, CR2 - 0F5F8918, CR3 -
> > > 0F597000
> > > CR4  - 0668, CR8 - 
> > > DR0  - , DR1 - , DR2 -
> > > 
> > > DR3  - , DR6 - 0FF0, DR7 -
> > > 0400
> > > GDTR - 0F57BF18 003F, LDTR - 
> > > IDTR - 0EEA5018 0FFF,   TR - 
> > > FXSAVE_STATE - 0F5F81F0
> > >  Find PE image 
> > /build/xen-unstable/src/xen-unstable/tools/firmware/ovmf-dir
> > -remote/Build
> > /OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/R
> > untime
> > Dxe/StatusCodeRuntimeDxe/DEBUG/StatusCodeRuntimeDxe.dll 
> > (ImageBase=0F556000, EntryPoint=0F55628F) 
> > > 
> > > I did check with other guest (Windows, Ubuntu, Debian Jessie), and
> > > they are
> > > working correctly. Debian Wheezy is the only one that fail.
> > 
> > I don't have an environment to reproduce this in. I think we should try
> > to understand this problem better, before deciding how to make it go
> > away.
> > 
> > Please locate the "StatusCodeRuntimeDxe.debug" file in your Build
> > directory (ie. under the location listed in the error report). Then,
> > please disassemble it with "objdump -S". The fault location in the
> > disassembly can be found based on RIP, ImageBase and EntryPoint;
> 
> I don't think the exact instruction at that address really matters. The
> main question appears to be why RIP and RSP both point into the
> same page (see also the subject of Anthony's mail).

I'm not 100% what is going on, but if this (executable code on stack) is
happening in grub is there something which is explicitly forbidden to UEFI
apps by the UEFI spec?

Or is it happening within UEFI itself based on a call from grub.efi?


>  I.e. we need to
> spot the entity setting the stack to a page that also contains code,
> or placing code on the stack. That's unlikely to be found by identifying
> the instruction RIP points to, but rather (sadly not part of the state
> dump) something higher up the call chain.
> 
> Jan
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

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

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [xen-unstable test] 61521: regressions - 
FAIL"):
> On Tue, 2015-09-08 at 17:13 +0100, Ian Jackson wrote:
> > I have looked a the histories of various `debianhvm' tests in many
> > other osstest `branches' and there do seem to be occasional similar
> > failures elsewhere.  So Ian C wasn't right to remember that this test
> 
> was ???

Err, yes, "was", sorry!

> >From the original report:
> > version targeted for testing:
> >  xen  a7b39c8bd6cba3fe1c8012987b9e28bdbac7e92d
> > baseline version:
> >  xen  801ab48e5556cb54f67e3cb57f077f47e8663ced
> 
> Therefore I have force pushed a7b39c8bd6cba3fe1c8012987b9e28bdbac7e92d.

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH v4 10/11] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Wang, Wei W]

On 09/09/2015 18:10,  Jan Beulich wrote:
>>> On 09.09.15 at 11:35,  wrote:
>> Using the drinking vessel analogy, we are not putting milk and water 
>> into the vessel at the same time. If the producer puts water into the 
>> vessel, then the consumer simply consumes water; If the producer puts 
>> milk into the vessel, then the consumer simply consumes milk. I think 
>> we don't need to worry about what type of drinking is put inside the 
>> vessel, because the vessel is just an intermediate place holding the 
>>liquid between the producer and consumer - the consumer has the 
>> privity of contract with the producer and it has the right logic to deal 
>> with what's inside the vessel.

> This analogy breaks for a blind: How would he know whether there's water or 
> milk in there? He'd have to try it. 
>Now what if we use >venom instead of milk in your analogy?

>Bottom line - the consumer needs to know what kind of data it is to expect to 
>consume.

There is a contract between the consumer and the producer. In our case, the 
contract is "p_cpufreq->scaling_driver". Before the right consumer consumes the 
value of " uint32_t scaling_max_perf ", it goes through the check:
 +if (!strncmp(p_cpufreq->scaling_driver,
 +  "intel_pstate", CPUFREQ_NAME_LEN) )
, where "intel_pstate" can be changed to other new driver names (contract) in 
the future.

Best,
Wei

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

Re: [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Laszlo Ersek]

On 09/09/15 11:37, Ian Campbell wrote:
> On Wed, 2015-09-09 at 01:06 -0600, Jan Beulich wrote:
> On 09.09.15 at 00:23,  wrote:
>>> On 09/08/15 19:26, Anthony PERARD wrote:
 And I get this on the console:
 Welcome to GRUB!

  X64 Exception Type - 0E(#PF - Page-Fault)  CPU Apic ID -
  
 RIP  - 0F5F8918, CS  - 0028, RFLAGS -
 00210206
 ExceptionData - 0011
 RAX  - , RCX - 07FCE000, RDX -
 
 RBX  - 0B6092C0, RSP - 0F5F8590, RBP -
 0B608EA0
 RSI  - 0F5F8838, RDI - 0B608EA0
 R8   - , R9  - 0B609200, R10 -
 
 R11  - 000A, R12 - , R13 -
 001B
 R14  - 0B609360, R15 - 
 DS   - 0008, ES  - 0008, FS  -
 0008
 GS   - 0008, SS  - 0008
 CR0  - 8033, CR2 - 0F5F8918, CR3 -
 0F597000
 CR4  - 0668, CR8 - 
 DR0  - , DR1 - , DR2 -
 
 DR3  - , DR6 - 0FF0, DR7 -
 0400
 GDTR - 0F57BF18 003F, LDTR - 
 IDTR - 0EEA5018 0FFF,   TR - 
 FXSAVE_STATE - 0F5F81F0
  Find PE image 
>>> /build/xen-unstable/src/xen-unstable/tools/firmware/ovmf-dir
>>> -remote/Build
>>> /OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/R
>>> untime
>>> Dxe/StatusCodeRuntimeDxe/DEBUG/StatusCodeRuntimeDxe.dll 
>>> (ImageBase=0F556000, EntryPoint=0F55628F) 

 I did check with other guest (Windows, Ubuntu, Debian Jessie), and
 they are
 working correctly. Debian Wheezy is the only one that fail.
>>>
>>> I don't have an environment to reproduce this in. I think we should try
>>> to understand this problem better, before deciding how to make it go
>>> away.
>>>
>>> Please locate the "StatusCodeRuntimeDxe.debug" file in your Build
>>> directory (ie. under the location listed in the error report). Then,
>>> please disassemble it with "objdump -S". The fault location in the
>>> disassembly can be found based on RIP, ImageBase and EntryPoint;
>>
>> I don't think the exact instruction at that address really matters. The
>> main question appears to be why RIP and RSP both point into the
>> same page (see also the subject of Anthony's mail).
> 
> I'm not 100% what is going on,

me neither :)

> but if this (executable code on stack) is
> happening in grub is there something which is explicitly forbidden to UEFI
> apps by the UEFI spec?

Yes, there is. This small OvmfPkg patch only enables the edk2 feature
added by Star Zeng in
 for OVMF. That patch
(also referenced in my commit message by SVN rev) says,

This feature is added for UEFI spec that says
"Stack may be marked as non-executable in identity mapped page
tables".
A PCD PcdSetNxForStack is added to turn on/off this feature, and it
is FALSE by default.

A UEFI app runs (well, *starts*, anyway) before ExitBootServices() /
SetVirtualAddressMap(), so it's bound by the above.

The spec passage above is quoted from "2.3.2 IA-32 Platforms", and
"2.3.4 x64 Platforms", in chapter "2.3 Calling Conventions", where the
boot services time environment is specified.

This is new in UEFI-2.5, and it comes from Mantis ticket 1224: "Adding
support for No executable data areas".

... The question could be then if grub (in Wheezy) should be adapted to
UEFI-2.5 (if that's possible) or if OVMF should be built without this
feature.

Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.

Namely, Mantis ticket 1224 has come up before. There's another edk2
sub-feature related to this UEFI spec feature / Mantis ticket; the
properties table (controlled by "PcdPropertiesTableEnable"), and the
effects it has on the UEFI memory map, and the requirements it presents
for UEFI OSes.

*That* sub-feature is extremely intrusive.
"MdeModulePkg/MdeModulePkg.dec" sets "PcdPropertiesTableEnable" TRUE by
default, and OvmfPkg inherits it. I have not overridden that default
just yet in OvmfPkg because the properties table feature depends on
something *else* too: sections in runtime DXE driver binaries must be
aligned at 4KB, and that is not ensured for OvmfPkg (yet?). Therefore,
the properties table feature is not active in OVMF at the moment due to
a "random build tools circumstance" -- and not because the feature flag
is actually disabled.

Now, the properties table feature absolutely cannot be (effectively)
enabled in OVMF as a default. It would break Windows 7 / Windows Server
2008 R2. A huge number of users run such guests for GPU passthrough.

The idea 

Re: [Xen-devel] [PATCH] x86/hvm: fix saved pmtimer value

[Wei Liu]

On Wed, Sep 09, 2015 at 01:46:35AM -0600, Jan Beulich wrote:
> >>> On 09.09.15 at 09:31,  wrote:
> > The ACPI PM timer is sometimes broken on live migration.
> > Since vcpu->arch.hvm_vcpu.guest_time is zero in most cases.
> 
> I.e. in other than "delay for missed ticks mode". Would have been
> nice if you had spelled this out explicitly. With that the question
> then is - should the field perhaps be used when non-zero, and the
> function only be called otherwise?
> 
> Wei - regardless of the answer to the above (i.e. regardless of
> whether a v2 is going to be needed) I think we want this in 4.6.
> 

Yes. I think so. I will leave the technical side to you and Andy.

Release-acked-by: Wei Liu 

> Jan

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

Re: [Xen-devel] [PATCH net v2 2/2] xen-netfront: respect user provided max_queues

[David Vrabel]

On 09/09/15 11:23, Wei Liu wrote:
> Originally that parameter was always reset to num_online_cpus during
> module initialisation, which renders it useless.
> 
> The fix is to only set max_queues to num_online_cpus when user has not
> provided a value.

Reviewed-by: David Vrabel 

Thanks.

David

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

Re: [Xen-devel] [PATCH net] xen-netback: respect user provided max_queues

[Ian Campbell]

On Wed, 2015-09-09 at 11:16 +0100, Wei Liu wrote:
> On Wed, Sep 09, 2015 at 11:12:44AM +0100, David Vrabel wrote:
> > On 09/09/15 11:09, Wei Liu wrote:
> > > Originally that parameter was always reset to num_online_cpus during
> > > module initialisation, which renders it useless.
> > > 
> > > The fix is to only set max_queues to num_online_cpus when user has
> > > not
> > > provided a value.
> > [...]
> > > --- a/drivers/net/xen-netback/netback.c
> > > +++ b/drivers/net/xen-netback/netback.c
> > > @@ -67,7 +67,7 @@ module_param(rx_drain_timeout_msecs, uint, 0444);
> > >  unsigned int rx_stall_timeout_msecs = 6;
> > >  module_param(rx_stall_timeout_msecs, uint, 0444);
> > >  
> > > -unsigned int xenvif_max_queues;
> > > +unsigned int xenvif_max_queues = 0;
> > 
> > You don't need this.
> > 
> > Otherwise,
> > 
> > Reviewed-by: David Vrabel 
> > 
> > Is an equivalent fix needed in netfront?
> > 
> 
> I think so.
> 
> I will address your comment

With that: Acked-by: Ian Campbell 



>  and send both fixes (front and back) in a
> series.
> 
> Wei.
> 
> > David
> > 
> > >  module_param_named(max_queues, xenvif_max_queues, uint, 0644);
> > >  MODULE_PARM_DESC(max_queues,
> > >"Maximum number of queues per virtual interface");
> > > @@ -2105,8 +2105,11 @@ static int __init netback_init(void)
> > >   if (!xen_domain())
> > >   return -ENODEV;
> > >  
> > > - /* Allow as many queues as there are CPUs, by default */
> > > - xenvif_max_queues = num_online_cpus();
> > > + /* Allow as many queues as there are CPUs if user has not
> > > +  * specified a value.
> > > +  */
> > > + if (xenvif_max_queues == 0)
> > > + xenvif_max_queues = num_online_cpus();
> > >  
> > >   if (fatal_skb_slots < XEN_NETBK_LEGACY_SLOTS_MAX) {
> > >   pr_info("fatal_skb_slots too small (%d), bump it to
> > > XEN_NETBK_LEGACY_SLOTS_MAX (%d)\n",
> > > 

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

Re: [Xen-devel] [PATCHv1 net] xen-netback: require fewer guest Rx slots when not using GSO

[Wei Liu]

On Tue, Sep 08, 2015 at 02:25:14PM +0100, David Vrabel wrote:
> Commit f48da8b14d04ca87ffcffe68829afd45f926ec6a (xen-netback: fix
> unlimited guest Rx internal queue and carrier flapping) introduced a
> regression.
> 
> The PV frontend in IPXE only places 4 requests on the guest Rx ring.
> Since netback required at least (MAX_SKB_FRAGS + 1) slots, IPXE could
> not receive any packets.
> 
> a) If GSO is not enabled on the VIF, fewer guest Rx slots are required
>for the largest possible packet.  Calculate the required slots
>based on the maximum GSO size or the MTU.
> 
>This calculation of the number of required slots relies on
>1650d5455bd2 (xen-netback: always fully coalesce guest Rx packets)
>which present in 4.0-rc1 and later.
> 
> b) Reduce the Rx stall detection to checking for at least one
>available Rx request.  This is fine since we're predominately
>concerned with detecting interfaces which are down and thus have
>zero available Rx requests.
> 
> Signed-off-by: David Vrabel 

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Ian Campbell]

On Wed, 2015-09-09 at 12:48 +0200, Laszlo Ersek wrote:

Thanks for all the info, I think I get it (although its not clear to me
whether how an app can claim to be UEFI 2.5 capable and what the transition
plan for legacy applications was going to be).

> ... The question could be then if grub (in Wheezy) should be adapted to
> UEFI-2.5 (if that's possible)

I don't know either I'm afraid.

[...]
> Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.

I have a question: What attack vector is setting the stack as Nx in OVMF
(or even UEFI generally) trying to protect against? Or is this being done
for a reason other than security?

I understand why it is done for kernels and apps, but where does the
untrusted element which is being protected against come from when running
UEFI?

Ian.



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

Re: [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Laszlo Ersek]

On 09/09/15 13:07, Ian Campbell wrote:
> On Wed, 2015-09-09 at 12:48 +0200, Laszlo Ersek wrote:
> 
> Thanks for all the info, I think I get it (although its not clear to me
> whether how an app can claim to be UEFI 2.5 capable and what the transition
> plan for legacy applications was going to be).
> 
>> ... The question could be then if grub (in Wheezy) should be adapted to
>> UEFI-2.5 (if that's possible)
> 
> I don't know either I'm afraid.
> 
> [...]
>> Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.
> 
> I have a question: What attack vector is setting the stack as Nx in OVMF
> (or even UEFI generally) trying to protect against? Or is this being done
> for a reason other than security?

I think it is being done for security, generally speaking (ie. as far as
edk2 and the UEFI spec are concerned).

Specifically for OVMF, I wrote the patch because Paolo suggested it,
after (I think) he saw Star's feature patch on the edk2 mailing list. I
thought it was a good suggestion, even in case we had no particular
attack in mind: OVMF is frequently used as a UEFI development / test
bed, and we try to enable new edk2 / UEFI features in it, unless the
firmware size increase would be large, or something would break. (In
which cases we usually bracket the new feature with build time flags.)

I did consider regressions while writing this patch. In the commit
message I mentioned "-cpu ,-nx" as a way to turn of NX support in
the virtual CPU (which the edk2 feature dynamically detects and depends
on), should anything break. I did not expect two things: (a) old grub
actually executes stuff from the stack, (b) Xen has no way (?) to
disable NX in a VCPU (or maybe that would be detrimental for other reasons).

> I understand why it is done for kernels and apps, but where does the
> untrusted element which is being protected against come from when running
> UEFI?

I'm sure this is explained in the five ECR documents attached to Mantis
ticket 1224:

https://mantis.uefi.org/mantis/view.php?id=1224

Unfortunately, I don't think I can just publish those ECRs here (or dump
the mantis ticket).

I have no clue why UEFI development is so secretive. I had been annoyed
by not seeing mantis tickets myself (and thereby missing the
justification behind new UEFI features), so at some point I applied for
"non-voter, observer-only" membership in both the PIWG and the USWG, and
I got them. (I'm fuzzy on the details if this was helped by my being
@redhat.com -- it probably was.)

Perhaps you can also apply for membership (and read the ECRs on the
ticket), or hopefully Star (who implemented the feature) could summarize
the purpose here?

(I won't try, based on the ECRs, because that will unavoidably put me in
the position where I have to explain and defend the feature. Thanks, but
no, thanks :))

Cheers
Laszlo

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

Re: [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Laszlo Ersek]

On 09/09/15 13:30, Paolo Bonzini wrote:
> 
> 
> On 09/09/2015 13:07, Ian Campbell wrote:
>> I have a question: What attack vector is setting the stack as Nx in OVMF
>> (or even UEFI generally) trying to protect against? Or is this being done
>> for a reason other than security?
>>
>> I understand why it is done for kernels and apps, but where does the
>> untrusted element which is being protected against come from when running
>> UEFI?
> 
> I guess something could attack shim.efi or GRUB, and subvert secure
> boot's chain of trust.

... or the firmware could be fed some malicious data over the network,
when the fw (e.g. shim) boots off PXE (or, in case of edk2, HTTP), and a
buffer overflow could lead to the execution of arbitrary code?...



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

Re: [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Laszlo Ersek]

On 09/09/15 09:06, Jan Beulich wrote:
 On 09.09.15 at 00:23,  wrote:
>> On 09/08/15 19:26, Anthony PERARD wrote:
>>> And I get this on the console:
>>> Welcome to GRUB!
>>>
>>>  X64 Exception Type - 0E(#PF - Page-Fault)  CPU Apic ID -  
>>> RIP  - 0F5F8918, CS  - 0028, RFLAGS - 00210206
>>> ExceptionData - 0011
>>> RAX  - , RCX - 07FCE000, RDX - 
>>> RBX  - 0B6092C0, RSP - 0F5F8590, RBP - 0B608EA0
>>> RSI  - 0F5F8838, RDI - 0B608EA0
>>> R8   - , R9  - 0B609200, R10 - 
>>> R11  - 000A, R12 - , R13 - 001B
>>> R14  - 0B609360, R15 - 
>>> DS   - 0008, ES  - 0008, FS  - 0008
>>> GS   - 0008, SS  - 0008
>>> CR0  - 8033, CR2 - 0F5F8918, CR3 - 0F597000
>>> CR4  - 0668, CR8 - 
>>> DR0  - , DR1 - , DR2 - 
>>> DR3  - , DR6 - 0FF0, DR7 - 0400
>>> GDTR - 0F57BF18 003F, LDTR - 
>>> IDTR - 0EEA5018 0FFF,   TR - 
>>> FXSAVE_STATE - 0F5F81F0
>>>  Find PE image 
>> /build/xen-unstable/src/xen-unstable/tools/firmware/ovmf-dir-remote/Build
>> /OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/Runtime
>> Dxe/StatusCodeRuntimeDxe/DEBUG/StatusCodeRuntimeDxe.dll 
>> (ImageBase=0F556000, EntryPoint=0F55628F) 
>>>
>>> I did check with other guest (Windows, Ubuntu, Debian Jessie), and they are
>>> working correctly. Debian Wheezy is the only one that fail.
>>
>> I don't have an environment to reproduce this in. I think we should try
>> to understand this problem better, before deciding how to make it go away.
>>
>> Please locate the "StatusCodeRuntimeDxe.debug" file in your Build
>> directory (ie. under the location listed in the error report). Then,
>> please disassemble it with "objdump -S". The fault location in the
>> disassembly can be found based on RIP, ImageBase and EntryPoint;
> 
> I don't think the exact instruction at that address really matters. The
> main question appears to be why RIP and RSP both point into the
> same page (see also the subject of Anthony's mail). I.e. we need to
> spot the entity setting the stack to a page that also contains code,
> or placing code on the stack.

Good point!

(... FWIW, I've had luck on several occasions in the past deducing the
the origin of the data from the data itself. So if we can see the "code
on the stack", maybe that could help us figure out where it comes from.
Just an idea; might not apply very well here.)

> That's unlikely to be found by identifying
> the instruction RIP points to, but rather (sadly not part of the state
> dump) something higher up the call chain.

As far as I can see, Debian switched from grub2 v1.99 to v2.02, from
Wheezy to Jessie. Based on the grub2 commit history, quite a few things
happened in grub2 in that timeframe. Should we perhaps ask the grub2
developers?

Thanks
Laszlo

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

Re: [Xen-devel] [PATCH v4 10/11] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Wang, Wei W]

On 09/09/2015 17:02,  Jan Beulich wrote:
>>> On 09.09.15 at 10:49,  wrote:
> On 09/09/2015 16:32,  Jan Beulich wrote:
 On 09.09.15 at 10:11,  wrote:
>> On 24/07/2015 22:16,  Jan Beulich wrote:
> On 25.06.15 at 13:17,  wrote:
>>> --- a/xen/drivers/acpi/pmstat.c
>>> +++ b/xen/drivers/acpi/pmstat.c
>>> --- a/xen/include/public/sysctl.h
>>> +++ b/xen/include/public/sysctl.h
>>> @@ -315,8 +315,18 @@ struct xen_get_cpufreq_para {
>>>  uint32_t scaling_cur_freq;
>>>  
>>>  char scaling_governor[CPUFREQ_NAME_LEN];
>>> -uint32_t scaling_max_freq;
>>> -uint32_t scaling_min_freq;
>>> +
>>> +union {
>>> +uint32_t freq;
>>> +uint32_t pct;
>>> +} scaling_max;
>>> +
>>> +union {
>>> +uint32_t freq;
>>> +uint32_t pct;
>>> +} scaling_min;
>> 
>>>scaling_min and scaling_max should really be of the same type, so 
>>>that
>> someone wanting to introduce helper functions
>>>or pointers to them can hand both interchangeably.
>> 
>>>Also I'm starting to get tired of repeating that it is still unclear 
>>>how a
>> consumer of the structure will know which of the
>>>two fields of the unions are applicable.
>> 
>>> Probably we don't need a union here. I plan to simply change them to 
>>> uint32_t scaling_max_perf; uint32_t scaling_max_perf;
>> 
>>> Then it's up to the driver to put what kind of value to it. It's 
>>> like we simply provide a drinking vessel, and it depends on the user 
>>> to put water or milk into it. In our case, the intel_pstate driver 
>>> assigns a percentage vale to it (in the "uint32_t" type), and the 
>>> legacy driver assigns the absolute value to it (in the "uint32_t" type, 
>>> too).
> 
>>I don't see how this will solve the problem of the consumer not 
>>knowing what
> kind of value it has to deal with.
> 
> The consumer is inside the print_cpufreq_para() function. I have put 
> the code below:

>No, this is not "the" consumer but just one out of potentially many.

> +if (!strncmp(p_cpufreq->scaling_driver,
> +  "intel_pstate", CPUFREQ_NAME_LEN) )

>And this is not really a proper way to distinguish which of an output 
>structure's sub-union is to be used. Just consider what happens 
>to the code when we end up gaining a few more drivers providing percentage 
>values, and perhaps another one providing a third
 >variant of output representation.

Only one driver is in use at one time. I am considering not using the union 
variable any more. We can simply use "uint32_t scaling_max_perf". No matter 
what kind of a third variant of output representation it may be, as long as it 
is in "uint32_t", it can use "uint32_t scaling_max_perf" to hold that value 
representation. The top producer of this " uint32_t scaling_max_perf " is a 
user command input via xenpm, then the related consumer has its own logic to 
deal with what's being put into " uint32_t scaling_max_perf ".

Using the drinking vessel analogy, we are not putting milk and water into the 
vessel at the same time. If the producer puts water into the vessel, then the 
consumer simply consumes water; If the producer puts milk into the vessel, then 
the consumer simply consumes milk. I think we don't need to worry about what 
type of drinking is put inside the vessel, because the vessel is just an 
intermediate place holding the liquid between the producer and consumer - the 
consumer has the privity of contract with the producer and it has the right 
logic to deal with what's inside the vessel.

Hope this is sensible.

Best,
Wei

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

Re: [Xen-devel] [Xen-users] Xen bridging issue.

[johnny Strom]

On 09/09/2015 12:35 PM, Wei Liu wrote:

On Wed, Sep 09, 2015 at 10:09:09AM +0300, johnny Strom wrote:
[...]

Hello again

I am not sure but there might be another issue with the xen_netback module
in Debian jessie.

I am not able to set the  max_queues options so that it is set at load time.

I have tested with the loop kernel module where it works to set an value but
doing the same for
the xen_netback driver dose not work:

This works with loop module like this:
options loop max_loop=50

And then doing the same with the xen_netback that dose not work:
options xen_netback max_queues=4

Or is there some other way it should be set in
/etc/modprobe.d/xen_netback.conf?


Best regards Johnny


After looking at the code more carefully I think that's a bug.

I will send a patch to fix it when I get around to do it. In the mean
time (till the bug fix is propagated to Debian, that probably takes
quite a bit of time), you can use a script to echo the value you want to
the control knob during system startup.

I will put a Reported-by tag with your email address in my patch if you
don't mind. I will also CC you on that patch I'm going to send so that
you have an idea when it's merged upstreamed.


Thanks

It's ok to put my email in the patch.

Best regards Johnny


Wei.



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

Re: [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Jan Beulich]

>>> On 09.09.15 at 11:37,  wrote:
> I'm not 100% what is going on, but if this (executable code on stack) is
> happening in grub is there something which is explicitly forbidden to UEFI
> apps by the UEFI spec?

Whether it's spelled out explicitly I don't know, but the separation
of memory types (*Code vs *Data) is clearly with the intention to
limit permissions. Hence an entity allocating *Data should not place
code there (as much as an entity allocating *Code shouldn't expect
to be able to write to that area, which kind of implies that such
allocations aren't useful from outside of UEFI, since then you have
no way to fill in the code you mean to execute).

> Or is it happening within UEFI itself based on a call from grub.efi?

That's still unclear at this point.

Jan


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

Re: [Xen-devel] [PATCH net] xen-netback: respect user provided max_queues

[David Vrabel]

On 09/09/15 11:09, Wei Liu wrote:
> Originally that parameter was always reset to num_online_cpus during
> module initialisation, which renders it useless.
> 
> The fix is to only set max_queues to num_online_cpus when user has not
> provided a value.
[...]
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -67,7 +67,7 @@ module_param(rx_drain_timeout_msecs, uint, 0444);
>  unsigned int rx_stall_timeout_msecs = 6;
>  module_param(rx_stall_timeout_msecs, uint, 0444);
>  
> -unsigned int xenvif_max_queues;
> +unsigned int xenvif_max_queues = 0;

You don't need this.

Otherwise,

Reviewed-by: David Vrabel 

Is an equivalent fix needed in netfront?

David

>  module_param_named(max_queues, xenvif_max_queues, uint, 0644);
>  MODULE_PARM_DESC(max_queues,
>"Maximum number of queues per virtual interface");
> @@ -2105,8 +2105,11 @@ static int __init netback_init(void)
>   if (!xen_domain())
>   return -ENODEV;
>  
> - /* Allow as many queues as there are CPUs, by default */
> - xenvif_max_queues = num_online_cpus();
> + /* Allow as many queues as there are CPUs if user has not
> +  * specified a value.
> +  */
> + if (xenvif_max_queues == 0)
> + xenvif_max_queues = num_online_cpus();
>  
>   if (fatal_skb_slots < XEN_NETBK_LEGACY_SLOTS_MAX) {
>   pr_info("fatal_skb_slots too small (%d), bump it to 
> XEN_NETBK_LEGACY_SLOTS_MAX (%d)\n",
> 


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

Re: [Xen-devel] [PATCH v6 16/18] vmx: Add some scheduler hooks for VT-d posted interrupts

[Jan Beulich]

>>> On 09.09.15 at 10:56,  wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: Monday, September 07, 2015 8:55 PM
>> >>> On 25.08.15 at 03:57,  wrote:
>> > @@ -1605,9 +1621,12 @@ void context_switch(struct vcpu *prev, struct
>> vcpu *next)
>> >
>> >  set_current(next);
>> >
>> > +pi_ctxt_switch_from(prev);
>> > +
>> >  if ( (per_cpu(curr_vcpu, cpu) == next) ||
>> >   (is_idle_domain(nextd) && cpu_online(cpu)) )
>> >  {
>> > +pi_ctxt_switch_to(next);
>> >  local_irq_enable();
>> 
>> This placement, if really intended that way, needs explanation (in a
>> comment) and perhaps even renaming of the involved symbols, as
> 
> I think maybe removing the static wrapper function can make thing
> clearer. What is your opinion?

Considering that there's going to be a second use of the
switch-to variant, I think the helpers are okay to be kept (but
I wouldn't insist on that), just that they need to be named in
a away making clear what their purpose is.

>> > @@ -117,10 +119,20 @@ static int vmx_vcpu_initialise(struct vcpu *v)
>> >  INIT_LIST_HEAD(>arch.hvm_vmx.pi_blocked_vcpu_list);
>> >  INIT_LIST_HEAD(>arch.hvm_vmx.pi_vcpu_on_set_list);
>> >
>> > +v->arch.hvm_vmx.pi_block_cpu = -1;
>> > +
>> > +spin_lock_init(>arch.hvm_vmx.pi_lock);
>> > +
>> >  v->arch.schedule_tail= vmx_do_resume;
>> >  v->arch.ctxt_switch_from = vmx_ctxt_switch_from;
>> >  v->arch.ctxt_switch_to   = vmx_ctxt_switch_to;
>> >
>> > +if ( iommu_intpost && is_hvm_vcpu(v) )
>> > +{
>> > +v->arch.pi_ctxt_switch_from = vmx_pre_ctx_switch_pi;
>> > +v->arch.pi_ctxt_switch_to = vmx_post_ctx_switch_pi;
>> > +}
>> 
>> Why conditional upon is_hvm_vcpu()?
> 
> I only think we need to add these hooks for PV guests, right?

"... we don't need to ..."?

> Or you mean I should use has_hvm_container_vcpu() instead?

Exactly.

>> > +
>> > +/*
>> > + * Delete the vCPU from the related block list
>> > + * if we are resuming from blocked state.
>> > + */
>> > +if ( v->arch.hvm_vmx.pi_block_cpu != -1 )
>> > +{
>> > +spin_lock_irqsave(_cpu(pi_blocked_vcpu_lock,
>> > +  v->arch.hvm_vmx.pi_block_cpu),
>> flags);
>> > +list_del_init(>arch.hvm_vmx.pi_blocked_vcpu_list);
>> 
>> Shouldn't you set .pi_block_cpu back to -1 along with removing
>> the vCPU from the list? Which then ought to allow using just
>> list_del() here?
> 
> Here is the story about using list_del_init() instead of list_del(), (the
> same reason for using list_del_init() in pi_wakeup_interrupt() ).
> If we use list_del(), that means we need to set .pi_block_cpu to
> -1 after removing the vCPU from the block list, there are two
> places where the vCPU is removed from the list:
> - arch_vcpu_wake_prepare()
> - pi_wakeup_interrupt()
> 
> That means we also need to set .pi_block_cpu to -1 in
> pi_wakeup_interrupt(), which seems problematic to me, since
> .pi_block_cpu is used to get the per-CPU spin lock, it is not safe
> to change it in pi_wakeup_interrupt(), maybe someone is using
> it at the same time.
> 
> And the check "if ( v->arch.hvm_vmx.pi_block_cpu != -1 )" here
> is only used when .pi_block_cpu is set to -1 at the initial time.
> 
> If you have any good suggestions about this, I will be all ears.

Latching pi_block_cpu into a local variable would take care of that
part of the problem. Of course you then may also need to check
that while waiting to acquire the lock pi_block_cpu didn't change.
And if that absolutely can't be made work correctly, these
apparently strange uses of list_del_init() would each need a
justifying comment.

>> > +do {
>> > +old.control = new.control = pi_desc->control;
>> > +
>> > +/* Should not block the vCPU if an interrupt was posted for 
>> > it.
>> */
>> > +if ( pi_test_on() )
>> > +{
>> > +spin_unlock_irqrestore(>arch.hvm_vmx.pi_lock,
>> gflags);
>> > +vcpu_unblock(v);
>> 
>> Calling vcpu_unblock() in the middle of context_switch()? Why? And
>> is this safe? 
> 
> I cannot see anything unsafe so far, can some scheduler maintainer
> help to confirm it? Dario? George?

But first and foremost you ought to answer the "why".

>> And if really needed to cover something not dealt with
>> elsewhere, wouldn't this need to happen _after_ having switched
>> the notification mechanism below?
> 
> If ON is set, we don't need to block the vCPU hence no need to change the
> notification vector to the wakeup one, which is used when vCPU is blocked.
> 
>> 
>> > +return;

Oh, right, I somehow managed to ignore the "return" here.

>> > +static void vmx_post_ctx_switch_pi(struct vcpu *v)
>> > +{
>> > +struct pi_desc *pi_desc = >arch.hvm_vmx.pi_desc;
>> > +
>> > +if ( !has_arch_pdevs(v->domain) )
>> > +return;
>> > +
>> > +if ( 

Re: [Xen-devel] [Patch V1 2/3] xen/usb: add capability for passing through isoc jobs to host devices

[Juergen Gross]

On 09/04/2015 03:25 PM, Gerd Hoffmann wrote:

On Do, 2015-09-03 at 12:45 +0200, Juergen Gross wrote:

When Xen is using the qemu usb framework for pure passthrough of I/Os
to host devices the handling of isoc jobs is rather complicated if
multiple isoc frames are transferred with one call.

Instead of calling the framework with each frame individually, using
timers to avoid polling in a loop and sampling all responses to
construct a sum response for the user, just add a capability to
use the libusb isoc framework instead. This capability is selected
via a device specific property.

When the property is selected the host usb driver will use xen specific
callbacks to signal the end of isoc I/Os. For now these callbacks will
just be nops, they'll be filled with sensible actions when the xen
pv-usb backend is being added.


So you basically add support for async isoc requests.  Fine.

There is nothing xen specific in this though, except that xen is (so
far) the only user.  It isn't going to work for uhci and ehci, put
possibly xhci can join the party.

So, the signaling needs to be different.  The host adapter needs to
signal somehow that it can handle async iso packets.  One way would be
to flag this per usb bus, another one per usb packet.  Also all xen
naming and the xen inlude should go away.  BTW: does this build without
xen-devel installed?


Okay, I'll try to make it more generic. I think the async iso capability
should be a bus attribute.


Can we get rid of the callbacks?  By filling the USBPacket iovec with
the iso request chunks for example?


Difficult. One iso request chunk could require multiple iovec entries.

The RFC version tried to avoid the callbacks and there you didn't like
exposing the additional structures. I could, of course, try to combine
both variants by using the device property to enable async isoc and
add generic callbacks with some new structures to replace the xen/libusb
specific variants. This would still leave USBPacket unmodified and all
hcds capable of supporting the new feature could join.

What do you think?


Juergen

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

[Xen-devel] [xen-4.4-testing test] 61599: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qcow2  9 debian-di-install fail REGR. vs. 60727
 test-amd64-i386-xl-raw9 debian-di-install fail REGR. vs. 60727

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail pass in 
61512

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-libvirt-vhd   9 debian-di-install fail REGR. vs. 60727
 test-amd64-amd64-libvirt-raw  9 debian-di-install fail REGR. vs. 60727
 test-amd64-amd64-libvirt-vhd  9 debian-di-install fail REGR. vs. 60727
 test-armhf-armhf-xl-multivcpu 16 guest-start/debian.repeatfail  like 60696
 test-amd64-i386-libvirt-qcow2  9 debian-di-installfail  like 60727
 test-amd64-i386-xl-vhd9 debian-di-installfail   like 60727
 test-amd64-amd64-xl-vhd   9 debian-di-installfail   like 60727
 test-amd64-i386-libvirt  11 guest-start  fail   like 60727
 test-amd64-amd64-libvirt 11 guest-start  fail   like 60727
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail like 60727

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-amd64-migrupgrade  1 build-check(1)   blocked  n/a
 test-amd64-i386-migrupgrade   1 build-check(1)   blocked  n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail in 61512 never pass
 test-armhf-armhf-xl-qcow2 9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 build-amd64-rumpuserxen   6 xen-buildfail   never pass
 test-armhf-armhf-xl-raw   9 debian-di-installfail   never pass
 build-i386-rumpuserxen6 xen-buildfail   never pass
 build-amd64-prev  5 xen-buildfail   never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 build-i386-prev   5 xen-buildfail   never pass
 test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail never pass
 test-amd64-amd64-xl-qcow2 9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt 11 guest-start  fail   never pass
 test-amd64-amd64-libvirt-pair 21 guest-migrate/src_host/dst_host fail never 
pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-amd64-i386-libvirt-raw  11 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-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xend-qemut-winxpsp3 21 leak-check/checkfail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass

version targeted for testing:
 xen  339f5743e84a28dd01ffa7498372e410301cd0b4
baseline version:
 xen  3646b134c1673f09c0a239de10b0da4c9265c8e8

Last test of basis60727  2015-08-16 16:15:09 Z   23 days
Failing since 60802  2015-08-20 14:41:37 Z   19 days9 attempts
Testing same since61512  2015-09-07 11:42:03 Z1 days2 attempts


People who touched revisions under test:
  Ian Campbell 
  Jan Beulich 
  Julien Grall 

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

[Xen-devel] linux-3.4 broken on chardonnay and huxelrebe (Re: [linux-3.4 test] 61301: regressions - FAIL)

[Ian Campbell]

On Fri, 2015-09-04 at 08:58 +, osstest service owner wrote:
> flight 61301 linux-3.4 real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/61301/
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-amd64-amd64-xl-qemut-win7-amd64  6 xen-boot  fail REGR. vs. 
> 30511

linux-3.4 seems to have trouble booting on chardonnay and huxelrebe, and
has accumulated _most_ of its jobs in persistent failures on one or the
other of those hosts[1]

Skimming a couple of logs it seems the systems fail for different reasons.

chardonanys are failing to find their root disk while the huxelrebes appear
to boot ok but not get any networking.

Of the chardonnay failures
http://logs.test-lab.xenproject.org/osstest/results/history/test-amd64-amd64-xl-qemut-win7-amd64/linux-3.4.html
is interesting. All the others show a history of passing on other machines
and then eventually hitting chardonnay and beginning to fail, while 
test-amd64-amd64-xl-qemut-win7-amd64 was booting on chardonnay and then at
some point started failing reliably (with one random looking boot early
on). This might provide sufficient points in the sand to try and bisection.

For the huxelrebes no such luck, I only looked at the first dozen failures
but they all have either always run (and failed) on huxelrebe or they
started failing as soon as they were scheduled on huxelrebe.

However taking test-amd64-amd64-xl-xsm it seems to have passed on huxelrebe
on linux-3.10, which gives us _something_ to go on.

So, I think I can try and convince the bisector to have another go at the
point in time where chardonnay started failing 
test-amd64-amd64-xl-qemut-win7-amd64.

For huxelrebe I can try running some adhoc tests at various points between
3.4 and 3.10 (initially 3.{5,6,7,8,9}) and then the -rcs around the change,
which might point to a region fix. If not then what I really want to do is
an inverted bisect to find the point at which it was fixed...

Ian.

[1]
> select steps.job,steps.testid,steps.status,runvars.val from steps,runvars 
> where steps.flight=runvars.flight and steps.job=runvars.job and 
> steps.flight='61301' and steps.testid = 'xen-boot' and runvars.name like 
> '%host' order by runvars.val;

  job  |  testid  | status |
 val 
---+--++-
 test-amd64-amd64-libvirt-qcow2| xen-boot | fail   | 
chardonnay0
 test-amd64-amd64-pygrub   | xen-boot | fail   | 
chardonnay0
 test-amd64-amd64-libvirt  | xen-boot | fail   | 
chardonnay0
 test-amd64-amd64-xl-qcow2 | xen-boot | fail   | 
chardonnay0
 test-amd64-amd64-xl-rtds  | xen-boot | fail   | 
chardonnay1
 test-amd64-amd64-xl-qemut-win7-amd64  | xen-boot | fail   | 
chardonnay1
 test-amd64-i386-libvirt-vhd   | xen-boot | pass   | 
elbling0
 test-amd64-i386-libvirt   | xen-boot | pass   | 
elbling0
 test-amd64-i386-xl-qcow2  | xen-boot | pass   | 
elbling0
 test-amd64-amd64-xl-qemuu-win7-amd64  | xen-boot | pass   | 
elbling1
 test-amd64-i386-xl-qemut-win7-amd64   | xen-boot | pass   | 
elbling1
 test-amd64-amd64-xl-vhd   | xen-boot | pass   | 
fiano0
 test-amd64-i386-libvirt-qcow2 | xen-boot | pass   | 
fiano0
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm | xen-boot | pass   | 
fiano1
 test-amd64-i386-xl-qemuu-win7-amd64   | xen-boot | pass   | 
fiano1
 test-amd64-amd64-xl-pvh-intel | xen-boot | pass   | 
fiano1
 test-amd64-amd64-libvirt-raw  | xen-boot | pass   | 
godello0
 test-amd64-amd64-libvirt-vhd  | xen-boot | pass   | 
godello1
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm | xen-boot | fail   | 
huxelrebe0
 test-amd64-amd64-xl-credit2   | xen-boot | fail   | 
huxelrebe0
 test-amd64-amd64-xl-qemut-winxpsp3| xen-boot | fail   | 
huxelrebe0
 test-amd64-amd64-xl   | xen-boot | fail   | 
huxelrebe0
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  | xen-boot | fail   | 
huxelrebe0
 test-amd64-amd64-xl-qemuu-ovmf-amd64  | xen-boot | fail   | 
huxelrebe0
 test-amd64-amd64-rumpuserxen-amd64| xen-boot | fail   | 
huxelrebe0
 test-amd64-i386-xl-qemut-winxpsp3 | xen-boot | fail   | 
huxelrebe0
 test-amd64-i386-rumpuserxen-i386  | xen-boot | fail   | 
huxelrebe0
 test-amd64-i386-xl-qemuu-winxpsp3 | xen-boot | fail   | 
huxelrebe0
 test-amd64-amd64-xl-xsm   | xen-boot | fail   | 

[Xen-devel] [PATCH net] xen-netback: respect user provided max_queues

[Wei Liu]

Originally that parameter was always reset to num_online_cpus during
module initialisation, which renders it useless.

The fix is to only set max_queues to num_online_cpus when user has not
provided a value.

Reported-by: Johnny Strom 
Signed-off-by: Wei Liu 
Cc: Ian Campbell 
---
Cc: Johnny Strom 
Cc: David Vrabel 
---
 drivers/net/xen-netback/netback.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/xen-netback/netback.c 
b/drivers/net/xen-netback/netback.c
index 42569b9..b219a80 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -67,7 +67,7 @@ module_param(rx_drain_timeout_msecs, uint, 0444);
 unsigned int rx_stall_timeout_msecs = 6;
 module_param(rx_stall_timeout_msecs, uint, 0444);
 
-unsigned int xenvif_max_queues;
+unsigned int xenvif_max_queues = 0;
 module_param_named(max_queues, xenvif_max_queues, uint, 0644);
 MODULE_PARM_DESC(max_queues,
 "Maximum number of queues per virtual interface");
@@ -2105,8 +2105,11 @@ static int __init netback_init(void)
if (!xen_domain())
return -ENODEV;
 
-   /* Allow as many queues as there are CPUs, by default */
-   xenvif_max_queues = num_online_cpus();
+   /* Allow as many queues as there are CPUs if user has not
+* specified a value.
+*/
+   if (xenvif_max_queues == 0)
+   xenvif_max_queues = num_online_cpus();
 
if (fatal_skb_slots < XEN_NETBK_LEGACY_SLOTS_MAX) {
pr_info("fatal_skb_slots too small (%d), bump it to 
XEN_NETBK_LEGACY_SLOTS_MAX (%d)\n",
-- 
2.1.4


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

Re: [Xen-devel] [Patch V1 2/3] xen/usb: add capability for passing through isoc jobs to host devices

[Gerd Hoffmann]

  Hi,

> > So, the signaling needs to be different.  The host adapter needs to
> > signal somehow that it can handle async iso packets.  One way would be
> > to flag this per usb bus, another one per usb packet.  Also all xen
> > naming and the xen inlude should go away.  BTW: does this build without
> > xen-devel installed?
> 
> Okay, I'll try to make it more generic. I think the async iso capability
> should be a bus attribute.

Makes sense.

> > Can we get rid of the callbacks?  By filling the USBPacket iovec with
> > the iso request chunks for example?
> 
> Difficult. One iso request chunk could require multiple iovec entries.

Why multiple small iovecs instead of one big iovec?

usb_host_req_complete_iso_xen() returns a single status for the whole
USBPacket anyway ...

> The RFC version tried to avoid the callbacks and there you didn't like
> exposing the additional structures.

-ENOPATCH.

cheers,
  Gerd



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

Re: [Xen-devel] [PATCH v6 04/31] xen/arm: Set nr_cpu_ids to available number of cpus

[Ian Campbell]

On Mon, 2015-08-31 at 15:25 +0100, Julien Grall wrote:
> title: Set nr_cpu_ids to the number of CPUs available
> 
> On 31/08/2015 12:06, vijay.kil...@gmail.com wrote:
> > From: Vijaya Kumar K 
> > 
> > nr_cpu_ids for arm platforms is set to NR_CPUS irrespective of
> 
> s/is set/is incorrectly set/ to make clear that it's the previous 
> behavior and not the one that you are implemented.
> 
> > number of cpus supported by platform.
> 
> ^ the number
> 
> > 
> > Signed-off-by: Vijaya Kumar K 
> 
> With the typos mentioned:
> 
> Reviewed-by: Julien Grall 

Agreed, with those fixed: Acked-by: Ian Campbell 


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

Re: [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Jan Beulich]

>>> On 09.09.15 at 12:48,  wrote:
> Personally I think that this dynamic approach is overkill (mainly
> because I'm fine with being unable to install Debian Wheezy guests, both
> wearing and not wearing my red fedora; and because the properties table
> feature is not active for *any* OVMF guests anyway, in practice).

I can only guess that PCD stands for "Platform Configuration Data".
However, I would want to suggest an even more dynamic approach:
Assuming that within the core UEFI code it ought to be possible to
flip between executable and non-executable mapping of the stack,
and considering that PE headers can carry target version numbers,
how about reverting to an executable stack as long as there's at
least one binary loaded that isn't claiming to be 2.5 compatible?

Jan


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

[Xen-devel] ANNOUNCEMENT: Xen 4.6 RC3

[Wei Liu]

Hi all

Xen 4.6 RC3 has been tagged. You can check out the tag 4.6.0-rc3 in xen.git.

The tarball can be downloaded from:

http://bits.xensource.com/oss-xen/release/4.6.0-rc3/xen-4.6.0-rc3.tar.gz

Signature for tarball:

http://bits.xensource.com/oss-xen/release/4.6.0-rc3/xen-4.6.0-rc3.tar.gz.sig

When reporting bugs, please send your bug report to
xen-de...@lists.xenproject.org, present as much information as possible, tag it
with "BUG-4.6" and CC release manager (wei.l...@citrix.com) and relevant
maintainers.

Annoucement for test day will be made separately.

Known issues / pending patches:

Subject: [PATCH v2] efi: introduce efi_arch_flush_dcache_area
Message-ID: <1441708697-578-1-git-send-email-stefano.stabell...@eu.citrix.com>

Subject: [PATCH for 4.6] x86/VPMU: Set VPMU context pointer to NULL when 
freeing it
Message-ID: <1441767352-9022-1-git-send-email-boris.ostrov...@oracle.com>

Subject: [v2][PATCH] xen/vtd/iommu: permit group devices to passthrough in 
relaxed mode
Message-ID: <1441763998-4937-1-git-send-email-tiejun.c...@intel.com>

Subject: [Xen-devel] [PATCH] x86/hvm: fix saved pmtimer value
Message-ID: <87egi8kpzy@pingu.sky.yk.fujitsu.co.jp>

Guest with vNUMA configured can't be saved because save record doesn't contain
node information. Patches under development.

./configure --enable-systemd won't fail even if no systemd development files
are found. Patch to be developed.

Wei.

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

Re: [Xen-devel] On distro packaging of stub domains (Re: Notes from Xen BoF at Debconf15)

[Antti Kantee]

Ian,

Thank you for the explanations.

Hmm.  (not replying to anything specific)

My guess is that shared libs won't be the biggest problem.  I'd find it 
extremely surprising if you can take a Linux (or any other !NetBSD) 
packaging system and discover the dozens of dependencies of QEMU to not 
contain any "'isms" which do not apply when building for what is 
essentially a NetBSD target.


I don't know how Wei Liu built his QEMU, but I assume it was by farming 
a lot of --disable-stuff.  That's what I'd do and somehow find a way to 
ship the result.  The requirements for stubdom QEMU and /usr/bin/qemu 
are IMO too dissimilar to stuffed into the same box.  By my judgement 
(which may be wrong), almost none of the dynamic dependencies of QEMU on 
a regular Linux system apply for the Xen stub domain use.  If the goal 
is to get a reduced footprint qemu, bundling unnecessary clutter is in 
direct conflict with the goal.  Besides, trying to get e.g. libsystemd 
to build or work with [a NetBSD-API'd] Rumprun will most likely earn you 
a ticket to the loony bin.


So while I think I understand your predicament (you need to sell the Xen 
improvement to distros which means playing by their rules) and the 
temptation of reusing existing packages for the job, I seriously doubt 
the approach will lead to a sensible result.  That, of course, shouldn't 
stop you from trying.  If the result of your experimentation matches 
your hypothesis and shared libs is the main problem, I'll figure out a 
way to make it work on the Rumprun side of things.


If you truly decide you need to use existing Linux infra, I'd start down 
that track by bolting a Linux userspace environment on top of the 
"kernel only" Rumprun stack (which could/should more or less work thanks 
to syscall emulation).  Of course, you'd need to do the same for FreeBSD 
and every other system you want to support, so it's not a free ticket 
either, but by my guess at least a cheaper one.


Anyway, I only have guesses.

  - antti

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

Re: [Xen-devel] [PATCH v6 11/29] xen/x86: add bitmap of enabled emulated devices

[Wei Liu]

On Fri, Sep 04, 2015 at 02:08:50PM +0200, Roger Pau Monne wrote:
> Introduce a bitmap in x86 xen_arch_domainconfig that allows enabling or
> disabling specific devices emulated inside of Xen for HVM guests.
> 
> Signed-off-by: Roger Pau Monné 
> Reviewed-by: Andrew Cooper 
> Cc: Ian Jackson 
> Cc: Stefano Stabellini 
> Cc: Ian Campbell 
> Cc: Wei Liu 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH] xen/domctl: lower loglevel of XEN_DOMCTL_memory_mapping

[Jan Beulich]

>>> On 09.09.15 at 16:20,  wrote:
> Perhaps the solution is remove the first printk(s) and just have them
> once the operation has completed? That may fix the outstanding tasklet
> problem?

Considering that this is a tool stack based retry, how would the
hypervisor know when the _whole_ operation is done?

Jan


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

Re: [Xen-devel] [PATCH v6 09/31] xen/arm: ITS: Add APIs to add and assign device

[Julien Grall]

On 09/09/15 14:28, Ian Campbell wrote:
> On Thu, 2015-09-03 at 18:34 +0100, Julien Grall wrote:
>> @@ -522,6 +535,205 @@ static void its_lpi_free(struct its_device *dev)
>>>  xfree(dev->event_map.lpi_map);
>>>  }
>>>  
>>> +static void its_discard_lpis(struct its_device *dev, u32 ids)
>>> +{
>>> +int i;
>>> +
>>
>> I would have expected a function more complex than that. If you discard
>> the LPIs, you also need to free the MSI desc and potentially reset the
>> IRQ desc.
>>
>> Otherwise you will left the irq_desc in an unknown state for the next
>> one.
> 
> But discard != free? (or at least "discard" has a specific meaning in its).

In the ITS, discard means removing the mapping from the MSI (eventID) to
the LPI.

So after discarding we have to clean up the irq_desc in order to let
some else re-using the IRQ desc. This means:
- Free the MSI desc otherwise there is a leak
- Reset the irq_desc fields in the initial value
- Potentially poke the hardware to disable the interrupt

> If this function is supposed to free everything then I would agree, and
> also add that the function is therefore badly (or at least confusingly)
> named.
> If it is just supposed to discard (==clear any h/w pending state of an LPI
> mapped by a given event on a given device) then I think it is correct,
> isn't it?

IHMO, the function is correctly named. Resetting the irq_desc in a valid
state is part of the discard because we just replicate the internal state.

Resetting the irq_desc means free the msi_desc in order to avoid leak.
Although, if the msi_desc would have been allocated in the event_lpi_map
all this extra care wouldn't have been necessary.

I mean having a new field in event_lpi_map msis which is an array of
msi_desc.

>>> +xfree(dev->event_map.col_map);
>>> +xfree(dev);
>>> +}
>>> +
>>> +static struct its_device *its_alloc_device(u32 devid, u32 nr_ites,
>>> +   struct dt_device_node
>>> *dt_its)
>>> +{
>>> +struct its_device *dev;
>>> +paddr_t *itt;
>>
>> Why paddr_t? You only allocate it and pass directly to the hardware.
> 
> paddr_t seems correct, it the fact that it is a paddr_t * (i.e. a pointer)
> which seems odd to me. I think you commented the same thing on dev
> ->itt_addr which is where this ends up assigned.

With the current usage, we store the result of xmalloc in the itt
variable. So it's a pointer to a virtual address not a paddr_t.

If we decide to use paddr_t, then we should also update the code.
Although, the Linux ITS code is using void * in order to store the
pointer. So I'd like to keep the same in order to avoid differing too
much for Linux (though with all this coding style it would be difficult
to backport code).

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v4 10/11] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Jan Beulich]

>>> On 09.09.15 at 14:56,  wrote:
> Can you please explain more why it doesn't scale? 
> From my point of view, any other future value representation can be passed 
> from the producer to the related consumer through this method. 

Did you read all of my earlier replies? I already said there "Just
consider what happens to the code when we end up gaining a
few more drivers providing percentage values, and perhaps
another one providing a third variant of output representation."

Jan


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

Re: [Xen-devel] [PATCH v2 0/4] Intel Code/Data Prioritization(CDP) feature enabling

[Ian Campbell]

On Wed, 2015-09-09 at 15:37 +0800, Chao Peng wrote:
> On Wed, Sep 09, 2015 at 01:16:44PM +0800, He Chen wrote:
> > Changes in v2:
> > - x86: Enable CDP by boot parameter instead of enabling/disabling CDP
> > at
> > runtime (suggested by Andrew)
> 
> As you added a new boot option, you also need a patch for
> docs/misc/xen-command-line.markdown.

FYI this is (usually) best done in the patch which adds the option, rather
than in a separate patch.

Ian.


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

Re: [Xen-devel] [PATCH v4 10/11] x86/intel_pstate: support the use of intel_pstate in pmstat.c

[Wang, Wei W]

On 09/09/2015 19:57,  Jan Beulich wrote:
>>> On 09.09.15 at 12:35,  wrote:
> On 09/09/2015 18:10,  Jan Beulich wrote:
 On 09.09.15 at 11:35,  wrote:
>>> Using the drinking vessel analogy, we are not putting milk and water  
>>>into the vessel at the same time. If the producer puts water into the  
>>>vessel, then the consumer simply consumes water; If the producer puts  
>>>milk into the vessel, then the consumer simply consumes milk. I think  
>>>we don't need to worry about what type of drinking is put inside the  
>>>vessel, because the vessel is just an intermediate place holding the 
>>>liquid between the producer and consumer - the consumer has the  
>>>privity of contract with the producer and it has the right logic to 
>>>deal
> with what's inside the vessel.
> 
>> This analogy breaks for a blind: How would he know whether there's 
>> water or
> milk in there? He'd have to try it. 
>>Now what if we use >venom instead of milk in your analogy?
> 
>>Bottom line - the consumer needs to know what kind of data it is to 
>>expect to
> consume.
> 
> There is a contract between the consumer and the producer. In our 
> case, the contract is "p_cpufreq->scaling_driver". Before the right 
> consumer consumes the value of " uint32_t scaling_max_perf ", it goes through 
> the check:
>  +if (!strncmp(p_cpufreq->scaling_driver,
>  +  "intel_pstate", CPUFREQ_NAME_LEN) )
> , where "intel_pstate" can be changed to other new driver names 
> (contract) in the future.

> Which is precisely what I already said doesn't scale.

Can you please explain more why it doesn't scale? 
>From my point of view, any other future value representation can be passed 
>from the producer to the related consumer through this method. 

Best,
Wei


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

Re: [Xen-devel] [PATCH v6 05/31] xen/arm: Rename NR_IRQs and vgic_num_irqs helper function

[Ian Campbell]

On Mon, 2015-08-31 at 15:40 +0100, Julien Grall wrote:
> [...]
> > +#define nr_irqs NR_LINE_IRQS
> > +#define nr_static_irqs NR_LINE_IRQS
> > +#define arch_hwdom_irqs(domid) NR_LINE_IRQS

An aside: While looking at this patch I realised that all of these are used
by common code solely in relation to d->nr_pirqs. Since we don't have pirqs
on ARM really that field ought to be 0 (or even non-existent) and all of
these should then go away.

(Actually nr_irqs is used in flask, but the above is true for the rest for
sure).

Nothing to do with this patch and no action for you to take Vijay.


Ian.

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

Re: [Xen-devel] [PATCH v6 08/31] xen/arm: ITS: Introduce msi_desc for LPIs

[Ian Campbell]

On Mon, 2015-08-31 at 16:36 +0530, vijay.kil...@gmail.com wrote:
> From: Vijaya Kumar K 
> 
> Define msi_desc structure for arm and introduce
> helper functions to access msi_desc member variables.
> 
> Signed-off-by: Vijaya Kumar K 
> ---
>  xen/arch/arm/gic-v3-its.c |   28 
>  xen/arch/arm/irq.c|   12 
>  xen/include/asm-arm/gic-its.h |4 
>  xen/include/asm-arm/irq.h |9 +
>  4 files changed, 53 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 88cc89d..e70c21a 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -109,6 +109,34 @@ static void dump_cmd(const its_cmd_block *cmd)
>  static void dump_cmd(const its_cmd_block *cmd) { }
>  #endif
>  
> +void irqdesc_set_lpi_event(struct irq_desc *desc, unsigned id)

LPIs are logically part of the GIC, not the ITS, so I think all four of
these belong at least in a gic (without -its) .c file if not in irq.c.

Or is there a reason for them to live here?

> +{
> +ASSERT(spin_is_locked(>lock));
> +
> +irq_get_msi_desc(desc)->eventID = id;
> +}
> +
> +unsigned int irqdesc_get_lpi_event(struct irq_desc *desc)
> +{
> +ASSERT(spin_is_locked(>lock));
> +
> +return irq_get_msi_desc(desc)->eventID;
> +}
> +
> +struct its_device *irqdesc_get_its_device(struct irq_desc *desc)
> +{
> +ASSERT(spin_is_locked(>lock));
> +
> +return irq_get_msi_desc(desc)->dev;
> +}
> +
> +void irqdesc_set_its_device(struct irq_desc *desc, struct its_device
> *dev)
> +{
> +ASSERT(spin_is_locked(>lock));
> +
> +irq_get_msi_desc(desc)->dev = dev;
> +}
> +
>  static struct its_collection *dev_event_to_col(struct its_device *dev,
> u32 event)
>  {
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index d8080c7..24c4f24 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -143,6 +143,18 @@ static inline struct domain *irq_get_domain(struct
> irq_desc *desc)
>  return irq_get_guest_info(desc)->d;
>  }
>  
> +void irq_set_msi_desc(struct irq_desc *desc, struct msi_desc *msi)
> +{
> +desc->msi_desc = msi;
> +}
> +
> +struct msi_desc *irq_get_msi_desc(struct irq_desc *desc)
> +{
> +ASSERT(desc->msi_desc != NULL);
> +
> +return desc->msi_desc;
> +}
> +
>  void irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask)
>  {
>  if ( desc != NULL )
> diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic
> -its.h
> index 70e7f54..25c2176 100644
> --- a/xen/include/asm-arm/gic-its.h
> +++ b/xen/include/asm-arm/gic-its.h
> @@ -271,6 +271,10 @@ struct its_device {
>  struct rb_node  node;
>  };
>  
> +void irqdesc_set_lpi_event(struct irq_desc *desc, unsigned id);
> +unsigned int irqdesc_get_lpi_event(struct irq_desc *desc);
> +struct its_device *irqdesc_get_its_device(struct irq_desc *desc);
> +void irqdesc_set_its_device(struct irq_desc *desc, struct its_device
> *dev);
>  int its_init(struct rdist_prop *rdists);
>  int its_cpu_init(void);
>  
> diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
> index cbdc1ab..bddd1ea 100644
> --- a/xen/include/asm-arm/irq.h
> +++ b/xen/include/asm-arm/irq.h
> @@ -18,6 +18,13 @@ struct arch_irq_desc {
>  unsigned int type;
>  };
>  
> +struct msi_desc {
> +#ifdef HAS_GICV3
> +unsigned int eventID;
> +struct its_device *dev;
> +#endif
> +};
> +
>  #define NR_LOCAL_IRQS32
>  /* Number of SGIs+PPIs+SPIs */
>  #define NR_LINE_IRQS 1024
> @@ -56,6 +63,8 @@ int irq_set_spi_type(unsigned int spi, unsigned int
> type);
>  int platform_get_irq(const struct dt_device_node *device, int index);
>  
>  void irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask);
> +void irq_set_msi_desc(struct irq_desc *desc, struct msi_desc *msi);
> +struct msi_desc *irq_get_msi_desc(struct irq_desc *desc);
>  
>  #endif /* _ASM_HW_IRQ_H */
>  /*

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

Re: [Xen-devel] [PULL 0/19] xen-2015-09-08-tag

[Stefano Stabellini]

On Tue, 8 Sep 2015, Peter Maydell wrote:
> On 8 September 2015 at 18:21, Stefano Stabellini
>  wrote:
> > The following changes since commit 8611280505119e296757a60711a881341603fa5a:
> >
> >   target-microblaze: Use setcond for pcmp* (2015-09-08 08:49:33 +0200)
> >
> > are available in the git repository at:
> >
> >   git://xenbits.xen.org/people/sstabellini/qemu-dm.git 
> > tags/xen-2015-09-08-tag
> >
> > for you to fetch changes up to ba2250ad148997b1352aba976aac66b55410e7e4:
> >
> >   xen/pt: Use XEN_PT_LOG properly to guard against compiler warnings. 
> > (2015-09-08 15:21:56 +)
> >
> > 
> > Xen branch xen-2015-09-08
> >
> > 
> 
> Hi. I'm afraid this fails to build on OSX (and probably Windows too,
> though that build hasn't run yet):
> 
>   CCi386-softmmu/hw/i386/pci-assign-load-rom.o
> /Users/pm215/src/qemu/hw/i386/pci-assign-load-rom.c:6:10: fatal error:
>   'sys/io.h' file not found
> #include 
>  ^
>   CCalpha-softmmu/hw/alpha/pci.o
> 1 error generated.

Tiejun,

this is caused by 33d33242b7d802e6c994f3d56ecba96a66465dc3,
"hw/pci-assign: split pci-assign.c". Could you please double-check
non-Linux builds?


I suspect that the fix would be quite small, but I don't have an OSX or
a Windows build environment to try it.

Speak about build environments, Peter, would you care to share your
scripts and setup so that I can run similar tests in the future on my
own?  I have no OSX machines so I tried to do a Windows
cross-compile, following http://wiki.qemu.org/Hosts/W32 on Debian 7, but
I failed very early with an "ERROR: zlib check failed".

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

Re: [Xen-devel] [PATCH v6 05/31] xen/arm: Rename NR_IRQs and vgic_num_irqs helper function

[Julien Grall]

On 09/09/15 14:08, Ian Campbell wrote:
> On Mon, 2015-08-31 at 15:40 +0100, Julien Grall wrote:
>> [...]
>>> +#define nr_irqs NR_LINE_IRQS
>>> +#define nr_static_irqs NR_LINE_IRQS
>>> +#define arch_hwdom_irqs(domid) NR_LINE_IRQS
> 
> An aside: While looking at this patch I realised that all of these are used
> by common code solely in relation to d->nr_pirqs. Since we don't have pirqs
> on ARM really that field ought to be 0 (or even non-existent) and all of
> these should then go away.

nr_pirqs is used in DOMCTL_irq_permission to know the last IRQ number
suported. It's working correctly domain_pirq_to_irq is an identity mapping.

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v6 08/31] xen/arm: ITS: Introduce msi_desc for LPIs

[Julien Grall]

On 09/09/15 14:16, Ian Campbell wrote:
> On Mon, 2015-08-31 at 16:36 +0530, vijay.kil...@gmail.com wrote:
>> From: Vijaya Kumar K 
>>
>> Define msi_desc structure for arm and introduce
>> helper functions to access msi_desc member variables.
>>
>> Signed-off-by: Vijaya Kumar K 
>> ---
>>  xen/arch/arm/gic-v3-its.c |   28 
>>  xen/arch/arm/irq.c|   12 
>>  xen/include/asm-arm/gic-its.h |4 
>>  xen/include/asm-arm/irq.h |9 +
>>  4 files changed, 53 insertions(+)
>>
>> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>> index 88cc89d..e70c21a 100644
>> --- a/xen/arch/arm/gic-v3-its.c
>> +++ b/xen/arch/arm/gic-v3-its.c
>> @@ -109,6 +109,34 @@ static void dump_cmd(const its_cmd_block *cmd)
>>  static void dump_cmd(const its_cmd_block *cmd) { }
>>  #endif
>>  
>> +void irqdesc_set_lpi_event(struct irq_desc *desc, unsigned id)
> 
> LPIs are logically part of the GIC, not the ITS, so I think all four of
> these belong at least in a gic (without -its) .c file if not in irq.c.
> 
> Or is there a reason for them to live here?

Putting those helpers in irq.c will require some #ifdery in order to not
compile when GICV3 is not built. I'd like to avoid any #ifdef in the
common code for ITS specific code (see irqdesc_set_its_device).

Although, as I said in a previous mail, those helpers are pointless as
they are only used a couple of time and always in a batch. So we should
result to fetch MSI everytime we'd like to access a field.

Regards,

-- 
Julien Grall

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

[Xen-devel] Xen 4.6 branched, unstable reopen for new contributions

[Wei Liu]

Hi all

Xen 4.6 has been branched and xen-unstable is reopen to new contributions.

The commit moratorium is now lifted. Committers can now commit any
remaining patches to both branches.  Note that Xen 4.6 is not yet out
released, please consider the impact on Xen 4.6 branch when applying new
patches.

Please let Ian Jackson and me know if there is any administrative error
or plumbing bug.

Wei.

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

Re: [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Laszlo Ersek]

On 09/09/15 14:08, Jan Beulich wrote:
 On 09.09.15 at 12:48,  wrote:
>> Personally I think that this dynamic approach is overkill (mainly
>> because I'm fine with being unable to install Debian Wheezy guests, both
>> wearing and not wearing my red fedora; and because the properties table
>> feature is not active for *any* OVMF guests anyway, in practice).
> 
> I can only guess that PCD stands for "Platform Configuration Data".

Yes, PCD stands for Platform Configuration Database.

> However, I would want to suggest an even more dynamic approach:
> Assuming that within the core UEFI code it ought to be possible to
> flip between executable and non-executable mapping of the stack,
> and considering that PE headers can carry target version numbers,
> how about reverting to an executable stack as long as there's at
> least one binary loaded that isn't claiming to be 2.5 compatible?

This would require very intrusive changes (to be implemented by people
other than me). Other concerns I have:

- I'm not sure if UEFI applications have any means to advertize what
  revision of the specification they target. (As you mention.)

- I could be mistaken, but this looks like it could weaken the
  protection offered by the platform feature. UEFI applications and
  UEFI drivers are expected to come from third parties. It could be
  argued that a non-compatible UEFI app or driver (like Wheezy's grub
  in this case) *should* preferably crash, immediately, in a controlled
  manner, instead of silently downgrading the protection for the stack,
  and thereby opening up an avenue for attacks.

  I'm not fully convinced about this, but it appears this should be
  controlled somewhere in the platform code. (Like a knob in the BIOS
  setup on physical platforms, or some enlighted info channel in
  guests.) Assuming we'd like it dynamic.

Thanks
Laszlo

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

Re: [Xen-devel] [Patch V1 2/3] xen/usb: add capability for passing through isoc jobs to host devices

[Juergen Gross]

On 09/09/2015 02:01 PM, Gerd Hoffmann wrote:

   Hi,


So, the signaling needs to be different.  The host adapter needs to
signal somehow that it can handle async iso packets.  One way would be
to flag this per usb bus, another one per usb packet.  Also all xen
naming and the xen inlude should go away.  BTW: does this build without
xen-devel installed?


Okay, I'll try to make it more generic. I think the async iso capability
should be a bus attribute.


Makes sense.


Can we get rid of the callbacks?  By filling the USBPacket iovec with
the iso request chunks for example?


Difficult. One iso request chunk could require multiple iovec entries.


Why multiple small iovecs instead of one big iovec?


The guest buffer might span multiple physical non contiguous pages. I
don't want to copy data to a new buffer due to performance reasons
(there is already at least one copy operation done by qemu).


usb_host_req_complete_iso_xen() returns a single status for the whole
USBPacket anyway ...


I need status per iso request, and libusb does deliver that.


The RFC version tried to avoid the callbacks and there you didn't like
exposing the additional structures.


-ENOPATCH.


Aah, sorry, my fault. I think this was part of the off-list discussion
we had.


Juergen


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

Re: [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Jan Beulich]

>>> On 09.09.15 at 15:04,  wrote:
> On 09/09/15 14:08, Jan Beulich wrote:
> On 09.09.15 at 12:48,  wrote:
>> However, I would want to suggest an even more dynamic approach:
>> Assuming that within the core UEFI code it ought to be possible to
>> flip between executable and non-executable mapping of the stack,
>> and considering that PE headers can carry target version numbers,
>> how about reverting to an executable stack as long as there's at
>> least one binary loaded that isn't claiming to be 2.5 compatible?
> 
> This would require very intrusive changes (to be implemented by people
> other than me). Other concerns I have:
> 
> - I'm not sure if UEFI applications have any means to advertize what
>   revision of the specification they target. (As you mention.)

They can (as I said). Whether the image loader in UEFI actually
looks at the data today is another thing, as is whether everyone
makes sure their binaries say so.

And then again I realize that usually one would rather state the
minimum version required than that of the specification a binary
was built against, so perhaps the idea was a bad one anyway.

Jan


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

Re: [Xen-devel] [Patch V1 3/3] xen: add pvUSB backend

[Stefano Stabellini]

On Wed, 9 Sep 2015, Juergen Gross wrote:
> On 09/07/2015 07:38 PM, Stefano Stabellini wrote:
> > On Thu, 3 Sep 2015, Juergen Gross wrote:
> > > Add a backend for para-virtualized USB devices for xen domains.
> > > 
> > > The backend is using host-libusb to forward USB requests from a
> > > domain via libusb to the real device(s) passed through.
> > > 
> > > Signed-off-by: Juergen Gross 
> > 
> > Aside from a few minor comments below, this looks pretty good from a Xen
> > perspective, however I am not too qualified to review the details of the
> > pvusb protocol or the way the requests are handled internally in QEMU.
> > 
> > I would be glad if Gerd could take a look at this too.
> > 
> > Juergen, if we commit this, would you be happy to maintain it going
> > forward?
> 
> Sure.
> 
> > > diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
> > > new file mode 100644
> > > index 000..2570bd7
> > > --- /dev/null
> > > +++ b/hw/usb/xen-usb.c
> ...
> > > +#define TR(fmt, args...)\
> > > +{   \
> > > +struct timeval tv;  \
> > > +\
> > > +gettimeofday(, NULL);\
> > > +fprintf(stderr, "%8ld.%06ld xen-usb(%s):" fmt, tv.tv_sec,   \
> > > +tv.tv_usec, __func__, ##args);  \
> > 
> > Please use xen_be_printf. I am not sure I would go as far as using
> > gettimeofday, but I can see it could be useful here.
> 
> Okay. I'll keep the time information as it has been proved to be really
> valuable.
> 
> > > +}
> > > +#define TR_REQ(fmt, args...) { if (tr_debug & 1) TR(fmt, ##args) }
> > > +#define TR_BUS(fmt, args...) { if (tr_debug & 2) TR(fmt, ##args) }
> > 
> > I would drop these and just use the loglevels of xen_be_printf.
> 
> Hmm, something like:
> 
> TR -> level 1
> TR_BUS -> level 2
> TR_REQ -> level 3
> 
> Is it possible to control trace levels per device?

Yes, with xendev->debug.


> If not, trying to
> catch an error requiring to trace on request level might be hard as
> concurrent qdisk trace entries could affect timing severely.
> 
> ...
> > > +static void xen_bus_child_detach(USBPort *port, USBDevice *child)
> > > +{
> > > +TR_BUS("called\n");
> > > +}
> > > +
> > > +static void xen_bus_complete(USBPort *port, USBPacket *packet)
> > > +{
> > > +TR_REQ("called\n");
> > 
> > Could you please either remove these debug messages or make them more
> > informative.
> 
> Which information are you missing? As long as the TR_* macros aren't
> changed they'll trace the function as well. In the context of other
> traces written they have been completely valuable.

I am saying that "called" is not very informative as a debug message,
you might as well remove it and rearrage the calls to be just:

TS_BUS();

But if you want to trace the call, maybe you could use the trace_*
framework?

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

Re: [Xen-devel] [PATCH v6 09/31] xen/arm: ITS: Add APIs to add and assign device

[Ian Campbell]

On Thu, 2015-09-03 at 18:34 +0100, Julien Grall wrote:
> @@ -522,6 +535,205 @@ static void its_lpi_free(struct its_device *dev)
> >  xfree(dev->event_map.lpi_map);
> >  }
> >  
> > +static void its_discard_lpis(struct its_device *dev, u32 ids)
> > +{
> > +int i;
> > +
> 
> I would have expected a function more complex than that. If you discard
> the LPIs, you also need to free the MSI desc and potentially reset the
> IRQ desc.
> 
> Otherwise you will left the irq_desc in an unknown state for the next
> one.

But discard != free? (or at least "discard" has a specific meaning in its).

If this function is supposed to free everything then I would agree, and
also add that the function is therefore badly (or at least confusingly)
named.

If it is just supposed to discard (==clear any h/w pending state of an LPI
mapped by a given event on a given device) then I think it is correct,
isn't it?

> > +xfree(dev->event_map.col_map);
> > +xfree(dev);
> > +}
> > +
> > +static struct its_device *its_alloc_device(u32 devid, u32 nr_ites,
> > +   struct dt_device_node
> > *dt_its)
> > +{
> > +struct its_device *dev;
> > +paddr_t *itt;
> 
> Why paddr_t? You only allocate it and pass directly to the hardware.

paddr_t seems correct, it the fact that it is a paddr_t * (i.e. a pointer)
which seems odd to me. I think you commented the same thing on dev
->itt_addr which is where this ends up assigned.

Ian.

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

Re: [Xen-devel] [PATCH v6 24/31] xen/arm: ITS: Add GICR register emulation

[Ian Campbell]

On Mon, 2015-09-07 at 20:56 +0530, Vijay Kilari wrote:
> On Mon, Sep 7, 2015 at 7:50 PM, Julien Grall 
> wrote:
> > Hi Vijay,
> > 
> > On 31/08/15 12:06, vijay.kil...@gmail.com wrote:
> > > +static int vgic_v3_gits_lpi_mmio_read(struct vcpu *v, mmio_info_t
> > > *info)
> > 
> > [...]
> > 
> > > +/* Neglect if not LPI. */
> > > +if ( offset < FIRST_GIC_LPI )
> > > +{
> > > +*r = 0;
> > > +return 1;
> > > +}
> > 
> > [...]
> > 
> > > +static int vgic_v3_gits_lpi_mmio_write(struct vcpu *v, mmio_info_t
> > > *info)
> > > +{
> > 
> > [...]
> > 
> > > +/* Neglect if not LPI. */
> > > +if ( offset < FIRST_GIC_LPI )
> > > +return 1;
> > 
> > Based on the spec, those 2 checks are wrong and make impossible to use
> > LPIs. Please test this patch series before sending it on the ML.
> 
> Why do you think so?.

Consider which LPI is the subject of the word at the address pointed to by
GICR_PROPBASER. I think it is LPI==0 (== IRQ 8192), whereas this code
suggests that you think the entry for LPI==0 is at offset 8192 in the prop
table, which I don't think is correct.

Ian.

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

Re: [Xen-devel] [PATCH] xen/domctl: lower loglevel of XEN_DOMCTL_memory_mapping

[Konrad Rzeszutek Wilk]

On Wed, Sep 09, 2015 at 02:50:25PM +0800, Tiejun Chen wrote:
> We should lower loglevel to XENLOG_G_DEBUG while mapping or
> unmapping memory via XEN_DOMCTL_memory_mapping since its
> fair enough to check this info just while debugging.

The issue you folks are hitting where it takes eons to boot
with a large BAR _may_ be related to tasklets and triggering
the hypercall_preempt_check().

I think you may be using 'console_to_ring' or such and collecting
the hypervisor logs. That will schedule an tasklet whenever printk
is used - and the tasklet will cause hypercall_preempt_check() to
return true.

So what we get is that on the entrace (even before calling map_mmio_region)
for this function we have an outstanding softirq we must process - which
means that this function exits as soon as possible to service it.

Perhaps the solution is remove the first printk(s) and just have them
once the operation has completed? That may fix the outstanding tasklet
problem?

Could you try that (or perhaps you have already tried it?) please?

> 
> CC: Ian Campbell 
> CC: Ian Jackson 
> CC: Jan Beulich 
> CC: Keir Fraser 
> CC: Tim Deegan 
> Signed-off-by: Tiejun Chen 
> ---
>  xen/common/domctl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index 7f959f3..3bf39f1 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -1049,7 +1049,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>  
>  if ( add )
>  {
> -printk(XENLOG_G_INFO
> +printk(XENLOG_G_DEBUG
> "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
> d->domain_id, gfn, mfn, nr_mfns);
>  
> @@ -1061,7 +1061,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>  }
>  else
>  {
> -printk(XENLOG_G_INFO
> +printk(XENLOG_G_DEBUG
> "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
> d->domain_id, gfn, mfn, nr_mfns);
>  
> -- 
> 1.9.1
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

[Xen-devel] [raisin][PATCH 2/3] Delete dangling white space

[Doug Goldstein]

Signed-off-by: Doug Goldstein 
---
 lib/commands.sh   |  4 ++--
 lib/common-functions.sh   | 10 +-
 scripts/mkdeb |  8 
 scripts/mkrpm |  4 ++--
 tests/busybox-hvm |  8 
 tests/busybox-hvm-migrate |  4 ++--
 tests/busybox-pv  |  6 +++---
 7 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/lib/commands.sh b/lib/commands.sh
index a79611e..be445a7 100755
--- a/lib/commands.sh
+++ b/lib/commands.sh
@@ -50,12 +50,12 @@ function install-builddep() {
 
 function build() {
 check-builddep
-
+
 mkdir -p "$INST_DIR" &>/dev/null
 
 # build and install under $DESTDIR
 for_each_component build
-
+
 build_package xen-system
 }
 
diff --git a/lib/common-functions.sh b/lib/common-functions.sh
index e96b105..4ab77ba 100644
--- a/lib/common-functions.sh
+++ b/lib/common-functions.sh
@@ -20,9 +20,9 @@ function common_init() {
 export PREFIX=${PREFIX-/usr}
 export INST_DIR=${DESTDIR-dist}
 export PREPEND="[raisin]"
-
+
 INST_DIR=`readlink -f $INST_DIR`
-
+
 # execution
 if [[ $EUID -eq 0 ]]
 then
@@ -62,7 +62,7 @@ function get_components() {
 COMPONENTS="$ENABLED_COMPONENTS"
 fi
 
-if [[ -z "$COMPONENTS" ]] 
+if [[ -z "$COMPONENTS" ]]
 then
 local component
 for component in `cat "$BASEDIR"/components/series`
@@ -85,7 +85,7 @@ function get_tests() {
 TESTS="$ENABLED_TESTS"
 fi
 
-if [[ -z "$TESTS" ]] 
+if [[ -z "$TESTS" ]]
 then
 local t
 for t in `cat "$BASEDIR"/tests/series`
@@ -384,7 +384,7 @@ function init_tests() {
 else
 error_echo "I don't know distro $DISTRO. It might be missing packages."
 fi
-
+
 if [[ -n "${missing[@]}" ]]
 then
 verbose_echo "Installing ${missing[@]}"
diff --git a/scripts/mkdeb b/scripts/mkdeb
index dbd92bd..5259f1e 100755
--- a/scripts/mkdeb
+++ b/scripts/mkdeb
@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 #
-# mkdeb: package $INST_DIR output in a .deb 
+# mkdeb: package $INST_DIR output in a .deb
 #
 # Takes 1 argument: the package name
 # It relies on RAISIN_ARCH and INST_DIR being set correctly
@@ -8,10 +8,10 @@
 set -e
 
 if [[ -z "$1" ]]
-then 
+then
   echo "usage: $0 package_name"
   exit 1
-fi 
+fi
 
 name=$1
 
@@ -41,7 +41,7 @@ cp config raise deb/opt/raisin
 
 # Debian doesn't use /usr/lib64 for 64-bit libraries
 if [[ -d deb/usr/lib64 ]]
-then 
+then
   cp -a deb/usr/lib64/* deb/usr/lib/
   rm -rf deb/usr/lib64
 fi
diff --git a/scripts/mkrpm b/scripts/mkrpm
index 7965c0c..88997c6 100755
--- a/scripts/mkrpm
+++ b/scripts/mkrpm
@@ -8,10 +8,10 @@
 set -e
 
 if [[ -z "$1" ]]
-then 
+then
   echo "usage: $0 package_name"
   exit 1
-fi 
+fi
 
 name="$1"
 
diff --git a/tests/busybox-hvm b/tests/busybox-hvm
index 2b6c1dd..51e9e19 100755
--- a/tests/busybox-hvm
+++ b/tests/busybox-hvm
@@ -16,17 +16,17 @@ function busybox-hvm-test() {
 echo $PREPEND busybox hvm test only valid on x86
 exit 0
 fi
-
+
 TMPDIR=`mktemp -d`
 cd $TMPDIR
-
+
 allocate_disk busybox-vm-disk $((20*1024*1024))
 LOOP=`create_loop busybox-vm-disk`
 LOOP_P0=`create_one_partition busybox-vm-disk`
 busybox_rootfs $LOOP_P0
 busybox_network_init $LOOP_P0
 bootloader_init $LOOP $LOOP_P0
-
+
 cat >busybox-hvm 

[Xen-devel] [raisin][PATCH 3/3] Fix up coding style

[Doug Goldstein]

Signed-off-by: Doug Goldstein 
---
 lib/git-checkout.sh | 26 +-
 scripts/mkdeb   | 22 +++---
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/lib/git-checkout.sh b/lib/git-checkout.sh
index 2ca8f25..d462453 100755
--- a/lib/git-checkout.sh
+++ b/lib/git-checkout.sh
@@ -3,8 +3,8 @@
 function git-checkout() {
 if [[ $# -lt 3 ]]
 then
-   echo "Usage: $0   "
-   exit 1
+echo "Usage: $0   "
+exit 1
 fi
 
 TREE=$1
@@ -15,17 +15,17 @@ function git-checkout() {
 
 if [[ ! -d $DIR-remote ]]
 then
-   rm -rf $DIR-remote $DIR-remote.tmp
-   mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp
-   $GIT clone $TREE $DIR-remote.tmp
-   if [[ "$TAG" ]]
-   then
-   cd $DIR-remote.tmp
-   $GIT branch -D dummy >/dev/null 2>&1 ||:
-   $GIT checkout -b dummy $TAG
-   cd ..
-   fi
-   mv $DIR-remote.tmp $DIR-remote
+rm -rf $DIR-remote $DIR-remote.tmp
+mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp
+$GIT clone $TREE $DIR-remote.tmp
+if [[ "$TAG" ]]
+then
+cd $DIR-remote.tmp
+$GIT branch -D dummy >/dev/null 2>&1 ||:
+$GIT checkout -b dummy $TAG
+cd ..
+fi
+mv $DIR-remote.tmp $DIR-remote
 fi
 rm -f $DIR
 ln -sf $DIR-remote $DIR
diff --git a/scripts/mkdeb b/scripts/mkdeb
index 5259f1e..3796300 100755
--- a/scripts/mkdeb
+++ b/scripts/mkdeb
@@ -9,8 +9,8 @@ set -e
 
 if [[ -z "$1" ]]
 then
-  echo "usage: $0 package_name"
-  exit 1
+echo "usage: $0 package_name"
+exit 1
 fi
 
 name=$1
@@ -19,13 +19,13 @@ cd "$BASEDIR"
 
 # map the architecture, if necessary
 case "$RAISIN_ARCH" in
-  x86_32|x86_32p)  arch=i386 ;;
-  x86_64)  arch=amd64 ;;
-  arm32)   arch=armhf ;;
-  arm64)   arch=$RAISIN_ARCH;;
-  *) echo "Unknown ARCH $RAISIN_ARCH" >&2
- exit 1
- ;;
+x86_32|x86_32p)  arch=i386 ;;
+x86_64)  arch=amd64 ;;
+arm32)   arch=armhf ;;
+arm64)   arch=$RAISIN_ARCH;;
+*) echo "Unknown ARCH $RAISIN_ARCH" >&2
+   exit 1
+   ;;
 esac
 
 # Prepare the directory to package
@@ -42,8 +42,8 @@ cp config raise deb/opt/raisin
 # Debian doesn't use /usr/lib64 for 64-bit libraries
 if [[ -d deb/usr/lib64 ]]
 then
-  cp -a deb/usr/lib64/* deb/usr/lib/
-  rm -rf deb/usr/lib64
+cp -a deb/usr/lib64/* deb/usr/lib/
+rm -rf deb/usr/lib64
 fi
 
 # Fill in the debian boilerplate
-- 
2.4.6


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

[Xen-devel] [raisin][PATCH 1/3] Handle unsupported distros with a prettier message

[Doug Goldstein]

Handle unknown distros by saying "unknown" instead of an empty string
and for Gentoo users actually mention it.

Signed-off-by: Doug Goldstein 
---
 lib/common-functions.sh | 4 
 1 file changed, 4 insertions(+)

diff --git a/lib/common-functions.sh b/lib/common-functions.sh
index c52174a..e96b105 100644
--- a/lib/common-functions.sh
+++ b/lib/common-functions.sh
@@ -150,6 +150,9 @@ function get_distro() {
  sed -r 's/\"|\(|\)//g' | awk '{print $2}'`
 os_RELEASE=`awk '/VERSION_ID=/' /etc/os-release | sed 
's/VERSION_ID=//' \
 | sed 's/\"//g'`
+elif [[ -f /etc/gentoo-release ]]
+then
+os_VENDOR="Gentoo"
 fi
 
 # Simply distro version string
@@ -175,6 +178,7 @@ function get_distro() {
 PKGTYPE="rpm"
 ;;
 *)
+[ -z $os_VENDOR ] && os_VENDOR="unknown"
 DISTRO=$os_VENDOR
 PKGTYPE="unknown"
 ;;
-- 
2.4.6


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

Re: [Xen-devel] [PATCH OSSTEST v2 0/6] Have OpenStack tested on top of xen's master and libvirt's master.

[Ian Campbell]

On Thu, 2015-08-06 at 18:03 +0100, Anthony PERARD wrote:

> Anthony PERARD (6):
>   ts-kernel-build: Enable CONFIG_NETFILTER_XT_TARGET_CHECKSUM
>   ts-kernel-build: Compile CONFIG_CRYPTO_XTS in
>   ts-xen-install: Add dom0_mem runvar to control dom0 memory

I stuck these 3 into osstest's pretest with the acks given.

>   ts-openstack-devstack: Deploy OpenStack on a host
>   ts-openstack-tempest: Run Tempest to check OpenStack
>   Create a flight to test OpenStack with xen-unstable and libvirt
> 
>  ap-common |   9 ++
>  ap-fetch-version  |   4 +
>  ap-fetch-version-old  |   5 +
>  ap-print-url  |   3 +
>  cri-common|   1 +
>  make-flight   |  42 ++-
>  mfi-common|   5 +
>  sg-run-job|   6 +
>  ts-kernel-build   |   8 ++
>  ts-openstack-devstack | 304
> ++
>  ts-openstack-tempest  |  35 ++
>  ts-xen-install|   3 +-
>  12 files changed, 423 insertions(+), 2 deletions(-)
>  create mode 100755 ts-openstack-devstack
>  create mode 100755 ts-openstack-tempest
> 

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

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

[Ian Campbell]

On Thu, 2015-08-13 at 19:07 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST v3 2/2] cambridge: arrange to test
> each new baseline"):
> > Provide a new cr-daily-branch setting OSSTEST_BASELINES_ONLY which
> > causes it to only attempt to test the current baseline (if it is
> > untested) and never the tip version. Such tests will not result in any
> > push.
> > 
> > Each new baseline is tested exactly once (i.e. we aren't repeating
> > hoping for a pass), hence the correct revision is just the one tested
> > by the last run on the branch.
> > 
> > Add a cronjob to Cambridge which runs in this manner, ensuring that
> > there will usually be some sort of reasonably up to date baseline for
> > any given branch which can be used for comparisons in adhoc testing or
> > bisections.
> > 
> > This will also give us some data on the success of various branches on
> > the set of machines in Cambridge, which can be useful/interesting.
> > 
> > Signed-off-by: Ian Campbell 
> 
> Acked-by: Ian Jackson 

Added both patches to pretest, thanks.


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

Re: [Xen-devel] EDK II & GPL - Re: [edk2] OVMF BoF @ KVM Forum 2015

[Laszlo Ersek]

On 09/09/15 18:17, Jordan Justen wrote:
> On 2015-09-09 01:57:51, Laszlo Ersek wrote:
>> On 08/10/15 18:24, Laszlo Ersek wrote:
>>> Hi.
>>>
>>> Let's do an OVMF BoF at this year's KVM Forum too.
>>
>> Here's a preliminary task list, after some off-list discussion (I tried
>> to incorporate comments):
>>
>> - create GPL'd fork called "ovmf" for expediting virt development
>>   (OvmfPkg, ArmVirtPkg)
>>   - maybe leverage the feature under
>>  for
>> setting up a separate "tianocore/edk2-gpl" repo, for GPL'd
>> contributions [Jordan]
>> - repo separation by license could make things harder for packagers
>>   and QEMU bundling [Laszlo]
> 
> I would like OVMF to follow a plan for GPL that the whole EDK II
> community decides on.
> 
> I would also like to see EDK II add a (permissively licensed; BSD,
> MIT, etc) DriverPkg (DriversPkg?). We discussed this on the list
> recently:
> 
> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/17544/focus=676
> 
> So, related to this, I wonder how the community would feel about a
> GplDriverPkg. Would the community allow it as a new package in EDK II
> directly, or would a separate repo be required?
> 
> With regards to adding it directly into the EDK II tree, here are some
> potential concerns that I might anticipate hearing from the community:
> 
> * It will make it easier for contributors to choose GPL compared to a
>   permissive license, thereby limiting some users of the contribution
> 
> * GPL code will more easily be copied into the permissively licensed
>   packages
> 
> * Some might refuse to look at EDK II entirely if it has a directory
>   with GPL source code in it
> 
> Of these, I personally only sympathize with the first.
> 
> Regarding the separate OVMF repo, my hope is that it is more of a OVMF
> specific working area, rather than a 'GPL OVMF'. For example, patches
> or features that we've not yet figured out how to upstream, but we
> hopefully plan to.

Yes, the "OVMF specific working area" is also my main goal with this.

If we can find a way to (a) replace the FAT driver with the libre
software one, and (b) more generally, accept GPL drivers, without
depending on this separate OVMF repo, that's great. So "owning" those
modules is not a goal per se, just something the new repo should cover
in case we can't solve it within edk2. (E.g. with your GplDriverPkg idea.)

> Additionally, it makes sense to use it as needed for OVMF specific
> releases. (Ie, OVMF release tags)

Good idea.

Thanks!
Laszlo

> 
> -Jordan
> 
>> - document the rules / justification for "ovmf" (licensing
>>   conflicts, non-technical blockage on edk2 etc).
>> - No new mailing list needed
>> - push RH's downstream-only patches to "ovmf", wherever that makes
>>   sense
>> - remove encumbered FAT driver
>> - import Peter Batard's GPL r/o FAT driver port of GRUB's
>> - secure OpenSSL linking exception for the former from the copyright
>>   holders (Peter Batard, GRUB project)
>> - "ovmf" should be periodically rebased / should fetch+merge edk2 as
>>   master (arguments both for and against merging); distros should
>>   then track "ovmf" as their upstream, not edk2
>> - get OVMF into Fedora (as pkg) and QEMU (as bundled binary)
>> - do OVMF releases, maybe in sync with QEMU's releases
>>   - we can probably build from known good revisions from git [Alex]
>> - revive Q35 SATA driver work / poke Reza
>>   - Hannes and Gabriel have refreshed patches, but their versions differ
>>
>> ___
>> edk2-devel mailing list
>> edk2-de...@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel


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

Re: [Xen-devel] [PATCH] xen/domctl: lower loglevel of XEN_DOMCTL_memory_mapping

[Jan Beulich]

>>> On 09.09.15 at 16:50,  wrote:
> On Wed, Sep 09, 2015 at 08:33:52AM -0600, Jan Beulich wrote:
>> >>> On 09.09.15 at 16:20,  wrote:
>> > Perhaps the solution is remove the first printk(s) and just have them
>> > once the operation has completed? That may fix the outstanding tasklet
>> > problem?
>> 
>> Considering that this is a tool stack based retry, how would the
>> hypervisor know when the _whole_ operation is done?
> 
> I was merely thinking of moving the printk _after_ the map_mmio_regions
> so there wouldn't be any outstanding preemption points in map_mmio_regions
> (so it can at least do the 64 PFNs).

But there are no preemption points. That's why you needed to use
tool stack based retries in the first place.

Jan


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

Re: [Xen-devel] [PATCH v6 00/31] Add ITS support

[Ian Campbell]

On Mon, 2015-08-31 at 16:36 +0530, vijay.kil...@gmail.com wrote:
>  Some Major TODOs:
>1) Avoid making vits_process_cmd() static in later point of time
>2) How to handle LPI that does not have LPI config table entry.
>3) Enable/disable ITS to Dom0

I'm not quite sure what any of these 3 are, can you expand upon them
please.

Major TODOs I can think of (which may overlap those above which I don't
understand):

   A. Ensuring that no aspect of the vITS is exposed to domains other than
  dom0, so no mapping, no GICR registers relating to LPIs or ITS etc.
   B. A bug fat warning in the code vits_map_translation_space about the
  issues with exposing this to dom0.
   C. The issue regarding what to do with physical LPIs which arrive before
  the guest has mapped the corresponding event in the vits or which is
  masked

Have I missed anything?

I think A and B are pretty straight forward to fix.

C I think is a bit thornier, and IIRC we do not currently have a plan for
how we can address this in a generic way. I think we might have some ideas
which work only for PCI devices (or maybe only PCI-e devices?).

Ian.


> 
> Changes in v5:
>   - Added following new patches
>   0001-xen-arm-Return-success-if-dt-node-does-not-have-irq-.patch
>   0004-xen-arm-Set-nr_cpu_ids-to-available-number-of-cpus.patch
>   0009-xen-arm-ITS-Export-ITS-info-to-Virtual-ITS.patch
>   0013-xen-arm-ITS-Implement-gic_is_lpi-helper-function.patch
>   - Split patch #12 into two patches #14 & #16
>   0014-xen-arm-ITS-Allocate-irq-descriptors-for-LPIs.patch
>   0016-xen-arm-ITS-Route-LPIs.patch
>   - Introduce new API to route LPI (route_lpi_to_guest() )
>   - Moved patch #8 in v4 as patch #19
>   - irq_descritors for LPIs are allocated dynamically
>   - Removed RB-tree for managing vitual its devices. Will be
> introduced when pci-passthrough is implemented
>   - its_add_device() api now takes nr_ites and DT its node as parameters
>   - some function are kept as non-static when introduced in a patch for
> compilation purpose and eventually made static when used.
>   - Tested compilation for arm32
> 
> Changes in v4:
>   - Patch for rate limiting of error message is removed.
>   - Patch #4 and #5 in v3 is merged
>   - Merged #13 and #16 as one patch
>   - hw_irq_controller is implemented for LPIs
>   - GITS and GICR emulation for LPIs in separate patches
>   - Removed build functions for ITS command in physical ITS driver
>   - Added new patch to add and assign devices from platform file
>   - Enable compilation of vits and pits driver in separate patch
>   - Replace msi-parent property in all pci dt nodes to single
> ITS node generated by Xen for Dom0
> 
> Vijaya Kumar K (31):
>   xen/dt: Handle correctly node with interrupt-map in
> dt_for_each_irq_map
>   xen/arm: Add bitmap_find_next_zero_area helper function
>   xen: Add log2 functionality
>   xen/arm: Set nr_cpu_ids to available number of cpus
>   xen/arm: Rename NR_IRQs and vgic_num_irqs helper function
>   xen/arm: ITS: Port ITS driver to Xen
>   xen/arm: ITS: Add helper functions to manage its_devices
>   xen/arm: ITS: Introduce msi_desc for LPIs
>   xen/arm: ITS: Add APIs to add and assign device
>   xen/arm: ITS: Introduce gic_is_lpi helper function
>   xen/arm: ITS: Enable compilation of physical ITS driver
>   xen/arm: Move vgic locking inside get_irq_priority callback
>   xen/arm: ITS: implement hw_irq_controller for LPIs
>   xen/arm: ITS: Initialize physical ITS and export lpi support
>   xen/arm: ITS: Add virtual ITS driver
>   xen/arm: ITS: Add virtual ITS commands support
>   xen/arm: ITS: Store LPIs allocated and IRQ ID bits per domain
>   xen/arm: ITS: Enable virtual ITS driver
>   xen/arm: ITS: Export ITS info to Virtual ITS
>   xen/arm: ITS: Introduce helper to get number of event IDs
>   xen/arm: ITS: Add GITS registers emulation
>   xen/arm: ITS: Add virtual ITS availability check helper
>   xen/arm: ITS: Add 32-bit access to GICR_TYPER
>   xen/arm: ITS: Add GICR register emulation
>   xen/arm: ITS: Allocate irq descriptors for LPIs
>   xen/arm: ITS: Allocate pending_lpi descriptors for LPIs
>   xen/arm: ITS: Route LPIs
>   xen/arm: ITS: Add domain specific ITS initialization
>   xen/arm: ITS: Map ITS translation space
>   xen/arm: ITS: Generate ITS node for Dom0
>   xen/arm: ITS: Add pci devices in ThunderX
> 
>  xen/arch/arm/Makefile |2 +
>  xen/arch/arm/domain_build.c   |   11 +
>  xen/arch/arm/gic-hip04.c  |   19 +-
>  xen/arch/arm/gic-v2.c |   19 +-
>  xen/arch/arm/gic-v3-its.c | 1594
> +
>  xen/arch/arm/gic-v3.c |  102 ++-
>  xen/arch/arm/gic.c|   73 +-
>  xen/arch/arm/irq.c|  176 +++-
>  xen/arch/arm/platforms/Makefile   |1 +
>  xen/arch/arm/platforms/thunderx.c |  151 
>  xen/arch/arm/setup.c  |2 +
>  xen/arch/arm/vgic-v2.c|9 +-
>  

Re: [Xen-devel] [PATCH OSSTEST] Osstest/TestSupport: Hide $ho->{Toolstack} from casual use

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST] Osstest/TestSupport: Hide 
$ho->{Toolstack} from casual use"):
> This should only be accessed via toolstack($ho), which is responsible
> for caching the value. Rename the field to _Toolstack to deter code
> from using it.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PULL 0/19] xen-2015-09-08-tag

[Stefano Stabellini]

On Wed, 9 Sep 2015, Stefano Stabellini wrote:
> On Tue, 8 Sep 2015, Peter Maydell wrote:
> > On 8 September 2015 at 18:21, Stefano Stabellini
> >  wrote:
> > > The following changes since commit 
> > > 8611280505119e296757a60711a881341603fa5a:
> > >
> > >   target-microblaze: Use setcond for pcmp* (2015-09-08 08:49:33 +0200)
> > >
> > > are available in the git repository at:
> > >
> > >   git://xenbits.xen.org/people/sstabellini/qemu-dm.git 
> > > tags/xen-2015-09-08-tag
> > >
> > > for you to fetch changes up to ba2250ad148997b1352aba976aac66b55410e7e4:
> > >
> > >   xen/pt: Use XEN_PT_LOG properly to guard against compiler warnings. 
> > > (2015-09-08 15:21:56 +)
> > >
> > > 
> > > Xen branch xen-2015-09-08
> > >
> > > 
> > 
> > Hi. I'm afraid this fails to build on OSX (and probably Windows too,
> > though that build hasn't run yet):
> > 
> >   CCi386-softmmu/hw/i386/pci-assign-load-rom.o
> > /Users/pm215/src/qemu/hw/i386/pci-assign-load-rom.c:6:10: fatal error:
> >   'sys/io.h' file not found
> > #include 
> >  ^
> >   CCalpha-softmmu/hw/alpha/pci.o
> > 1 error generated.
> 
> Tiejun,
> 
> this is caused by 33d33242b7d802e6c994f3d56ecba96a66465dc3,
> "hw/pci-assign: split pci-assign.c". Could you please double-check
> non-Linux builds?

I found another issue introduced by the gfx passthrough series on
Windows:

../hw/pci-host/piix.o: In function `host_pci_config_read':
/root/qemu/hw/pci-host/piix.c:778: undefined reference to `_pread'

It is introduced by:

commit fdb70721ba0496a767137e5505dd27627d19c4a8
Author: Tiejun Chen 
Date:   Wed Jul 15 13:37:43 2015 +0800

piix: create host bridge to passthrough


You might have to replace the pread call with lseek and read.

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

Re: [Xen-devel] [PATCH v2] Remove XenPTReg->data and use dev.config for guest configuration values.

[Konrad Rzeszutek Wilk]

On Tue, Sep 08, 2015 at 06:22:13PM +0100, Stefano Stabellini wrote:
> Now that is fully Acked, could you please rebase on
> 
> http://marc.info/?i=alpine.DEB.2.02.1509081818590.2672%40kaball.uk.xensource.com
> 
> and resend?

I've rebased it (and put in your Reviewed-by tags) and stuck it in:

 git://xenbits.xen.org/people/konradwilk/qemu.git for-stefano-xen-2015-09-08-tag

and also tested it.


> Thanks!
> 
> - Stefano
> 
> On Tue, 8 Sep 2015, Konrad Rzeszutek Wilk wrote:
> > Hey!
> > 
> > Since v1: 
> > (http://lists.xen.org/archives/html/xen-devel/2015-07/msg00442.html)
> >  - Acked on review.
> > RFC [https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg07350.html]
> >  - Added Acks
> >  - Fixed bugs
> > 
> > This patchset is dependent on the "Cleanups + various fixes due to libxl ABI
> > more logging on errors" which is located at 
> > git://xenbits.xen.org/people/konradwilk/qemu.git 
> > v2-cleanups-fixes-due-to-libxlABI
> > (or http://lists.xen.org/archives/html/xen-devel/2015-09/msg00935.html)
> > 
> > The status of the patches is as follow:
> > 
> >  R xen/pt: Use xen_host_pci_get_[byte|word] instead of dev.config
> >  R xen/pt: Sync up the dev.config and data values.
> >xen/pt: Check if reg->init function sets the 'data' past the reg->size
> >xen/pt: Remove XenPTReg->data field.
> >  A xen/pt: Log xen_host_pci_get in two init functions
> >  R xen/pt: Log xen_host_pci_get/set errors in MSI code.
> >  R xen/pt: Make xen_pt_unregister_device idempotent
> >  A xen/pt: Move bulk of xen_pt_unregister_device in its own routine.
> >  A xen/pt: Check for return values for xen_host_pci_[get|set] in init
> >  R xen/pt: Don't slurp wholesale the PCI configuration registers
> > 
> > Where 'A' - Acked-by, 'R' - Reviewed-by.
> > 
> > The two patches:
> >  [PATCH v2 03/10] xen/pt: Check if reg->init function sets the 'data' past 
> > the reg->size
> > 
> > Stefano asked me to make this a build time check but I could not figure
> > out how. See 
> > http://lists.xen.org/archives/html/xen-devel/2015-08/msg01547.html for 
> > details.
> > 
> >  [PATCH v2 04/10] xen/pt: Remove XenPTReg->data field.
> > 
> > Stefano asked me to s/word/half-word/g s/dbword/word/ - which this does.
> > 
> > Please review.
> > 
> > The patches are also available at:
> > >From Konrad Rzeszutek Wilk  # This line is ignored.
> > From: Konrad Rzeszutek Wilk 
> > Subject: [PATCH v2]  Remove XenPTReg->data and use dev.config for guest 
> > configuration values.
> > In-Reply-To: 
> > 
> > Hey!
> > 
> > Since v1: 
> > (http://lists.xen.org/archives/html/xen-devel/2015-07/msg00442.html)
> >  - Acked on review.
> > RFC [https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg07350.html]
> >  - Added Acks
> >  - Fixed bugs
> > 
> > This patchset is dependent on the "Cleanups + various fixes due to libxl ABI
> > more logging on errors" which is located at 
> > git://xenbits.xen.org/people/konradwilk/qemu.git 
> > v2-cleanups-fixes-due-to-libxlABI
> > (or http://lists.xen.org/archives/html/xen-devel/2015-09/msg00935.html)
> > 
> > The status of the patches is as follow:
> > 
> >  R xen/pt: Use xen_host_pci_get_[byte|word] instead of dev.config
> >  R xen/pt: Sync up the dev.config and data values.
> >xen/pt: Check if reg->init function sets the 'data' past the reg->size
> >xen/pt: Remove XenPTReg->data field.
> >  A xen/pt: Log xen_host_pci_get in two init functions
> >  R xen/pt: Log xen_host_pci_get/set errors in MSI code.
> >  R xen/pt: Make xen_pt_unregister_device idempotent
> >  A xen/pt: Move bulk of xen_pt_unregister_device in its own routine.
> >  A xen/pt: Check for return values for xen_host_pci_[get|set] in init
> >  R xen/pt: Don't slurp wholesale the PCI configuration registers
> > 
> > Where 'A' - Acked-by, 'R' - Reviewed-by.
> > 
> > The two patches:
> >  [PATCH v2 03/10] xen/pt: Check if reg->init function sets the 'data' past 
> > the reg->size
> > 
> > Stefano asked me to make this a build time check but I could not figure
> > out how. See 
> > http://lists.xen.org/archives/html/xen-devel/2015-08/msg01547.html for 
> > details.
> > 
> >  [PATCH v2 04/10] xen/pt: Remove XenPTReg->data field.
> > 
> > Stefano asked me to s/word/half-word/g s/dbword/word/ - which this does.
> > 
> > Please review.
> > 
> > The patches are also available at:
> > 
> >  git://xenbits.xen.org/people/konradwilk/qemu.git postxsa120.v2
> > 
> > Thank you!
> > 
> > 
> >  hw/xen/xen-host-pci-device.c |   5 +
> >  hw/xen/xen-host-pci-device.h |   1 +
> >  hw/xen/xen_pt.c  | 152 +
> >  hw/xen/xen_pt.h  |   8 +-
> >  hw/xen/xen_pt_config_init.c  | 222 
> > ---
> >  hw/xen/xen_pt_msi.c  |  18 +++-
> >  6 files changed, 288 insertions(+), 118 deletions(-)
> > 
> > Konrad Rzeszutek Wilk (10):
> >   xen/pt: Use xen_host_pci_get_[byte|word] instead of dev.config
> >   xen/pt: Sync up the 

Re: [Xen-devel] [PATCH v6 31/31] xen/arm: ITS: Add pci devices in ThunderX

[Ian Campbell]

On Mon, 2015-08-31 at 16:36 +0530, vijay.kil...@gmail.com wrote:
> [...]
> +#define NUM_DEVIDS   54
> +
> +static struct pci_dev_list bdf[NUM_DEVIDS] =

FYI you could make this 
static struct pci_dev_list bdf[] =

and use ARRAY_SIZE(bdf) in the loops. That would save you having to
manually keep NUM_DEVIDS up to date and avoid any possibility of
miscounting.


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

Re: [Xen-devel] [PATCH] xen/domctl: lower loglevel of XEN_DOMCTL_memory_mapping

[Jan Beulich]

>>> On 09.09.15 at 17:19,  wrote:
> On 09/09/15 15:50, Konrad Rzeszutek Wilk wrote:
>> On Wed, Sep 09, 2015 at 08:33:52AM -0600, Jan Beulich wrote:
>> On 09.09.15 at 16:20,  wrote:
 Perhaps the solution is remove the first printk(s) and just have them
 once the operation has completed? That may fix the outstanding tasklet
 problem?
>>>
>>> Considering that this is a tool stack based retry, how would the
>>> hypervisor know when the _whole_ operation is done?
>> 
>> I was merely thinking of moving the printk _after_ the map_mmio_regions
>> so there wouldn't be any outstanding preemption points in map_mmio_regions
>> (so it can at least do the 64 PFNs).
>> 
>> But going forward a couple of ideas:
>> 
>>  - The 64 limit was arbitrary. It could have been 42 or PFNs / 
>> num_online_cpus(),
>>or actually finding out the size of the BAR and figuring the optimal
>>case so that it will be done under 1ms. Or perhaps just provide an
>>boot time parameter for those that really are struggling with this.
> 
> The issue of it taking a long time to map a large BAR is caused by the 
> unconditional
> memory_type_changed call at the bottom of XEN_DOMCTL_memory_mapping 
> function.
> 
> The memory_type_changed call results in a full data cache flush on VM with a 
> PCI device
> assigned.
> 
> We have seen this overhead cause a 1GB BAR to take 20 seconds to map it's 
> MMIO.
> 
> If the 64 limit was arbitrary then I would suggest increasing it to at least 
> 1024 so that
> at least 4M of BAR can be mapped in one go and it reduces the overhead by a 
> factor of 16.

1024 may be a little much, but 256 is certainly a possibility, plus
Konrad's suggestion to allow this limit to be controlled via command
line option.

> Currently the toolstack attempts lower and lower powers of 2 size's of the 
> MMIO region to be mapped
> until the hypercall succeeds.
> 
> Is it not clear to me why we have an unconditional call to 
> memory_type_changed in the domctl.
> Can somebody explain why it can't be made condition on errors?

You mean only on success? That may be possible (albeit as you
can see from the commit introducing it I wasn't sure, and I'm still
not sure), but wouldn't buy you anything.

Jan


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