Re: [Xen-devel] [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"
>>> On 11.02.16 at 20:41, wrote:
> On 11/02/16 19:25, Andrew Cooper wrote:
>> --- a/xen/arch/x86/flushtlb.c
>> +++ b/xen/arch/x86/flushtlb.c
>> @@ -141,10 +141,10 @@ void flush_area_local(const void *va, unsigned int
>> flags)
>> {
>> alternative(ASM_NOP3, "sfence", X86_FEATURE_CLFLUSHOPT);
>> for ( i = 0; i < sz; i += c->x86_clflush_size )
>> - alternative_input("rex clflush %0",
>> - "data16 clflush %0",
>> - X86_FEATURE_CLFLUSHOPT,
>> - "m" (((const char *)va)[i]));
>> +alternative_input(".byte 0x3e; clflush %0", /* %ds
>> override. */
>> + "data16 clflush %0", /* clflushopt.
> */
>> + X86_FEATURE_CLFLUSHOPT,
>> + "m" (((const char *)va)[i]));
>> }
>> else
>> {
>
> It turns out that Clang is far more useful at diagnosing this issue than
> GCC.
>
> flushtlb.c:144:18: error: invalid instruction mnemonic 'rex'
> alternative_input("rex clflush %0",
> ^
Except that 'rex' is by no means invalid. If anything Clang's internal
assembler doesn't support it (and hence is not gas compatible).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 3/7] xen/vm-events: Move monitor_domctl to common-side.
On 2/12/2016 8:05 AM, Corneliu ZUZU wrote: On 2/11/2016 5:44 PM, Tamas K Lengyel wrote: * the #ifdefs make it possible for that code to be put in common => that makes it *clear* that those code parts are NOT architecture specific and their implementation can be safely used for all architectures. The current practice has been to put empty static inline functions into architecture-specific headers if the part is not handled/implemented for the specific architecture. This has already been the case for monitor_domctl (there is already separate asm-arm/monitor.h and asm-x86/monitor.h) so it should be followed as more of the code moves into common. Tamas Point is, they *are* implemented, because that's *common* code, it doesn't make sense to be moved to the arch-side when you know that their implementation will be *the same* from arch-to-arch. Not *everything* needs to stay on the arch-side, just what is architecture-specific - that's why e.g. arch_hvm_event_fill_regs, arch_hvm_event_gfn_of_ip are not in common and are static inline functions as you say, because they have *different* implementations *depending on the architecture*. Finally, if Ian or any other ARM maintainer feels the same with moving code that can be moved and has been moved to common back on the arch-side (effectively undoing 50% of my efforts), I will do so. Corneliu. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel Actually, noted, will move single-step and software-breakpoint back on the arch-side as you suggest in v3. If someone else disagrees, I suppose it will be discussed then. Corneliu. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 4/6] xen: add capability to load initrd outside of initial mapping
Hey Juergen, On Fri, Feb 12, 2016 at 07:25:02AM +0100, Juergen Gross wrote: [...] > Okay, let me do some cleanup work on the xen loader: > > - add the possibility to call it multiple times (state reset, free the > allocated memory) > - merge all necessary global variables into one state structure, use > local variables where possible > - introduce lots of constants instead of using numerical values all over > the code Make sense for me. > The first two items should go in before my series. The third is more a > question of style, so it might block the complete series if I put it > early in the series and such a change isn't regarded to be positive. > Would you be okay if I put the constant introduction at the end of the > series? This would result in readable code (at least I hope so) in case OK, let's do that in that way. > it is accepted and wouldn't block the functionality in case it is > rejected. In case there is an early okay from the maintainer(s), I can > do the constant introduction early, too, of course. Granted! Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"
>>> On 11.02.16 at 20:25, wrote: > CentOS 7 gets into trouble when compiling Xen citing: > > flushtlb.c: Assembler messages: > flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1 > > The line number is wrong, and the error message not helpful. It turns out > that the intermediate generated assembly was > > # 139 "arch/x86/flushtlb.c" 1 > 661: > rex clflush (%r15) > 662: > .pushsection .altinstructions,"a" > > and it was having trouble combining the explicit REX prefix with the REX.B > required for the use of %r15. What gas version is this? I just checked with 2.20, which has no problem combining an explicit with a generated REX prefix. Or wait, no, your description of the issue is wrong: It actually is the folding of the two REX prefixes which causes the problem, since that results in the replacement instruction being one byte longer than the to be replaced one. > Follow what Linux does and use a redundant %ds prefix instead, for a final > generated instruction of `3e 41 0f ae 3f` > > While modifying this line, fix the indentation which was out by one space. > > Signed-off-by: Andrew Cooper I think before committing I'll extend your patch by introducing and using Linux'es NOP_DS_PREFIX. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
>>> On 12.02.16 at 04:08, wrote: > in the keyhandler.h file. Otherwise on ARM builds if we > just use the keyhandler file - the compile will fail. > > CC: ian.campb...@citrix.com > CC: wei.l...@citrix.com > CC: stefano.stabell...@citrix.com > Signed-off-by: Konrad Rzeszutek Wilk See commit d1d181328d. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] vm_event: Remove xc_mem_access_enable_emulate() and friends
xc_mem_access_enable_emulate() and xc_mem_access_disable_emulate()
are currently no-ops, that is all they do is set a flag that
nobody else checks. The user can already set the EMULATE flags in
the vm_event response if emulation is desired, and having an extra
check above that is not inherently safer, but it does complicate
(currenly unnecessarily) the API. This patch removes these
functions and the corresponding hypervisor code.
Signed-off-by: Razvan Cojocaru
---
tools/libxc/include/xenctrl.h | 11 ---
tools/libxc/xc_mem_access.c | 24
xen/common/mem_access.c | 8
xen/include/asm-arm/p2m.h | 14 --
xen/include/asm-x86/domain.h | 1 -
xen/include/asm-x86/p2m.h | 24
xen/include/public/memory.h | 2 --
7 files changed, 84 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 1a5f4ec..42eafa4 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2101,17 +2101,6 @@ int xc_set_mem_access(xc_interface *xch, domid_t
domain_id,
int xc_get_mem_access(xc_interface *xch, domid_t domain_id,
uint64_t pfn, xenmem_access_t *access);
-/*
- * Instructions causing a mem_access violation can be emulated by Xen
- * to progress the execution without having to relax the mem_access
- * permissions.
- * This feature has to be first enabled, then in the vm_event
- * response to a mem_access event it can be indicated if the instruction
- * should be emulated.
- */
-int xc_mem_access_enable_emulate(xc_interface *xch, domid_t domain_id);
-int xc_mem_access_disable_emulate(xc_interface *xch, domid_t domain_id);
-
/***
* Monitor control operations.
*
diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c
index 3634c39..eee088c 100644
--- a/tools/libxc/xc_mem_access.c
+++ b/tools/libxc/xc_mem_access.c
@@ -62,30 +62,6 @@ int xc_get_mem_access(xc_interface *xch,
return rc;
}
-int xc_mem_access_enable_emulate(xc_interface *xch,
- domid_t domain_id)
-{
-xen_mem_access_op_t mao =
-{
-.op = XENMEM_access_op_enable_emulate,
-.domid = domain_id,
-};
-
-return do_memory_op(xch, XENMEM_access_op, &mao, sizeof(mao));
-}
-
-int xc_mem_access_disable_emulate(xc_interface *xch,
- domid_t domain_id)
-{
-xen_mem_access_op_t mao =
-{
-.op = XENMEM_access_op_disable_emulate,
-.domid = domain_id,
-};
-
-return do_memory_op(xch, XENMEM_access_op, &mao, sizeof(mao));
-}
-
/*
* Local variables:
* mode: C
diff --git a/xen/common/mem_access.c b/xen/common/mem_access.c
index 159c036..0fb6699 100644
--- a/xen/common/mem_access.c
+++ b/xen/common/mem_access.c
@@ -98,14 +98,6 @@ int mem_access_memop(unsigned long cmd,
break;
}
-case XENMEM_access_op_enable_emulate:
-rc = p2m_mem_access_enable_emulate(d);
-break;
-
-case XENMEM_access_op_disable_emulate:
-rc = p2m_mem_access_disable_emulate(d);
-break;
-
default:
rc = -ENOSYS;
break;
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 4c62725..433952a 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -89,20 +89,6 @@ typedef enum {
} p2m_type_t;
static inline
-int p2m_mem_access_enable_emulate(struct domain *d)
-{
-/* Not supported on ARM */
-return -ENOSYS;
-}
-
-static inline
-int p2m_mem_access_disable_emulate(struct domain *d)
-{
-/* Not supported on ARM */
-return -ENOSYS;
-}
-
-static inline
void p2m_mem_access_emulate_check(struct vcpu *v,
const vm_event_response_t *rsp)
{
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 4072e27..4fad638 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -390,7 +390,6 @@ struct arch_domain
} monitor;
/* Mem_access emulation control */
-bool_t mem_access_emulate_enabled;
bool_t mem_access_emulate_each_rep;
/* Emulated devices enabled bitmap. */
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index fa46dd9..2a90491 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -647,30 +647,6 @@ bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
struct npfec npfec,
vm_event_request_t **req_ptr);
-/*
- * Emulating a memory access requires custom handling. These non-atomic
- * functions should be called under domctl lock.
- */
-static inline
-int p2m_mem_access_enable_emulate(struct domain *d)
-{
-if ( d->arch.mem_access_emulate_enabled )
-return -EEXIST;
-
-d->arch.mem_access_emulate_enabled = 1;
-return 0;
-}
-
-static inline
-int p2m_mem_access_disable_emulate(struct domain *d)
-{
-if ( !d->arch.mem_access_emulate_enabled )
-
Re: [Xen-devel] [PATCH v2 2/2] vm_event: consolidate hvm_event_fill_regs and p2m_vm_event_fill_regs
>>> On 12.02.16 at 01:22, wrote:
> --- a/xen/arch/x86/hvm/event.c
> +++ b/xen/arch/x86/hvm/event.c
> @@ -23,40 +23,9 @@
> #include
> #include
> #include
> +#include
> #include
>
> -static void hvm_event_fill_regs(vm_event_request_t *req)
> -{
> -const struct cpu_user_regs *regs = guest_cpu_user_regs();
> -const struct vcpu *curr = current;
> -
> -req->data.regs.x86.rax = regs->eax;
> -req->data.regs.x86.rcx = regs->ecx;
> -req->data.regs.x86.rdx = regs->edx;
> -req->data.regs.x86.rbx = regs->ebx;
> -req->data.regs.x86.rsp = regs->esp;
> -req->data.regs.x86.rbp = regs->ebp;
> -req->data.regs.x86.rsi = regs->esi;
> -req->data.regs.x86.rdi = regs->edi;
> -
> -req->data.regs.x86.r8 = regs->r8;
> -req->data.regs.x86.r9 = regs->r9;
> -req->data.regs.x86.r10 = regs->r10;
> -req->data.regs.x86.r11 = regs->r11;
> -req->data.regs.x86.r12 = regs->r12;
> -req->data.regs.x86.r13 = regs->r13;
> -req->data.regs.x86.r14 = regs->r14;
> -req->data.regs.x86.r15 = regs->r15;
> -
> -req->data.regs.x86.rflags = regs->eflags;
> -req->data.regs.x86.rip= regs->eip;
> -
> -req->data.regs.x86.msr_efer = curr->arch.hvm_vcpu.guest_efer;
> -req->data.regs.x86.cr0 = curr->arch.hvm_vcpu.guest_cr[0];
> -req->data.regs.x86.cr3 = curr->arch.hvm_vcpu.guest_cr[3];
> -req->data.regs.x86.cr4 = curr->arch.hvm_vcpu.guest_cr[4];
> -}
With this diff I suppose the patch here is meant to replace
"vm_event: Record FS_BASE/GS_BASE during events"? Such should
be made explicit, either by adding a note here (after the first ---
separator) or by explicitly withdrawing the other patch.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] vm_event: Remove xc_mem_access_enable_emulate() and friends
>>> On 12.02.16 at 10:04, wrote: > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -390,8 +390,6 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_paging_op_t); > #define XENMEM_access_op21 > #define XENMEM_access_op_set_access 0 > #define XENMEM_access_op_get_access 1 > -#define XENMEM_access_op_enable_emulate 2 > -#define XENMEM_access_op_disable_emulate3 Considering that the interface isn't versioned, I think you should just comment those out instead of entirely removing them, to make obvious for the next one to add an operation here that these two values may not be re-used. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] hvm/vmx: save dr7 during vmx_vmcs_save
>>> On 12.02.16 at 01:22, wrote: > Sending the dr7 register during vm_events is useful for various applications, > but the current way the register value is gathered is incorrent. In this > patch > we extend vmx_vmcs_save so that we get the correct value. > > Suggested-by: Andrew Cooper Iirc Andrew suggested ... > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -490,6 +490,7 @@ static void vmx_vmcs_save(struct vcpu *v, struct > hvm_hw_cpu *c) > __vmread(GUEST_SYSENTER_CS, &c->sysenter_cs); > __vmread(GUEST_SYSENTER_ESP, &c->sysenter_esp); > __vmread(GUEST_SYSENTER_EIP, &c->sysenter_eip); > +__vmread(GUEST_DR7, &c->dr7); ... just when v == current. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] vm_event: Remove xc_mem_access_enable_emulate() and friends
On 02/12/2016 11:10 AM, Jan Beulich wrote: On 12.02.16 at 10:04, wrote: >> --- a/xen/include/public/memory.h >> +++ b/xen/include/public/memory.h >> @@ -390,8 +390,6 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_paging_op_t); >> #define XENMEM_access_op21 >> #define XENMEM_access_op_set_access 0 >> #define XENMEM_access_op_get_access 1 >> -#define XENMEM_access_op_enable_emulate 2 >> -#define XENMEM_access_op_disable_emulate3 > > Considering that the interface isn't versioned, I think you should > just comment those out instead of entirely removing them, to > make obvious for the next one to add an operation here that > these two values may not be re-used. Right, I'll do that in V2. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/common: Uniformally use __ varients for attribute names
>>> On 11.02.16 at 20:51, wrote: > Otherwise, debug code such as "void __attribute__((noreturn)) foobar()" fails > to compile when the noreturn itself gets expanded, resulting in > __attribute__((__attribute__((noreturn. Well, why would the debugging code not use plain "noreturn" then, instead of open coding its expansion? That notwithstanding the patch is fine of course. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/4] xen/pciback: Check PF instead of VF for PCI_COMMAND_MEMORY
>>> On 11.02.16 at 22:10, wrote: > c/s 408fb0e5aa7fda0059db282ff58c3b2a4278baa0 > "xen/pciback: Don't allow MSI-X ops if PCI_COMMAND_MEMORY is not set." > would check the device for PCI_COMMAND_MEMORY which is great. > Except that VF devices are unique - for example they have no > legacy interrupts, and also any writes to PCI_COMMAND_MEMORY > are silently ignored (by the hardware). > > CC: sta...@vger.kernel.org > Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Jan Beulich albeit ... > --- a/drivers/xen/xen-pciback/pciback_ops.c > +++ b/drivers/xen/xen-pciback/pciback_ops.c > @@ -213,6 +213,7 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev, > int i, result; > struct msix_entry *entries; > u16 cmd; > + struct pci_dev *phys_dev; > > if (unlikely(verbose_request)) > printk(KERN_DEBUG DRV_NAME ": %s: enable MSI-X\n", > @@ -227,8 +228,10 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev, > /* >* PCI_COMMAND_MEMORY must be enabled, otherwise we may not be able >* to access the BARs where the MSI-X entries reside. > + * But VF devices are unique in which the PF needs to be checked. >*/ > - pci_read_config_word(dev, PCI_COMMAND, &cmd); > + phys_dev = pci_physfn(dev); > + pci_read_config_word(phys_dev, PCI_COMMAND, &cmd); ... I don't think the use of a local variable here is really needed. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/4] xen/pciback: Save the number of MSI-X entries to be copied later.
>>> On 11.02.16 at 22:10, wrote: > c/s 8135cf8b092723dbfcc611fe6fdcb3a36c9951c5 > "xen/pciback: Save xen_pci_op commands before processing it" > would copyback the processed values - which was great. > > However it missed the case that xen_pcibk_enable_msix - when > completing would overwrite op->value (which had the number > of MSI-X vectors requested) with the return value (which for > success was zero). Hence the copy-back routine (which would use > op->value) would copy exactly zero MSI-X vectors back. > > CC: sta...@vger.kernel.org > Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Jan Beulich but I'd prefer to see ... > --- a/drivers/xen/xen-pciback/pciback_ops.c > +++ b/drivers/xen/xen-pciback/pciback_ops.c > @@ -335,7 +335,9 @@ void xen_pcibk_do_op(struct work_struct *data) > struct xen_pcibk_dev_data *dev_data = NULL; > struct xen_pci_op *op = &pdev->op; > int test_intx = 0; > - > +#ifdef CONFIG_PCI_MSI > + unsigned int nr = 0; > +#endif > *op = pdev->sh_info->op; > barrier(); > dev = xen_pcibk_get_pci_dev(pdev, op->domain, op->bus, op->devfn); ... the blank line separating declarations from statements to stay. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 1/2] xen: credit1: trace vCPU boost/unboost
Add tracepoints and a performance counter for
boosting and unboosting in Credit1.
Note that they (the trace points) do not cover
the case of the idle vCPU being boosted to run
a tasklet, as there already is
TRC_CSCHED_SCHED_TASKLET for that.
Signed-off-by: Dario Faggioli
---
Cc: George Dunlap
---
xen/common/sched_credit.c|8
xen/include/xen/perfc_defn.h |1 +
2 files changed, 9 insertions(+)
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 671bbee..5708701 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -126,6 +126,8 @@
#define TRC_CSCHED_STOLEN_VCPU TRC_SCHED_CLASS_EVT(CSCHED, 4)
#define TRC_CSCHED_PICKED_CPUTRC_SCHED_CLASS_EVT(CSCHED, 5)
#define TRC_CSCHED_TICKLETRC_SCHED_CLASS_EVT(CSCHED, 6)
+#define TRC_CSCHED_BOOST_START TRC_SCHED_CLASS_EVT(CSCHED, 7)
+#define TRC_CSCHED_BOOST_END TRC_SCHED_CLASS_EVT(CSCHED, 8)
/*
@@ -856,7 +858,11 @@ csched_vcpu_acct(struct csched_private *prv, unsigned int
cpu)
* amount of CPU resources and should no longer be boosted.
*/
if ( svc->pri == CSCHED_PRI_TS_BOOST )
+{
svc->pri = CSCHED_PRI_TS_UNDER;
+TRACE_2D(TRC_CSCHED_BOOST_END, svc->sdom->dom->domain_id,
+ svc->vcpu->vcpu_id);
+}
/*
* Update credits
@@ -1023,6 +1029,8 @@ csched_vcpu_wake(const struct scheduler *ops, struct vcpu
*vc)
if ( svc->pri == CSCHED_PRI_TS_UNDER &&
!test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
{
+TRACE_2D(TRC_CSCHED_BOOST_START, vc->domain->domain_id, vc->vcpu_id);
+SCHED_STAT_CRANK(vcpu_boost);
svc->pri = CSCHED_PRI_TS_BOOST;
}
diff --git a/xen/include/xen/perfc_defn.h b/xen/include/xen/perfc_defn.h
index 76ee803..21c1e0b 100644
--- a/xen/include/xen/perfc_defn.h
+++ b/xen/include/xen/perfc_defn.h
@@ -40,6 +40,7 @@ PERFCOUNTER(acct_reorder, "csched: acct_reorder")
PERFCOUNTER(acct_min_credit,"csched: acct_min_credit")
PERFCOUNTER(acct_vcpu_active, "csched: acct_vcpu_active")
PERFCOUNTER(acct_vcpu_idle, "csched: acct_vcpu_idle")
+PERFCOUNTER(vcpu_boost, "csched: vcpu_boost")
PERFCOUNTER(vcpu_park, "csched: vcpu_park")
PERFCOUNTER(vcpu_unpark,"csched: vcpu_unpark")
PERFCOUNTER(load_balance_idle, "csched: load_balance_idle")
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 0/2] xen: sched: Credit1 shouldn't boost vcpus being migrated.
Hi again, Here it comes v2, redone following Jan's suggestion, which allowed to get rid of patch 2, and do everything in sched_credit.c. So, in summary, because of the fact that vcpu_migrate() forces the vcpus into a sleep+wakeup cycle, vcpus being migrated to a new pcpu, were also being granted BOOST priority, inside Credit1, and that is not correct. More info on v1's cover letter, which is here: http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg01620.html I re-run the same set of benchmarks described in v1, and the result for this rework of the series are basically the same as there. Thanks and Regards, Dario --- Dario Faggioli (2): xen: credit1: trace vCPU boost/unboost xen: credit1: avoid boosting vCPUs being "just" migrated xen/common/sched_credit.c| 34 ++ xen/include/xen/perfc_defn.h |1 + 2 files changed, 31 insertions(+), 4 deletions(-) -- <> (Raistlin Majere) --- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] Anyone know where the official qemu(1) man pages are hosted?
I noticed that the qemu(1) links in http://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html (and probably other docs) are broken. They go to http://man.he.net/man1/qemu, which does not exist any more Lars ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 2/2] xen: credit1: avoid boosting vCPUs being "just" migrated
Moving a vCPU to a different pCPU means offlining it and
then waking it up, on the new pCPU. Credit1 grants BOOST
priority to vCPUs that wakes up, with the aim of improving
I/O latency. The net effect of this all is that vCPUs get
boosted when migrating, which shouldn't happen.
For instance, this causes scheduling anomalies and,
potentially, performance problems, as reported here:
http://lists.xen.org/archives/html/xen-devel/2015-10/msg02851.html
This patch fixes this by noting down (by means of a flag)
the fact that the vCPU is about to undergo a migration.
This way we can tell, later, during a wakeup, whether the
vCPU is migrating or unblocking, and decide whether or
not to apply the boosting.
Signed-off-by: Dario Faggioli
---
Cc: George Dunlap
Cc: Jan Beulich
---
Changes from v1:
* rewritten, following suggestion got during review: there
are no wakeup flags any longer, and all is done in sched_credit.c
by setting a flag in csched_cpu_pick() and testing (and
cleating) it in csched_vcpu_wake().
---
xen/common/sched_credit.c | 26 ++
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 5708701..756e884 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -66,6 +66,7 @@
*/
#define CSCHED_FLAG_VCPU_PARKED0x0 /* VCPU over capped credits */
#define CSCHED_FLAG_VCPU_YIELD 0x1 /* VCPU yielding */
+#define CSCHED_FLAG_VCPU_MIGRATING 0x2 /* VCPU may have moved to a new pcpu */
/*
@@ -787,6 +788,16 @@ _csched_cpu_pick(const struct scheduler *ops, struct vcpu
*vc, bool_t commit)
static int
csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc)
{
+struct csched_vcpu *svc = CSCHED_VCPU(vc);
+
+/*
+ * We have been called by vcpu_migrate() (in schedule.c), as part
+ * of the process of seeing if vc can be migrated to another pcpu.
+ * We make a note about this in svc->flags so that later, in
+ * csched_vcpu_wake() (still called from vcpu_migrate()) we won't
+ * get boosted, which we don't deserve as we are "only" migrating.
+ */
+set_bit(CSCHED_FLAG_VCPU_MIGRATING, &svc->flags);
return _csched_cpu_pick(ops, vc, 1);
}
@@ -1022,11 +1033,18 @@ csched_vcpu_wake(const struct scheduler *ops, struct
vcpu *vc)
* more CPU resource intensive VCPUs without impacting overall
* system fairness.
*
- * The one exception is for VCPUs of capped domains unpausing
- * after earning credits they had overspent. We don't boost
- * those.
+ * There are two cases, when we don't want to boost:
+ * - VCPUs that are waking up after a migration, rather than
+ *after having block;
+ * - VCPUs of capped domains unpausing after earning credits
+ *they had overspent.
+ *
+ * Note that checking whether we are "only" migrating must be
+ * done up front, as we do not want the clearing of the bit we
+ * set in csched_cpu_pick() to be short-circuited away.
*/
-if ( svc->pri == CSCHED_PRI_TS_UNDER &&
+if ( !__test_and_clear_bit(CSCHED_FLAG_VCPU_MIGRATING, &svc->flags) &&
+ svc->pri == CSCHED_PRI_TS_UNDER &&
!test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
{
TRACE_2D(TRC_CSCHED_BOOST_START, vc->domain->domain_id, vc->vcpu_id);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"
On 12/02/16 08:13, Jan Beulich wrote:
On 11.02.16 at 20:41, wrote:
>> On 11/02/16 19:25, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/flushtlb.c
>>> +++ b/xen/arch/x86/flushtlb.c
>>> @@ -141,10 +141,10 @@ void flush_area_local(const void *va, unsigned int
>>> flags)
>>> {
>>> alternative(ASM_NOP3, "sfence", X86_FEATURE_CLFLUSHOPT);
>>> for ( i = 0; i < sz; i += c->x86_clflush_size )
>>> - alternative_input("rex clflush %0",
>>> - "data16 clflush %0",
>>> - X86_FEATURE_CLFLUSHOPT,
>>> - "m" (((const char *)va)[i]));
>>> +alternative_input(".byte 0x3e; clflush %0", /* %ds
>>> override. */
>>> + "data16 clflush %0", /* clflushopt.
>>>
>> */
>>> + X86_FEATURE_CLFLUSHOPT,
>>> + "m" (((const char *)va)[i]));
>>> }
>>> else
>>> {
>> It turns out that Clang is far more useful at diagnosing this issue than
>> GCC.
>>
>> flushtlb.c:144:18: error: invalid instruction mnemonic 'rex'
>> alternative_input("rex clflush %0",
>> ^
> Except that 'rex' is by no means invalid. If anything Clang's internal
> assembler doesn't support it (and hence is not gas compatible).
That is tangential to the point. The useful part is the end message:
> :2:2: note: instantiated into assembly here
> rex clflush (%r15,%rdx)
> ^~~
which makes it substantially more clear that there is both a REX prefix
and REX.B required.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] xen: credit1: avoid boosting vCPUs being "just" migrated
>>> On 12.02.16 at 10:37, wrote:
> @@ -787,6 +788,16 @@ _csched_cpu_pick(const struct scheduler *ops, struct
> vcpu *vc, bool_t commit)
> static int
> csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc)
> {
> +struct csched_vcpu *svc = CSCHED_VCPU(vc);
> +
> +/*
> + * We have been called by vcpu_migrate() (in schedule.c), as part
> + * of the process of seeing if vc can be migrated to another pcpu.
> + * We make a note about this in svc->flags so that later, in
> + * csched_vcpu_wake() (still called from vcpu_migrate()) we won't
> + * get boosted, which we don't deserve as we are "only" migrating.
> + */
> +set_bit(CSCHED_FLAG_VCPU_MIGRATING, &svc->flags);
> return _csched_cpu_pick(ops, vc, 1);
> }
I think you either want __set_bit() here or ...
> @@ -1022,11 +1033,18 @@ csched_vcpu_wake(const struct scheduler *ops, struct
> vcpu *vc)
> * more CPU resource intensive VCPUs without impacting overall
> * system fairness.
> *
> - * The one exception is for VCPUs of capped domains unpausing
> - * after earning credits they had overspent. We don't boost
> - * those.
> + * There are two cases, when we don't want to boost:
> + * - VCPUs that are waking up after a migration, rather than
> + *after having block;
> + * - VCPUs of capped domains unpausing after earning credits
> + *they had overspent.
> + *
> + * Note that checking whether we are "only" migrating must be
> + * done up front, as we do not want the clearing of the bit we
> + * set in csched_cpu_pick() to be short-circuited away.
> */
> -if ( svc->pri == CSCHED_PRI_TS_UNDER &&
> +if ( !__test_and_clear_bit(CSCHED_FLAG_VCPU_MIGRATING, &svc->flags) &&
> + svc->pri == CSCHED_PRI_TS_UNDER &&
> !test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
> {
... you ought to use test_and_clear_bit() here.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"
On 12/02/16 08:23, Jan Beulich wrote: On 11.02.16 at 20:25, wrote: >> CentOS 7 gets into trouble when compiling Xen citing: >> >> flushtlb.c: Assembler messages: >> flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1 >> >> The line number is wrong, and the error message not helpful. It turns out >> that the intermediate generated assembly was >> >> # 139 "arch/x86/flushtlb.c" 1 >> 661: >> rex clflush (%r15) >> 662: >> .pushsection .altinstructions,"a" >> >> and it was having trouble combining the explicit REX prefix with the REX.B >> required for the use of %r15. > What gas version is this? I just checked with 2.20, which has no > problem combining an explicit with a generated REX prefix. bash-4.2$ as --version GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226 > Or > wait, no, your description of the issue is wrong: It actually is the > folding of the two REX prefixes which causes the problem, This is what I said. What do you think I meant with "trouble combining the" ? > since > that results in the replacement instruction being one byte longer > than the to be replaced one. But that is still the case with an explicit %ds override. The assembler still has to insert an extra byte somehow. > >> Follow what Linux does and use a redundant %ds prefix instead, for a final >> generated instruction of `3e 41 0f ae 3f` >> >> While modifying this line, fix the indentation which was out by one space. >> >> Signed-off-by: Andrew Cooper > I think before committing I'll extend your patch by introducing > and using Linux'es NOP_DS_PREFIX. Ok. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] vm_event: consolidate hvm_event_fill_regs and p2m_vm_event_fill_regs
>>> On 12.02.16 at 01:22, wrote:
> --- a/xen/arch/x86/vm_event.c
> +++ b/xen/arch/x86/vm_event.c
> @@ -122,6 +122,65 @@ void vm_event_set_registers(struct vcpu *v,
> vm_event_response_t *rsp)
> v->arch.user_regs.eip = rsp->data.regs.x86.rip;
> }
>
> +void vm_event_fill_regs(vm_event_request_t *req)
> +{
> +const struct cpu_user_regs *regs = guest_cpu_user_regs();
> +struct segment_register seg;
> +struct hvm_hw_cpu ctxt;
> +struct vcpu *curr = current;
> +
> +req->data.regs.x86.rax = regs->eax;
> +req->data.regs.x86.rcx = regs->ecx;
> +req->data.regs.x86.rdx = regs->edx;
> +req->data.regs.x86.rbx = regs->ebx;
> +req->data.regs.x86.rsp = regs->esp;
> +req->data.regs.x86.rbp = regs->ebp;
> +req->data.regs.x86.rsi = regs->esi;
> +req->data.regs.x86.rdi = regs->edi;
> +
> +req->data.regs.x86.r8 = regs->r8;
> +req->data.regs.x86.r9 = regs->r9;
> +req->data.regs.x86.r10 = regs->r10;
> +req->data.regs.x86.r11 = regs->r11;
> +req->data.regs.x86.r12 = regs->r12;
> +req->data.regs.x86.r13 = regs->r13;
> +req->data.regs.x86.r14 = regs->r14;
> +req->data.regs.x86.r15 = regs->r15;
> +
> +req->data.regs.x86.rflags = regs->eflags;
> +req->data.regs.x86.rip= regs->eip;
> +
> +if ( !is_hvm_domain(curr->domain) )
> +return;
No such check existed in either of the two original functions. Why is
it needed all of the sudden? And if it is needed, why do the other
fields not get filled (as far as possible at least) for PV guests?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/common: Uniformally use __ varients for attribute names
On 12/02/16 09:16, Jan Beulich wrote: On 11.02.16 at 20:51, wrote: >> Otherwise, debug code such as "void __attribute__((noreturn)) foobar()" fails >> to compile when the noreturn itself gets expanded, resulting in >> __attribute__((__attribute__((noreturn. > Well, why would the debugging code not use plain "noreturn" then, > instead of open coding its expansion? That notwithstanding the > patch is fine of course. Which one works depends on whether has been included. There are some translation units in Xen where this is not the case. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"
>>> On 12.02.16 at 10:51, wrote: > On 12/02/16 08:23, Jan Beulich wrote: > On 11.02.16 at 20:25, wrote: >>> CentOS 7 gets into trouble when compiling Xen citing: >>> >>> flushtlb.c: Assembler messages: >>> flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1 >>> >>> The line number is wrong, and the error message not helpful. It turns out >>> that the intermediate generated assembly was >>> >>> # 139 "arch/x86/flushtlb.c" 1 >>> 661: >>> rex clflush (%r15) >>> 662: >>> .pushsection .altinstructions,"a" >>> >>> and it was having trouble combining the explicit REX prefix with the REX.B >>> required for the use of %r15. >> What gas version is this? I just checked with 2.20, which has no >> problem combining an explicit with a generated REX prefix. > > bash-4.2$ as --version > GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226 > > >> Or >> wait, no, your description of the issue is wrong: It actually is the >> folding of the two REX prefixes which causes the problem, > > This is what I said. What do you think I meant with "trouble combining > the" ? Argh - I meant to say "It actually isn't ...". >> since >> that results in the replacement instruction being one byte longer >> than the to be replaced one. > > But that is still the case with an explicit %ds override. The assembler > still has to insert an extra byte somehow. No. We now always have one non-REX prefix, and both instructions will have the same REX/ModRM/SIB encoding. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"
On 12/02/16 10:00, Jan Beulich wrote: On 12.02.16 at 10:51, wrote: >> On 12/02/16 08:23, Jan Beulich wrote: >> On 11.02.16 at 20:25, wrote: CentOS 7 gets into trouble when compiling Xen citing: flushtlb.c: Assembler messages: flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1 The line number is wrong, and the error message not helpful. It turns out that the intermediate generated assembly was # 139 "arch/x86/flushtlb.c" 1 661: rex clflush (%r15) 662: .pushsection .altinstructions,"a" and it was having trouble combining the explicit REX prefix with the REX.B required for the use of %r15. >>> What gas version is this? I just checked with 2.20, which has no >>> problem combining an explicit with a generated REX prefix. >> bash-4.2$ as --version >> GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226 >> >> >>> Or >>> wait, no, your description of the issue is wrong: It actually is the >>> folding of the two REX prefixes which causes the problem, >> This is what I said. What do you think I meant with "trouble combining >> the" ? > Argh - I meant to say "It actually isn't ...". > >>> since >>> that results in the replacement instruction being one byte longer >>> than the to be replaced one. >> But that is still the case with an explicit %ds override. The assembler >> still has to insert an extra byte somehow. > No. We now always have one non-REX prefix, and both instructions > will have the same REX/ModRM/SIB encoding. I see now, given your wording on the patch committed. In hindsight this should have been obvious, but GCCs error message was particularly unhelpful at diagnosing the issue. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"
>>> On 12.02.16 at 11:02, wrote: > On 12/02/16 10:00, Jan Beulich wrote: > On 12.02.16 at 10:51, wrote: >>> On 12/02/16 08:23, Jan Beulich wrote: >>> On 11.02.16 at 20:25, wrote: > CentOS 7 gets into trouble when compiling Xen citing: > > flushtlb.c: Assembler messages: > flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1 > > The line number is wrong, and the error message not helpful. It turns out > that the intermediate generated assembly was > > # 139 "arch/x86/flushtlb.c" 1 > 661: > rex clflush (%r15) > 662: > .pushsection .altinstructions,"a" > > and it was having trouble combining the explicit REX prefix with the REX.B > required for the use of %r15. What gas version is this? I just checked with 2.20, which has no problem combining an explicit with a generated REX prefix. >>> bash-4.2$ as --version >>> GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226 >>> >>> Or wait, no, your description of the issue is wrong: It actually is the folding of the two REX prefixes which causes the problem, >>> This is what I said. What do you think I meant with "trouble combining >>> the" ? >> Argh - I meant to say "It actually isn't ...". >> since that results in the replacement instruction being one byte longer than the to be replaced one. >>> But that is still the case with an explicit %ds override. The assembler >>> still has to insert an extra byte somehow. >> No. We now always have one non-REX prefix, and both instructions >> will have the same REX/ModRM/SIB encoding. > > I see now, given your wording on the patch committed. > > In hindsight this should have been obvious, but GCCs error message was > particularly unhelpful at diagnosing the issue. It was actually an assembler error message, and I can't really see how we could improve that (since afaict the intended checking can only be done at assembly time). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] vm_event: consolidate hvm_event_fill_regs and p2m_vm_event_fill_regs
On 02/12/2016 11:57 AM, Jan Beulich wrote:
On 12.02.16 at 01:22, wrote:
>> --- a/xen/arch/x86/vm_event.c
>> +++ b/xen/arch/x86/vm_event.c
>> @@ -122,6 +122,65 @@ void vm_event_set_registers(struct vcpu *v,
>> vm_event_response_t *rsp)
>> v->arch.user_regs.eip = rsp->data.regs.x86.rip;
>> }
>>
>> +void vm_event_fill_regs(vm_event_request_t *req)
>> +{
>> +const struct cpu_user_regs *regs = guest_cpu_user_regs();
>> +struct segment_register seg;
>> +struct hvm_hw_cpu ctxt;
>> +struct vcpu *curr = current;
>> +
>> +req->data.regs.x86.rax = regs->eax;
>> +req->data.regs.x86.rcx = regs->ecx;
>> +req->data.regs.x86.rdx = regs->edx;
>> +req->data.regs.x86.rbx = regs->ebx;
>> +req->data.regs.x86.rsp = regs->esp;
>> +req->data.regs.x86.rbp = regs->ebp;
>> +req->data.regs.x86.rsi = regs->esi;
>> +req->data.regs.x86.rdi = regs->edi;
>> +
>> +req->data.regs.x86.r8 = regs->r8;
>> +req->data.regs.x86.r9 = regs->r9;
>> +req->data.regs.x86.r10 = regs->r10;
>> +req->data.regs.x86.r11 = regs->r11;
>> +req->data.regs.x86.r12 = regs->r12;
>> +req->data.regs.x86.r13 = regs->r13;
>> +req->data.regs.x86.r14 = regs->r14;
>> +req->data.regs.x86.r15 = regs->r15;
>> +
>> +req->data.regs.x86.rflags = regs->eflags;
>> +req->data.regs.x86.rip= regs->eip;
>> +
>> +if ( !is_hvm_domain(curr->domain) )
>> +return;
>
> No such check existed in either of the two original functions. Why is
> it needed all of the sudden? And if it is needed, why do the other
> fields not get filled (as far as possible at least) for PV guests?
I can't speak for Tamas, but I suspect the check has been placed there
because calls to hvm_funcs.save_cpu_ctxt(curr, &ctxt) and
hvm_get_segment_register(curr, x86_seg_fs, &seg) follow, and he's put
vm_event_fill_regs() in xen/arch/x86/vm_event.c (a previous function was
called hvm_event_fill_regs(), in arch/x86/hvm/event.c, so no checking
for HVM was needed).
I don't think the check is needed for the current codepaths, but since
the code has been moved to xen/arch/x86/ the question about future PV
events is fair.
Thanks,
Razvan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH net-next v1 1/8] skbuff: store hash type in socket buffer...
...rather than a boolean merely indicating a canonical L4 hash.
skb_set_hash() takes a hash type (from enum pkt_hash_types) as an
argument but information is lost since only a single bit in the skb
stores whether that hash type is PKT_HASH_TYPE_L4 or not.
By using two bits it's possible to store the complete hash type
information for use by drivers. A subsequent patch to xen-netback
needs this information to forward packet hashes to VM frontend drivers.
Signed-off-by: Paul Durrant
Cc: "David S. Miller"
Cc: Jay Vosburgh
Cc: Veaceslav Falico
Cc: Andy Gospodarek
---
drivers/net/bonding/bond_main.c | 2 +-
include/linux/skbuff.h | 53 -
include/net/flow_dissector.h| 5
include/net/sock.h | 2 +-
include/trace/events/net.h | 2 +-
net/core/flow_dissector.c | 27 -
6 files changed, 65 insertions(+), 26 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 45bdd87..ad0ee00 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3173,7 +3173,7 @@ u32 bond_xmit_hash(struct bonding *bond, struct sk_buff
*skb)
u32 hash;
if (bond->params.xmit_policy == BOND_XMIT_POLICY_ENCAP34 &&
- skb->l4_hash)
+ skb_has_l4_hash(skb))
return skb->hash;
if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER2 ||
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6ec86f1..9021b52 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -595,8 +595,7 @@ static inline bool skb_mstamp_after(const struct skb_mstamp
*t1,
* @xmit_more: More SKBs are pending for this queue
* @ndisc_nodetype: router type (from link layer)
* @ooo_okay: allow the mapping of a socket to a queue to be changed
- * @l4_hash: indicate hash is a canonical 4-tuple hash over transport
- * ports.
+ * @hash_type: indicates type of hash (see enum pkt_hash_types below)
* @sw_hash: indicates hash was computed in software stack
* @wifi_acked_valid: wifi_acked was set
* @wifi_acked: whether frame was acked on wifi or not
@@ -701,10 +700,10 @@ struct sk_buff {
__u8nf_trace:1;
__u8ip_summed:2;
__u8ooo_okay:1;
- __u8l4_hash:1;
__u8sw_hash:1;
__u8wifi_acked_valid:1;
__u8wifi_acked:1;
+ /* 1 bit hole */
__u8no_fcs:1;
/* Indicates the inner headers are valid in the skbuff. */
@@ -721,7 +720,8 @@ struct sk_buff {
__u8ipvs_property:1;
__u8inner_protocol_type:1;
__u8remcsum_offload:1;
- /* 3 or 5 bit hole */
+ __u8hash_type:2;
+ /* 1 or 3 bit hole */
#ifdef CONFIG_NET_SCHED
__u16 tc_index; /* traffic control index */
@@ -1030,19 +1030,35 @@ static inline void skb_clear_hash(struct sk_buff *skb)
{
skb->hash = 0;
skb->sw_hash = 0;
- skb->l4_hash = 0;
+ skb->hash_type = 0;
+}
+
+static inline enum pkt_hash_types skb_hash_type(struct sk_buff *skb)
+{
+ return skb->hash_type;
+}
+
+static inline bool skb_has_l4_hash(struct sk_buff *skb)
+{
+ return skb_hash_type(skb) == PKT_HASH_TYPE_L4;
+}
+
+static inline bool skb_has_sw_hash(struct sk_buff *skb)
+{
+ return !!skb->sw_hash;
}
static inline void skb_clear_hash_if_not_l4(struct sk_buff *skb)
{
- if (!skb->l4_hash)
+ if (!skb_has_l4_hash(skb))
skb_clear_hash(skb);
}
static inline void
-__skb_set_hash(struct sk_buff *skb, __u32 hash, bool is_sw, bool is_l4)
+__skb_set_hash(struct sk_buff *skb, __u32 hash, bool is_sw,
+ enum pkt_hash_types type)
{
- skb->l4_hash = is_l4;
+ skb->hash_type = type;
skb->sw_hash = is_sw;
skb->hash = hash;
}
@@ -1051,13 +1067,13 @@ static inline void
skb_set_hash(struct sk_buff *skb, __u32 hash, enum pkt_hash_types type)
{
/* Used by drivers to set hash from HW */
- __skb_set_hash(skb, hash, false, type == PKT_HASH_TYPE_L4);
+ __skb_set_hash(skb, hash, false, type);
}
static inline void
-__skb_set_sw_hash(struct sk_buff *skb, __u32 hash, bool is_l4)
+__skb_set_sw_hash(struct sk_buff *skb, __u32 hash, enum pkt_hash_types type)
{
- __skb_set_hash(skb, hash, true, is_l4);
+ __skb_set_hash(skb, hash, true, type);
}
void __skb_get_hash(struct sk_buff *skb);
@@ -1110,9 +1126,10 @@ static inline bool skb_flow_dissect_flow_keys_buf(struct
flow_keys *flow,
data, proto, nhoff, hlen, flags);
}
+
static inline __u32 skb_get_hash(struct sk_buff *skb)
{
- if (!skb->l4_hash && !skb->sw_hash)
+
[Xen-devel] [PATCH net-next v1 6/8] xen-netback: add an implementation of toeplitz hashing...
...for receive-side packets.
My recent patch to include/xen/interface/io/netif.h defines a set of
control messages that can be used by a VM frontend driver to configure
toeplitz hashing of receive-side packets and consequent steering of those
packets to particular queues.
This patch introduces an implementation of toeplitz hashing and into
xen-netback and allows it to be configured using the new control messages.
Signed-off-by: Paul Durrant
Cc: Ian Campbell
Cc: Wei Liu
---
drivers/net/xen-netback/common.h| 13
drivers/net/xen-netback/interface.c | 149
drivers/net/xen-netback/netback.c | 128 ++-
3 files changed, 287 insertions(+), 3 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 093a12a..6687702 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -220,6 +220,12 @@ struct xenvif_mcast_addr {
#define XEN_NETBK_MCAST_MAX 64
+#define XEN_NETBK_MAX_TOEPLITZ_KEY_SIZE 40
+
+#define XEN_NETBK_MAX_TOEPLITZ_MAPPING_ORDER 7
+#define XEN_NETBK_MAX_TOEPLITZ_MAPPING_SIZE \
+ BIT(XEN_NETBK_MAX_TOEPLITZ_MAPPING_ORDER)
+
struct xenvif {
/* Unique identifier for this interface. */
domid_t domid;
@@ -251,6 +257,13 @@ struct xenvif {
unsigned int num_queues; /* active queues, resource allocated */
unsigned int stalled_queues;
+ struct {
+ u32 flags;
+ u8 key[XEN_NETBK_MAX_TOEPLITZ_KEY_SIZE];
+ u32 mapping[XEN_NETBK_MAX_TOEPLITZ_MAPPING_SIZE];
+ unsigned int order;
+ } toeplitz;
+
struct xenbus_watch credit_watch;
struct xenbus_watch mcast_ctrl_watch;
diff --git a/drivers/net/xen-netback/interface.c
b/drivers/net/xen-netback/interface.c
index 1850ebb..230afde 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -1,3 +1,4 @@
+
/*
* Network-device interface management.
*
@@ -151,6 +152,153 @@ void xenvif_wake_queue(struct xenvif_queue *queue)
netif_tx_wake_queue(netdev_get_tx_queue(dev, id));
}
+static u32 toeplitz_hash(const u8 *k, unsigned int klen,
+const u8 *d, unsigned int dlen)
+{
+ unsigned int di, ki;
+ u64 prefix = 0;
+ u64 hash = 0;
+
+ /* Pre-load prefix with the first 8 bytes of the key */
+ for (ki = 0; ki < 8; ki++) {
+ prefix <<= 8;
+ prefix |= (ki < klen) ? k[ki] : 0;
+ }
+
+ for (di = 0; di < dlen; di++) {
+ u8 byte = d[di];
+ unsigned int bit;
+
+ for (bit = 0x80; bit != 0; bit >>= 1) {
+ if (byte & bit)
+ hash ^= prefix;
+ prefix <<= 1;
+ }
+
+ /* prefix has now been left-shifted by 8, so OR in
+* the next byte.
+*/
+ prefix |= (ki < klen) ? k[ki] : 0;
+ ki++;
+ }
+
+ /* The valid part of the hash is in the upper 32 bits. */
+ return hash >> 32;
+}
+
+static void xenvif_set_toeplitz_hash(struct xenvif *vif, struct sk_buff *skb)
+{
+ struct flow_keys flow;
+ u32 hash = 0;
+ enum pkt_hash_types type = PKT_HASH_TYPE_NONE;
+ const u8 *key = vif->toeplitz.key;
+ u32 flags = vif->toeplitz.flags;
+ const unsigned int len = XEN_NETBK_MAX_TOEPLITZ_KEY_SIZE;
+ bool has_tcp_hdr;
+
+ /* Quick rejection test: If the network protocol doesn't
+* correspond to any enabled hash type then there's no point
+* in parsing the packet header.
+*/
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ if (flags & (XEN_NETIF_CTRL_TOEPLITZ_HASH_IPV4_TCP |
+XEN_NETIF_CTRL_TOEPLITZ_HASH_IPV4))
+ break;
+
+ goto done;
+
+ case htons(ETH_P_IPV6):
+ if (flags & (XEN_NETIF_CTRL_TOEPLITZ_HASH_IPV6_TCP |
+XEN_NETIF_CTRL_TOEPLITZ_HASH_IPV6))
+ break;
+
+ goto done;
+
+ default:
+ goto done;
+ }
+
+ memset(&flow, 0, sizeof(flow));
+ if (!skb_flow_dissect_flow_keys(skb, &flow, 0))
+ goto done;
+
+ has_tcp_hdr = (flow.basic.ip_proto == IPPROTO_TCP) &&
+ !(flow.control.flags & FLOW_DIS_IS_FRAGMENT);
+
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ if (has_tcp_hdr &&
+ (flags & XEN_NETIF_CTRL_TOEPLITZ_HASH_IPV4_TCP)) {
+ u8 data[12];
+
+ memcpy(&data[0], &flow.addrs.v4addrs.src, 4);
+ memcpy(&data[4], &flow.addrs.v4addrs.dst, 4);
+ memcpy(&data[8], &flow.ports.src, 2);
+ memcpy(&data[10], &flow.ports
[Xen-devel] [PATCH net-next v1 2/8] xen-netback: re-import canonical netif header
The canonical netif header (in the Xen source repo) and the Linux variant have diverged significantly. Recently much documentation has been added to the canonical header and new definitions and types to support packet hash configuration. Subsequent patches in this series add support for packet hash configuration in xen-netback so this patch re-imports the canonical header in readiness. To maintain compatibility and some style consistency with the old Linux variant, the header was stripped of its emacs boilerplate, and post-processed and copied into place with the following commands: ed -s netif.h << EOF H ,s/NETTXF_/XEN_NETTXF_/g ,s/NETRXF_/XEN_NETRXF_/g ,s/NETIF_RSP/XEN_NETIF_RSP/g ,s/NETIF_CTRL/XEN_NETIF_CTRL/g ,s/netif_tx/xen_netif_tx/g ,s/netif_rx/xen_netif_rx/g ,s/netif_ctrl/xen_netif_ctrl/g ,s/netif_extra_info/xen_netif_extra_info/g $ w EOF indent --line-length 80 --linux-style netif.h \ -o include/xen/interface/io/netif.h Signed-off-by: Paul Durrant Cc: Konrad Rzeszutek Wilk Cc: Boris Ostrovsky Cc: David Vrabel Cc: Wei Liu --- include/xen/interface/io/netif.h | 758 ++- 1 file changed, 667 insertions(+), 91 deletions(-) diff --git a/include/xen/interface/io/netif.h b/include/xen/interface/io/netif.h index 252ffd4..78e0815 100644 --- a/include/xen/interface/io/netif.h +++ b/include/xen/interface/io/netif.h @@ -3,14 +3,32 @@ * * Unified network-device I/O interface for Xen guest OSes. * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * * Copyright (c) 2003-2004, Keir Fraser */ #ifndef __XEN_PUBLIC_IO_NETIF_H__ #define __XEN_PUBLIC_IO_NETIF_H__ -#include -#include +#include "ring.h" +#include "../grant_table.h" /* * Older implementation of Xen network frontend / backend has an @@ -38,10 +56,10 @@ * that it cannot safely queue packets (as it may not be kicked to send them). */ - /* +/* * "feature-split-event-channels" is introduced to separate guest TX - * and RX notificaion. Backend either doesn't support this feature or - * advertise it via xenstore as 0 (disabled) or 1 (enabled). + * and RX notification. Backend either doesn't support this feature or + * advertises it via xenstore as 0 (disabled) or 1 (enabled). * * To make use of this feature, frontend should allocate two event * channels for TX and RX, advertise them to backend as @@ -118,151 +136,709 @@ */ /* - * This is the 'wire' format for packets: - * Request 1: xen_netif_tx_request -- XEN_NETTXF_* (any flags) - * [Request 2: xen_netif_extra_info](only if request 1 has XEN_NETTXF_extra_info) - * [Request 3: xen_netif_extra_info](only if request 2 has XEN_NETIF_EXTRA_MORE) - * Request 4: xen_netif_tx_request -- XEN_NETTXF_more_data - * Request 5: xen_netif_tx_request -- XEN_NETTXF_more_data + * "feature-multicast-control" and "feature-dynamic-multicast-control" + * advertise the capability to filter ethernet multicast packets in the + * backend. If the frontend wishes to take advantage of this feature then + * it may set "request-multicast-control". If the backend only advertises + * "feature-multicast-control" then "request-multicast-control" must be set + * before the frontend moves into the connected state. The backend will + * sample the value on this state transition and any subsequent change in + * value will have no effect. However, if the backend also advertises + * "feature-dynamic-multicast-control" then "request-multicast-control" + * may be set by the frontend at any time. In this case, the backend will + * watch the value and re-sample on watch events. + * + * If the sampled value of "request-multicast-control" is set then the + * backend transmit side should no longer flood multicast packets to the + * frontend, it should instead drop any multicast packet that does not + * match in a filter list. + * The list is amended by the frontend by sending dummy transmit requests + * containing XEN_NETIF_EXTRA_TYPE
[Xen-devel] [PATCH net-next v1 0/8] xen-netback: support toeplitz hashing
This patch series adds support for frontend-configurable toeplitz hashing in xen-netback (on the guest receive side). This support has been testing against a Windows frontend and has proven to be sufficient to pass the Microsoft HCK NDIS RSS tests. For convenience my development branch is available at http://xenbits.xen.org/gitweb/?p=people/pauldu/linux.git;a=shortlog;h=refs/heads/rss18. Patch #1 contains a small modification to struct sk_buff to use one more bit's worth of space to enable full hasf information to be stored (as opposed to simply whether a calculated hash is L4 or not). Patch #2 re-imports the canonical netif.h from the Xen master branch at git://xenbits.xen.org/xen.git. To minimize the diff it was post-processed as detailed in the commit message. Patch #3 fixes a short-coming in netback so that it can actually cope with multiple extra_info fragments being passed from the frontend on the guest transmit side. Patch #4 is a trivial patch to reduce log spam from netback. Patch #5 adds support for a new shared ring for control messages between frontend and backend as detailed in the updated netif.h. Patch #6 builds on patch #5 and adds support for messages passed from the frontend to configure a toeplitz hash of packets on the guest receive side. Patch #7 adds code to pass the hash calculated by netback to the frontend Patch #8 adds code to take hashes calculated by the guest on its transmit side and set the approprate values in the constructed socket buffer. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH net-next v1 8/8] xen-netback: use toeplitz hash value from the frontend
This patch adds code to xen-netback to use the value in a toeplitz hash
extra info fragment passed from the VM frontend in a transmit-side packet
to set the skb hash accordingly.
Signed-off-by: Paul Durrant
Cc: Ian Campbell
Cc: Wei Liu
---
drivers/net/xen-netback/netback.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index 57c91fe..f550b8a 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1341,6 +1341,28 @@ void xenvif_mcast_addr_list_free(struct xenvif *vif)
}
}
+static void xenvif_set_skb_hash(struct xenvif *vif,
+ struct sk_buff *skb,
+ struct xen_netif_extra_info *extra)
+{
+ u32 hash = *(u32 *)extra->u.toeplitz.value;
+ enum pkt_hash_types type = PKT_HASH_TYPE_NONE;
+
+ switch (extra->u.toeplitz.type) {
+ case _XEN_NETIF_CTRL_TOEPLITZ_HASH_IPV4:
+ case _XEN_NETIF_CTRL_TOEPLITZ_HASH_IPV6:
+ type = PKT_HASH_TYPE_L3;
+ break;
+
+ case _XEN_NETIF_CTRL_TOEPLITZ_HASH_IPV4_TCP:
+ case _XEN_NETIF_CTRL_TOEPLITZ_HASH_IPV6_TCP:
+ type = PKT_HASH_TYPE_L4;
+ break;
+ }
+
+ skb_set_hash(skb, hash, type);
+}
+
static void xenvif_tx_build_gops(struct xenvif_queue *queue,
int budget,
unsigned *copy_ops,
@@ -1502,6 +1524,13 @@ static void xenvif_tx_build_gops(struct xenvif_queue
*queue,
}
}
+ if (extras[XEN_NETIF_EXTRA_TYPE_TOEPLITZ - 1].type) {
+ struct xen_netif_extra_info *extra;
+
+ extra = &extras[XEN_NETIF_EXTRA_TYPE_TOEPLITZ - 1];
+ xenvif_set_skb_hash(queue->vif, skb, extra);
+ }
+
XENVIF_TX_CB(skb)->pending_idx = pending_idx;
__skb_put(skb, data_len);
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH net-next v1 7/8] xen-netback: pass toeplitz hash value to the frontend
My recent patch to include/xen/interface/io/netif.h defines a new extra
info type that can be used to pass toeplitz hash values between backend
and VM frontend.
This patch adds code to xen-netback to pass hash values calculated for
receive-side packets to the VM frontend.
Signed-off-by: Paul Durrant
Cc: Ian Campbell
Cc: Wei Liu
---
drivers/net/xen-netback/common.h | 1 +
drivers/net/xen-netback/netback.c | 76 ---
2 files changed, 65 insertions(+), 12 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 6687702..469dcf0 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -243,6 +243,7 @@ struct xenvif {
u8 ip_csum:1;
u8 ipv6_csum:1;
u8 multicast_control:1;
+ u8 hash_extra:1;
/* Is this interface disabled? True when backend discovers
* frontend is rogue.
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index 41ec7e9..57c91fe 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -163,6 +163,8 @@ static bool xenvif_rx_ring_slots_available(struct
xenvif_queue *queue)
needed = DIV_ROUND_UP(skb->len, XEN_PAGE_SIZE);
if (skb_is_gso(skb))
needed++;
+ if (queue->vif->hash_extra)
+ needed++;
do {
prod = queue->rx.sring->req_prod;
@@ -280,6 +282,8 @@ struct gop_frag_copy {
struct xenvif_rx_meta *meta;
int head;
int gso_type;
+ int protocol;
+ int hash_type;
struct page *page;
};
@@ -326,8 +330,19 @@ static void xenvif_setup_copy_gop(unsigned long gfn,
npo->copy_off += *len;
info->meta->size += *len;
+ if (!info->head)
+ return;
+
/* Leave a gap for the GSO descriptor. */
- if (info->head && ((1 << info->gso_type) & queue->vif->gso_mask))
+ if ((1 << info->gso_type) & queue->vif->gso_mask)
+ queue->rx.req_cons++;
+
+ /* Leave a gap for the hash extra segment. */
+ if (queue->vif->hash_extra &&
+ (info->hash_type == PKT_HASH_TYPE_L4 ||
+info->hash_type == PKT_HASH_TYPE_L3) &&
+ (info->protocol == htons(ETH_P_IP) ||
+info->protocol == htons(ETH_P_IPV6)))
queue->rx.req_cons++;
info->head = 0; /* There must be something in this buffer now */
@@ -362,6 +377,8 @@ static void xenvif_gop_frag_copy(struct xenvif_queue
*queue, struct sk_buff *skb
.npo = npo,
.head = *head,
.gso_type = XEN_NETIF_GSO_TYPE_NONE,
+ .protocol = skb->protocol,
+ .hash_type = skb_hash_type(skb),
};
unsigned long bytes;
@@ -550,6 +567,7 @@ void xenvif_kick_thread(struct xenvif_queue *queue)
static void xenvif_rx_action(struct xenvif_queue *queue)
{
+ struct xenvif *vif = queue->vif;
s8 status;
u16 flags;
struct xen_netif_rx_response *resp;
@@ -567,6 +585,8 @@ static void xenvif_rx_action(struct xenvif_queue *queue)
skb_queue_head_init(&rxq);
+ vif->hash_extra = !!vif->toeplitz.flags;
+
while (xenvif_rx_ring_slots_available(queue)
&& (skb = xenvif_rx_dequeue(queue)) != NULL) {
queue->last_rx_time = jiffies;
@@ -585,9 +605,10 @@ static void xenvif_rx_action(struct xenvif_queue *queue)
gnttab_batch_copy(queue->grant_copy_op, npo.copy_prod);
while ((skb = __skb_dequeue(&rxq)) != NULL) {
+ struct xen_netif_extra_info *extra = NULL;
if ((1 << queue->meta[npo.meta_cons].gso_type) &
- queue->vif->gso_prefix_mask) {
+ vif->gso_prefix_mask) {
resp = RING_GET_RESPONSE(&queue->rx,
queue->rx.rsp_prod_pvt++);
@@ -605,7 +626,7 @@ static void xenvif_rx_action(struct xenvif_queue *queue)
queue->stats.tx_bytes += skb->len;
queue->stats.tx_packets++;
- status = xenvif_check_gop(queue->vif,
+ status = xenvif_check_gop(vif,
XENVIF_RX_CB(skb)->meta_slots_used,
&npo);
@@ -627,21 +648,52 @@ static void xenvif_rx_action(struct xenvif_queue *queue)
flags);
if ((1 << queue->meta[npo.meta_cons].gso_type) &
- queue->vif->gso_mask) {
- struct xen_netif_extra_info *gso =
- (struct xen_netif_extra_info *)
+ vif->gso_mask) {
+ extra = (struct xen_netif_extra_info *)
RING_GET_RESPONSE(&queue->rx,
queue->rx.rsp_prod_pvt++);
[Xen-devel] [PATCH net-next v1 5/8] xen-netback: add support for the control ring
My recent patch to include/xen/interface/io/netif.h defines a new shared
ring (in addition to the rx and tx rings) for passing control messages
from a VM frontend driver to a backend driver.
This patch adds the necessary code to xen-netback to map this new shared
ring, should it be created by a frontend, and process messages passed on
the ring. Note that, to avoid it becoming overly large, this patch does
not introduce an implementation of any of the control messages specified in
netif.h (that is deferred to a subsequent patch) and so all messages
elicit a 'not supported' response.
Signed-off-by: Paul Durrant
Cc: Ian Campbell
Cc: Wei Liu
---
drivers/net/xen-netback/common.h| 28 +++---
drivers/net/xen-netback/interface.c | 101 +---
drivers/net/xen-netback/netback.c | 100 +--
drivers/net/xen-netback/xenbus.c| 75 ++
4 files changed, 274 insertions(+), 30 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index f44b388..093a12a 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -260,6 +260,11 @@ struct xenvif {
struct dentry *xenvif_dbg_root;
#endif
+ struct xen_netif_ctrl_back_ring ctrl;
+ struct task_struct *ctrl_task;
+ wait_queue_head_t ctrl_wq;
+ unsigned int ctrl_irq;
+
/* Miscellaneous private stuff. */
struct net_device *dev;
};
@@ -285,10 +290,15 @@ struct xenvif *xenvif_alloc(struct device *parent,
int xenvif_init_queue(struct xenvif_queue *queue);
void xenvif_deinit_queue(struct xenvif_queue *queue);
-int xenvif_connect(struct xenvif_queue *queue, unsigned long tx_ring_ref,
- unsigned long rx_ring_ref, unsigned int tx_evtchn,
- unsigned int rx_evtchn);
-void xenvif_disconnect(struct xenvif *vif);
+int xenvif_connect_data(struct xenvif_queue *queue,
+ unsigned long tx_ring_ref,
+ unsigned long rx_ring_ref,
+ unsigned int tx_evtchn,
+ unsigned int rx_evtchn);
+void xenvif_disconnect_data(struct xenvif *vif);
+int xenvif_connect_ctrl(struct xenvif *vif, grant_ref_t ring_ref,
+ unsigned int evtchn);
+void xenvif_disconnect_ctrl(struct xenvif *vif);
void xenvif_free(struct xenvif *vif);
int xenvif_xenbus_init(void);
@@ -300,10 +310,10 @@ int xenvif_queue_stopped(struct xenvif_queue *queue);
void xenvif_wake_queue(struct xenvif_queue *queue);
/* (Un)Map communication rings. */
-void xenvif_unmap_frontend_rings(struct xenvif_queue *queue);
-int xenvif_map_frontend_rings(struct xenvif_queue *queue,
- grant_ref_t tx_ring_ref,
- grant_ref_t rx_ring_ref);
+void xenvif_unmap_frontend_data_rings(struct xenvif_queue *queue);
+int xenvif_map_frontend_data_rings(struct xenvif_queue *queue,
+ grant_ref_t tx_ring_ref,
+ grant_ref_t rx_ring_ref);
/* Check for SKBs from frontend and schedule backend processing */
void xenvif_napi_schedule_or_enable_events(struct xenvif_queue *queue);
@@ -318,6 +328,8 @@ void xenvif_kick_thread(struct xenvif_queue *queue);
int xenvif_dealloc_kthread(void *data);
+int xenvif_ctrl_kthread(void *data);
+
void xenvif_rx_queue_tail(struct xenvif_queue *queue, struct sk_buff *skb);
void xenvif_carrier_on(struct xenvif *vif);
diff --git a/drivers/net/xen-netback/interface.c
b/drivers/net/xen-netback/interface.c
index f5231a2..1850ebb 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -128,6 +128,15 @@ irqreturn_t xenvif_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
+irqreturn_t xenvif_ctrl_interrupt(int irq, void *dev_id)
+{
+ struct xenvif *vif = dev_id;
+
+ wake_up(&vif->ctrl_wq);
+
+ return IRQ_HANDLED;
+}
+
int xenvif_queue_stopped(struct xenvif_queue *queue)
{
struct net_device *dev = queue->vif->dev;
@@ -527,9 +536,66 @@ void xenvif_carrier_on(struct xenvif *vif)
rtnl_unlock();
}
-int xenvif_connect(struct xenvif_queue *queue, unsigned long tx_ring_ref,
- unsigned long rx_ring_ref, unsigned int tx_evtchn,
- unsigned int rx_evtchn)
+int xenvif_connect_ctrl(struct xenvif *vif, grant_ref_t ring_ref,
+ unsigned int evtchn)
+{
+ struct net_device *dev = vif->dev;
+ void *addr;
+ struct xen_netif_ctrl_sring *shared;
+ struct task_struct *task;
+ int err = -ENOMEM;
+
+ err = xenbus_map_ring_valloc(xenvif_to_xenbus_device(vif),
+&ring_ref, 1, &addr);
+ if (err)
+ goto err;
+
+ shared = (struct xen_netif_ctrl_sring *)addr;
+ BACK_RING_INIT(&vif->ctrl, shared, XEN_PAGE_SIZE);
+
+ init_waitqueue_head(&vif->
[Xen-devel] [PATCH net-next v1 4/8] xen-netback: reduce log spam
Remove the "prepare for reconnect" pr_info in xenbus.c. It's largely
uninteresting and the states of the frontend and backend can easily be
observed by watching the (o)xenstored log.
Signed-off-by: Paul Durrant
Cc: Ian Campbell
Cc: Wei Liu
---
drivers/net/xen-netback/xenbus.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 39a303d..bd182cd 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -511,8 +511,6 @@ static void set_backend_state(struct backend_info *be,
switch (state) {
case XenbusStateInitWait:
case XenbusStateConnected:
- pr_info("%s: prepare for reconnect\n",
- be->dev->nodename);
backend_switch_state(be, XenbusStateInitWait);
break;
case XenbusStateClosing:
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH net-next v1 3/8] xen-netback: support multiple extra info fragments passed from frontend
The code does not currently support a frontend passing multiple extra info
fragments to the backend in a tx request. The xenvif_get_extras() function
handles multiple extra_info fragments but make_tx_response() assumes there
is only ever a single extra info fragment.
This patch modifies xenvif_get_extras() to pass back a count of extra
info fragments, which is then passed to make_tx_response() (after
possibly being stashed in pending_tx_info for deferred responses).
Signed-off-by: Paul Durrant
Cc: Ian Campbell
Cc: Wei Liu
---
drivers/net/xen-netback/common.h | 1 +
drivers/net/xen-netback/netback.c | 65 +--
2 files changed, 43 insertions(+), 23 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 1128252..f44b388 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -52,6 +52,7 @@ typedef unsigned int pending_ring_idx_t;
struct pending_tx_info {
struct xen_netif_tx_request req; /* tx request */
+ unsigned int extra_count;
/* Callback data for released SKBs. The callback is always
* xenvif_zerocopy_callback, desc contains the pending_idx, which is
* also an index in pending_tx_info array. It is initialized in
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index 61b97c3..b42f260 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -95,6 +95,7 @@ static void xenvif_idx_release(struct xenvif_queue *queue,
u16 pending_idx,
static void make_tx_response(struct xenvif_queue *queue,
struct xen_netif_tx_request *txp,
+unsigned int extra_count,
s8 st);
static void push_tx_responses(struct xenvif_queue *queue);
@@ -696,14 +697,15 @@ void xenvif_tx_credit_callback(unsigned long data)
}
static void xenvif_tx_err(struct xenvif_queue *queue,
- struct xen_netif_tx_request *txp, RING_IDX end)
+ struct xen_netif_tx_request *txp,
+ unsigned int extra_count, RING_IDX end)
{
RING_IDX cons = queue->tx.req_cons;
unsigned long flags;
do {
spin_lock_irqsave(&queue->response_lock, flags);
- make_tx_response(queue, txp, XEN_NETIF_RSP_ERROR);
+ make_tx_response(queue, txp, extra_count, XEN_NETIF_RSP_ERROR);
push_tx_responses(queue);
spin_unlock_irqrestore(&queue->response_lock, flags);
if (cons == end)
@@ -724,6 +726,7 @@ static void xenvif_fatal_tx_err(struct xenvif *vif)
static int xenvif_count_requests(struct xenvif_queue *queue,
struct xen_netif_tx_request *first,
+unsigned int extra_count,
struct xen_netif_tx_request *txp,
int work_to_do)
{
@@ -812,7 +815,7 @@ static int xenvif_count_requests(struct xenvif_queue *queue,
} while (more_data);
if (drop_err) {
- xenvif_tx_err(queue, first, cons + slots);
+ xenvif_tx_err(queue, first, extra_count, cons + slots);
return drop_err;
}
@@ -827,9 +830,10 @@ struct xenvif_tx_cb {
#define XENVIF_TX_CB(skb) ((struct xenvif_tx_cb *)(skb)->cb)
static inline void xenvif_tx_create_map_op(struct xenvif_queue *queue,
- u16 pending_idx,
- struct xen_netif_tx_request *txp,
- struct gnttab_map_grant_ref *mop)
+ u16 pending_idx,
+ struct xen_netif_tx_request *txp,
+ unsigned int extra_count,
+ struct gnttab_map_grant_ref *mop)
{
queue->pages_to_map[mop-queue->tx_map_ops] =
queue->mmap_pages[pending_idx];
gnttab_set_map_op(mop, idx_to_kaddr(queue, pending_idx),
@@ -838,6 +842,7 @@ static inline void xenvif_tx_create_map_op(struct
xenvif_queue *queue,
memcpy(&queue->pending_tx_info[pending_idx].req, txp,
sizeof(*txp));
+ queue->pending_tx_info[pending_idx].extra_count = extra_count;
}
static inline struct sk_buff *xenvif_alloc_skb(unsigned int size)
@@ -880,7 +885,7 @@ static struct gnttab_map_grant_ref
*xenvif_get_requests(struct xenvif_queue *que
shinfo->nr_frags++, txp++, gop++) {
index = pending_index(queue->pending_cons++);
pending_idx = queue->pending_ring[index];
- xenvif_tx_create_map_op(queue, pending_idx, txp, gop);
+ xenvif_tx_create_map_op(queue, pending_idx, txp, 0, gop);
frag_set_pending_idx(&frags[shinfo->nr_frags], pending_idx);
Re: [Xen-devel] [PATCH v2 2/2] vm_event: consolidate hvm_event_fill_regs and p2m_vm_event_fill_regs
>>> On 12.02.16 at 11:19, wrote:
> On 02/12/2016 11:57 AM, Jan Beulich wrote:
> On 12.02.16 at 01:22, wrote:
>>> --- a/xen/arch/x86/vm_event.c
>>> +++ b/xen/arch/x86/vm_event.c
>>> @@ -122,6 +122,65 @@ void vm_event_set_registers(struct vcpu *v,
> vm_event_response_t *rsp)
>>> v->arch.user_regs.eip = rsp->data.regs.x86.rip;
>>> }
>>>
>>> +void vm_event_fill_regs(vm_event_request_t *req)
>>> +{
>>> +const struct cpu_user_regs *regs = guest_cpu_user_regs();
>>> +struct segment_register seg;
>>> +struct hvm_hw_cpu ctxt;
>>> +struct vcpu *curr = current;
>>> +
>>> +req->data.regs.x86.rax = regs->eax;
>>> +req->data.regs.x86.rcx = regs->ecx;
>>> +req->data.regs.x86.rdx = regs->edx;
>>> +req->data.regs.x86.rbx = regs->ebx;
>>> +req->data.regs.x86.rsp = regs->esp;
>>> +req->data.regs.x86.rbp = regs->ebp;
>>> +req->data.regs.x86.rsi = regs->esi;
>>> +req->data.regs.x86.rdi = regs->edi;
>>> +
>>> +req->data.regs.x86.r8 = regs->r8;
>>> +req->data.regs.x86.r9 = regs->r9;
>>> +req->data.regs.x86.r10 = regs->r10;
>>> +req->data.regs.x86.r11 = regs->r11;
>>> +req->data.regs.x86.r12 = regs->r12;
>>> +req->data.regs.x86.r13 = regs->r13;
>>> +req->data.regs.x86.r14 = regs->r14;
>>> +req->data.regs.x86.r15 = regs->r15;
>>> +
>>> +req->data.regs.x86.rflags = regs->eflags;
>>> +req->data.regs.x86.rip= regs->eip;
>>> +
>>> +if ( !is_hvm_domain(curr->domain) )
>>> +return;
>>
>> No such check existed in either of the two original functions. Why is
>> it needed all of the sudden? And if it is needed, why do the other
>> fields not get filled (as far as possible at least) for PV guests?
>
> I can't speak for Tamas, but I suspect the check has been placed there
> because calls to hvm_funcs.save_cpu_ctxt(curr, &ctxt) and
> hvm_get_segment_register(curr, x86_seg_fs, &seg) follow, and he's put
> vm_event_fill_regs() in xen/arch/x86/vm_event.c (a previous function was
> called hvm_event_fill_regs(), in arch/x86/hvm/event.c, so no checking
> for HVM was needed).
>
> I don't think the check is needed for the current codepaths, but since
> the code has been moved to xen/arch/x86/ the question about future PV
> events is fair.
In which case ASSERT(is_hvm_vcpu(curr)) would be the common
way to document this (at once avoiding the open coding of
is_hvm_vcpu()).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] xen: credit1: avoid boosting vCPUs being "just" migrated
On Fri, 2016-02-12 at 02:50 -0700, Jan Beulich wrote:
> > > > On 12.02.16 at 10:37, wrote:
> > @@ -787,6 +788,16 @@ _csched_cpu_pick(const struct scheduler *ops,
> > struct vcpu *vc, bool_t commit)
> > static int
> > csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc)
> > {
> > +struct csched_vcpu *svc = CSCHED_VCPU(vc);
> > +
> > +/*
> > + * We have been called by vcpu_migrate() (in schedule.c), as
> > part
> > + * of the process of seeing if vc can be migrated to another
> > pcpu.
> > + * We make a note about this in svc->flags so that later, in
> > + * csched_vcpu_wake() (still called from vcpu_migrate()) we
> > won't
> > + * get boosted, which we don't deserve as we are "only"
> > migrating.
> > + */
> > +set_bit(CSCHED_FLAG_VCPU_MIGRATING, &svc->flags);
> > return _csched_cpu_pick(ops, vc, 1);
> > }
>
> I think you either want __set_bit() here or ...
>
Yes, this is completely serialized by the vcpu's scheduler lock, so I
indeed want __set_bit(), sorry for the overlook.
Thanks and Regards,
Dario
--
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"
On 12/02/16 10:12, Jan Beulich wrote: On 12.02.16 at 11:02, wrote: >> On 12/02/16 10:00, Jan Beulich wrote: >> On 12.02.16 at 10:51, wrote: On 12/02/16 08:23, Jan Beulich wrote: On 11.02.16 at 20:25, wrote: >> CentOS 7 gets into trouble when compiling Xen citing: >> >> flushtlb.c: Assembler messages: >> flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1 >> >> The line number is wrong, and the error message not helpful. It turns >> out >> that the intermediate generated assembly was >> >> # 139 "arch/x86/flushtlb.c" 1 >> 661: >> rex clflush (%r15) >> 662: >> .pushsection .altinstructions,"a" >> >> and it was having trouble combining the explicit REX prefix with the >> REX.B >> required for the use of %r15. > What gas version is this? I just checked with 2.20, which has no > problem combining an explicit with a generated REX prefix. bash-4.2$ as --version GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226 > Or > wait, no, your description of the issue is wrong: It actually is the > folding of the two REX prefixes which causes the problem, This is what I said. What do you think I meant with "trouble combining the" ? >>> Argh - I meant to say "It actually isn't ...". >>> > since > that results in the replacement instruction being one byte longer > than the to be replaced one. But that is still the case with an explicit %ds override. The assembler still has to insert an extra byte somehow. >>> No. We now always have one non-REX prefix, and both instructions >>> will have the same REX/ModRM/SIB encoding. >> I see now, given your wording on the patch committed. >> >> In hindsight this should have been obvious, but GCCs error message was >> particularly unhelpful at diagnosing the issue. > It was actually an assembler error message, and I can't really see > how we could improve that (since afaict the intended checking can > only be done at assembly time). Oh right. It is an assembler BUILD_BUG_ON(). Is there anything useful we can do to get the error message to properly point at alternative.h: DISCARD_ENTRY()? That would at least have helped. As it stood, the actual reported error line was a closing brace after the wbinvd() call. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: fix handling returns in libxl_get_version_info()
On Thu, Feb 11, 2016 at 3:18 PM, Dario Faggioli wrote: > Hi Harmandeep, > > So, I think the code in this patch is ok. Still, a few comments... > > On Thu, 2016-02-11 at 14:00 +0530, Harmandeep Kaur wrote: >> Avoid handling issue of the return value of xc_version() in many >> cases. >> > This can be rephrased to be easier to understand (I'm not sure I can > tell what you mean with "Avoid handling issue of the return value"). > > Something like "Check the return value of xc_version() and return NULL > if it fails." > > In fact, I think the fact that the function can now actually return > NULL should be mentioned here in the changelog. > > Another thing that you should check, and probably quickly mention too, > is whether or not the callers of these functions --the ones inside > xen.git of course-- are prepared to deal with this. I mean, returning > NULL on failure of xc_version() is, IMO, the right thing to do here > anyway, but it is something important to know whether more work is > needed to fix the callers as well, or if we're just fine. > > To do so, search (e.g., with grep or cscope) for uses of > libxl_get_version_info inside the tools/ dir of xen.git. For instance, > there is one such use in xl_cmdimpl.c, in the output_xeninfo() > function, which looks like it would interact well with this patch... > Can you confirm this is the case also for all the other instances and > note it down here? I think NULL may not be handled properly in auto_autoballoon() in tools/libxl/xl.c Other instances seems okay to me. Should I fix the caller, too? >> Coverity ID 1351217 >> >> Signed-off-by: Harmandeep Kaur >> --- >> tools/libxl/libxl.c | 39 ++- >> 1 file changed, 30 insertions(+), 9 deletions(-) >> >> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c >> index 2d18b8d..a939e51 100644 >> --- a/tools/libxl/libxl.c >> +++ b/tools/libxl/libxl.c >> @@ -5267,42 +5267,63 @@ const libxl_version_info* >> libxl_get_version_info(libxl_ctx *ctx) >> xen_platform_parameters_t p_parms; >> xen_commandline_t xen_commandline; >> } u; >> +int rc; >> > So, according to tools/libxl/CODING_STYLE: > > The following local variable names should be used where applicable: > > int rc;/* a libxl error code - and not anything else */ > int r; /* the return value from a system call (or libxc call) */ > > Given how it's used, this variable you're introducing falls into the > latter category. > > And I also think you need to initialize it. > >> long xen_version; >> libxl_version_info *info = &ctx->version_info; >> >> if (info->xen_version_extra != NULL) >> goto out; >> >> -xen_version = xc_version(ctx->xch, XENVER_version, NULL); >> +rc = xen_version = xc_version(ctx->xch, XENVER_version, NULL); >> +if ( rc < 0 ) >> +goto out; >> + >> info->xen_version_major = xen_version >> 16; >> info->xen_version_minor = xen_version & 0xFF; >> > I think you can just get rid of the xen_version variable and, if > xc_version() returns > 0, do the necessary manipulations on r itself > (this, for instance, matches what happens in > xc_core.c:elfnote_fill_xen_version, and is IMO ok to be done here as > well). > >> +rc = xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra); >> +if ( rc < 0 ) >> +goto out; >> >> -xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra); >> info->xen_version_extra = libxl__strdup(NOGC, u.xen_extra); >> +rc = xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc); >> +if ( rc < 0 ) >> +goto out; >> >> -xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc); >> info->compiler = libxl__strdup(NOGC, u.xen_cc.compiler); >> info->compile_by = libxl__strdup(NOGC, u.xen_cc.compile_by); >> info->compile_domain = libxl__strdup(NOGC, >> u.xen_cc.compile_domain); >> info->compile_date = libxl__strdup(NOGC, u.xen_cc.compile_date); >> > Although functionally correct, the final look of all these "blocks" > would result rather hard to read. > > I think it would be better if you make the patch in such a way that the > final code would look like this: > > r = xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra); > if ( r < 0 ) > goto out; > info->xen_version_extra = libxl__strdup(NOGC, u.xen_extra); > > r = xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc); > if ( r < 0 ) > goto out; > info->compiler = libxl__strdup(NOGC, u.xen_cc.compiler); > info->compile_by = libxl__strdup(NOGC, u.xen_cc.compile_by); > info->compile_domain = libxl__strdup(NOGC, > u.xen_cc.compile_domain); > info->compile_date = libxl__strdup(NOGC, u.xen_cc.compile_date); > > r = xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps); > if ( r < 0 ) > goto out; > info->capabilities = libxl__strdup(NOGC, u.xen_caps); > > etc. > >> out: >> GC_FREE; >> -return info; >> +if ( rc < 0 ) >> + retu
Re: [Xen-devel] [PATCH net-next v1 0/8] xen-netback: support toeplitz hashing
From: Paul Durrant Date: Fri, 12 Feb 2016 10:13:17 + > This patch series adds support for frontend-configurable toeplitz hashing > in xen-netback (on the guest receive side). This support has been testing > against a Windows frontend and has proven to be sufficient to pass the > Microsoft HCK NDIS RSS tests. We want less Toeplitz users, not more of them. It is a more unsuitable hash than jhash2. I'm not applying these patches, sorry. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [qemu-upstream-4.5-testing test] 81790: regressions - FAIL
flight 81790 qemu-upstream-4.5-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/81790/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 77858 test-amd64-amd64-qemuu-nested-amd 9 debian-hvm-install fail REGR. vs. 77858 test-amd64-amd64-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 77858 test-amd64-i386-qemuu-rhel6hvm-intel 9 redhat-installfail REGR. vs. 77858 test-amd64-i386-qemuu-rhel6hvm-amd 9 redhat-install fail REGR. vs. 77858 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 9 windows-install fail REGR. vs. 77858 test-amd64-amd64-xl-qemuu-winxpsp3 9 windows-install fail REGR. vs. 77858 test-amd64-i386-xl-qemuu-winxpsp3 9 windows-install fail REGR. vs. 77858 test-amd64-amd64-xl-qemuu-win7-amd64 9 windows-install fail REGR. vs. 77858 test-amd64-i386-xl-qemuu-win7-amd64 9 windows-installfail REGR. vs. 77858 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-qemuu-nested-intel 9 debian-hvm-install fail like 77752 test-amd64-i386-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail like 77752 test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail like 77752 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeatfail like 77858 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 10 guest-start fail never pass test-armhf-armhf-libvirt-raw 10 guest-start fail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 10 guest-start fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-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-rtds 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 guest-saverestorefail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass version targeted for testing: qemuudf8471f739cc24211fd42964de4367e72ab433f5 baseline version: qemuu32bba3499008c847e08858f310d65806e0bade36 Last test of basis77858 2016-01-11 19:23:09 Z 31 days Failing since 80731 2016-02-05 15:18:36 Z6 days2 attempts Testing same since81790 2016-02-10 12:35:30 Z1 days1 attempts People who touched revisions under test: Gerd Hoffmann Jason Wang John Snow Laszlo Ersek Michael S. Tsirkin P J P Paolo Bonzini Peter Maydell Prasad J Pandit Roger Pau Monne Roger Pau Monné Stefano Stabellini jobs: build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass test-armhf-armhf-xl pass test-amd64-i386-xl pass test-amd64-amd64-qemuu-nested-amdfail test-amd64-amd64-xl-pvh-amd fail test-amd64-i386-qemuu-rhel6hvm-amd fail test-amd64-amd64-xl-qemuu-debianhvm-amd64fail test-amd
Re: [Xen-devel] [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"
>>> On 12.02.16 at 11:50, wrote: > On 12/02/16 10:12, Jan Beulich wrote: > On 12.02.16 at 11:02, wrote: >>> On 12/02/16 10:00, Jan Beulich wrote: >>> On 12.02.16 at 10:51, wrote: > On 12/02/16 08:23, Jan Beulich wrote: > On 11.02.16 at 20:25, wrote: >>> CentOS 7 gets into trouble when compiling Xen citing: >>> >>> flushtlb.c: Assembler messages: >>> flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at >>> 1 >>> >>> The line number is wrong, and the error message not helpful. It turns >>> out >>> that the intermediate generated assembly was >>> >>> # 139 "arch/x86/flushtlb.c" 1 >>> 661: >>> rex clflush (%r15) >>> 662: >>> .pushsection .altinstructions,"a" >>> >>> and it was having trouble combining the explicit REX prefix with the >>> REX.B >>> required for the use of %r15. >> What gas version is this? I just checked with 2.20, which has no >> problem combining an explicit with a generated REX prefix. > bash-4.2$ as --version > GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226 > > >> Or >> wait, no, your description of the issue is wrong: It actually is the >> folding of the two REX prefixes which causes the problem, > This is what I said. What do you think I meant with "trouble combining > the" ? Argh - I meant to say "It actually isn't ...". >> since >> that results in the replacement instruction being one byte longer >> than the to be replaced one. > But that is still the case with an explicit %ds override. The assembler > still has to insert an extra byte somehow. No. We now always have one non-REX prefix, and both instructions will have the same REX/ModRM/SIB encoding. >>> I see now, given your wording on the patch committed. >>> >>> In hindsight this should have been obvious, but GCCs error message was >>> particularly unhelpful at diagnosing the issue. >> It was actually an assembler error message, and I can't really see >> how we could improve that (since afaict the intended checking can >> only be done at assembly time). > > Oh right. It is an assembler BUILD_BUG_ON(). > > Is there anything useful we can do to get the error message to properly > point at alternative.h: DISCARD_ENTRY()? That would at least have helped. I'd like to, but I can't see how, not the least because ... > As it stood, the actual reported error line was a closing brace after > the wbinvd() call. ... I've also noticed that, but the assembler would just use whatever line directive the compiler had put in the assembly file. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: fix handling returns in libxl_get_version_info()
On Fri, 2016-02-12 at 16:22 +0530, Harmandeep Kaur wrote: > On Thu, Feb 11, 2016 at 3:18 PM, Dario Faggioli > wrote: > > > > Another thing that you should check, and probably quickly mention > > too, > > is whether or not the callers of these functions --the ones inside > > xen.git of course-- are prepared to deal with this. I mean, > > returning > > NULL on failure of xc_version() is, IMO, the right thing to do here > > anyway, but it is something important to know whether more work is > > needed to fix the callers as well, or if we're just fine. > > > > To do so, search (e.g., with grep or cscope) for uses of > > libxl_get_version_info inside the tools/ dir of xen.git. For > > instance, > > there is one such use in xl_cmdimpl.c, in the output_xeninfo() > > function, which looks like it would interact well with this > > patch... > > Can you confirm this is the case also for all the other instances > > and > > note it down here? > > I think NULL may not be handled properly in auto_autoballoon() in > tools/libxl/xl.c Other instances seems okay to me. > So, here: info = libxl_get_version_info(ctx); if (!info) return 1; /* default to on */ Why do you think this is a problem? It looks like this call site is actually prepared to see and handle a NULL return value... > Should I fix the caller, too? > If there are issues with them, yes. If there aren't just put something like "xxx callers are already fine xxx" in the changelog. Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: fix handling returns in libxl_get_version_info()
On Fri, Feb 12, 2016 at 4:30 PM, Dario Faggioli
wrote:
> On Fri, 2016-02-12 at 16:22 +0530, Harmandeep Kaur wrote:
>> On Thu, Feb 11, 2016 at 3:18 PM, Dario Faggioli
>> wrote:
>> >
>> > Another thing that you should check, and probably quickly mention
>> > too,
>> > is whether or not the callers of these functions --the ones inside
>> > xen.git of course-- are prepared to deal with this. I mean,
>> > returning
>> > NULL on failure of xc_version() is, IMO, the right thing to do here
>> > anyway, but it is something important to know whether more work is
>> > needed to fix the callers as well, or if we're just fine.
>> >
>> > To do so, search (e.g., with grep or cscope) for uses of
>> > libxl_get_version_info inside the tools/ dir of xen.git. For
>> > instance,
>> > there is one such use in xl_cmdimpl.c, in the output_xeninfo()
>> > function, which looks like it would interact well with this
>> > patch...
>> > Can you confirm this is the case also for all the other instances
>> > and
>> > note it down here?
>>
>> I think NULL may not be handled properly in auto_autoballoon() in
>> tools/libxl/xl.c Other instances seems okay to me.
>>
> So, here:
>
> info = libxl_get_version_info(ctx);
> if (!info)
> return 1; /* default to on */
>
> Why do you think this is a problem? It looks like this call site is
> actually prepared to see and handle a NULL return value...
>
Sorry I actually meant,
tools/libxl/xl_cmdimpl.c:vinfo = libxl_get_version_info(ctx);
vinfo = libxl_get_version_info(ctx);
if (vinfo) {
i = (1 << 20) / vinfo->pagesize;
printf("total_memory : %"PRIu64"\n", info.total_pages / i);
printf("free_memory: %"PRIu64"\n",
(info.free_pages - info.outstanding_pages) / i);
printf("sharing_freed_memory : %"PRIu64"\n",
info.sharing_freed_pages / i);
printf("sharing_used_memory: %"PRIu64"\n",
info.sharing_used_frames / i);
printf("outstanding_claims : %"PRIu64"\n",
info.outstanding_pages / i);
}
Regards.
>> Should I fix the caller, too?
>>
> If there are issues with them, yes. If there aren't just put something
> like "xxx callers are already fine xxx" in the changelog.
>
> Regards,
> Dario
> --
> <> (Raistlin Majere)
> -
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] Anyone know where the official qemu(1) man pages are hosted?
On Fri, 12 Feb 2016, Lars Kurth wrote: > I noticed that the qemu(1) links in > http://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html (and probably other > docs) are broken. They go to http://man.he.net/man1/qemu, which does not > exist any more It doesn't look like QEMU maintains a traditional man page, but it has a "QEMU Emulator User Documentation" html generated from source: http://qemu.weilnetz.de/qemu-doc.html ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86: Fix build following c/s 623c720f "x86: use CLFLUSHOPT when available"
On 12/02/16 10:57, Jan Beulich wrote: On 12.02.16 at 11:50, wrote: >> On 12/02/16 10:12, Jan Beulich wrote: >> On 12.02.16 at 11:02, wrote: On 12/02/16 10:00, Jan Beulich wrote: On 12.02.16 at 10:51, wrote: >> On 12/02/16 08:23, Jan Beulich wrote: >> On 11.02.16 at 20:25, wrote: CentOS 7 gets into trouble when compiling Xen citing: flushtlb.c: Assembler messages: flushtlb.c:149: Error: value of 256 too large for field of 1 bytes at 1 The line number is wrong, and the error message not helpful. It turns out that the intermediate generated assembly was # 139 "arch/x86/flushtlb.c" 1 661: rex clflush (%r15) 662: .pushsection .altinstructions,"a" and it was having trouble combining the explicit REX prefix with the REX.B required for the use of %r15. >>> What gas version is this? I just checked with 2.20, which has no >>> problem combining an explicit with a generated REX prefix. >> bash-4.2$ as --version >> GNU assembler version 2.23.52.0.1-30.el7_1.2 20130226 >> >> >>> Or >>> wait, no, your description of the issue is wrong: It actually is the >>> folding of the two REX prefixes which causes the problem, >> This is what I said. What do you think I meant with "trouble combining >> the" ? > Argh - I meant to say "It actually isn't ...". > >>> since >>> that results in the replacement instruction being one byte longer >>> than the to be replaced one. >> But that is still the case with an explicit %ds override. The assembler >> still has to insert an extra byte somehow. > No. We now always have one non-REX prefix, and both instructions > will have the same REX/ModRM/SIB encoding. I see now, given your wording on the patch committed. In hindsight this should have been obvious, but GCCs error message was particularly unhelpful at diagnosing the issue. >>> It was actually an assembler error message, and I can't really see >>> how we could improve that (since afaict the intended checking can >>> only be done at assembly time). >> Oh right. It is an assembler BUILD_BUG_ON(). >> >> Is there anything useful we can do to get the error message to properly >> point at alternative.h: DISCARD_ENTRY()? That would at least have helped. > I'd like to, but I can't see how, not the least because ... > >> As it stood, the actual reported error line was a closing brace after >> the wbinvd() call. > ... I've also noticed that, but the assembler would just use whatever > line directive the compiler had put in the assembly file. Hmm. Well, the one saving grace is that if you google the error message, you now get to this thread. Hopefully this might help someone else out of a similar situation in the future. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH net-next v1 0/8] xen-netback: support toeplitz hashing
> -Original Message- > From: netdev-ow...@vger.kernel.org [mailto:netdev- > ow...@vger.kernel.org] On Behalf Of David Miller > Sent: 12 February 2016 10:54 > To: Paul Durrant > Cc: net...@vger.kernel.org; xen-de...@lists.xenproject.org > Subject: Re: [PATCH net-next v1 0/8] xen-netback: support toeplitz hashing > > From: Paul Durrant > Date: Fri, 12 Feb 2016 10:13:17 + > > > This patch series adds support for frontend-configurable toeplitz hashing > > in xen-netback (on the guest receive side). This support has been testing > > against a Windows frontend and has proven to be sufficient to pass the > > Microsoft HCK NDIS RSS tests. > > We want less Toeplitz users, not more of them. It is a more unsuitable > hash than jhash2. > > I'm not applying these patches, sorry. Windows *requires* use of Teoplitz so your position completely rules out being able to support receive side scaling in Windows PV frontends on Linux kernel backends, not only for Xen but for any other hypervisor, which I think is totally unreasonable. Paul ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3] libxc: fix leak of t_info in xc_tbuf_get_size()
Avoid leaking the memory mapping of the trace buffer Coverity ID 1351228 Signed-off-by: Harmandeep Kaur Reviewed-by: Dario Faggioli --- v2: call to unmapping function reduced to one from two v3: passed correct argument sysctl.u.tbuf_op.size in xenforeignmemory_unmap() --- tools/libxc/xc_tbuf.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/libxc/xc_tbuf.c b/tools/libxc/xc_tbuf.c index 695939a..283fbd1 100644 --- a/tools/libxc/xc_tbuf.c +++ b/tools/libxc/xc_tbuf.c @@ -70,11 +70,13 @@ int xc_tbuf_get_size(xc_interface *xch, unsigned long *size) sysctl.u.tbuf_op.buffer_mfn); if ( t_info == NULL || t_info->tbuf_size == 0 ) -return -1; +rc = -1; +else + *size = t_info->tbuf_size; -*size = t_info->tbuf_size; +xenforeignmemory_unmap(xch->fmem, t_info, sysctl.u.tbuf_op.size); -return 0; +return rc; } int xc_tbuf_enable(xc_interface *xch, unsigned long pages, unsigned long *mfn, -- 2.5.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: fix handling returns in libxl_get_version_info()
On Fri, 2016-02-12 at 16:34 +0530, Harmandeep Kaur wrote:
> On Fri, Feb 12, 2016 at 4:30 PM, Dario Faggioli
> wrote:
> >
> Sorry I actually meant,
> tools/libxl/xl_cmdimpl.c:vinfo = libxl_get_version_info(ctx);
>
> vinfo = libxl_get_version_info(ctx);
> if (vinfo) {
>
Which again checks for vinfo to not be NULL before using it, so this
also seems ok to me.
> i = (1 << 20) / vinfo->pagesize;
> printf("total_memory : %"PRIu64"\n",
> info.total_pages / i);
> printf("free_memory: %"PRIu64"\n",
> (info.free_pages - info.outstanding_pages) / i);
> printf("sharing_freed_memory : %"PRIu64"\n",
> info.sharing_freed_pages / i);
> printf("sharing_used_memory: %"PRIu64"\n",
> info.sharing_used_frames / i);
> printf("outstanding_claims : %"PRIu64"\n",
> info.outstanding_pages / i);
> }
>
Dario
--
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
On Thu, 11 Feb 2016, Konrad Rzeszutek Wilk wrote: > Otherwise any code that tries to use Elf_* macros instead of > Elf32_ or Elf_64 fails to compile. > > CC: ian.campb...@citrix.com > CC: wei.l...@citrix.com > CC: stefano.stabell...@citrix.com > Signed-off-by: Konrad Rzeszutek Wilk > --- > xen/include/asm-arm/config.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h > index bd832df..4ea66bf 100644 > --- a/xen/include/asm-arm/config.h > +++ b/xen/include/asm-arm/config.h > @@ -15,8 +15,10 @@ > > #if defined(CONFIG_ARM_64) > # define LONG_BYTEORDER 3 > +# define ELFSIZE 64 > #else > # define LONG_BYTEORDER 2 > +# define ELFSIZE 32 > #endif I wonder if we should use ELF64 on ARM32 too, for simplicity (x86 only uses ELF64) and because ARM32 is LPAE. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] libxl: fix handling of returns in libxl_get_version_info()
Check the return value of xc_version() and return NULL if it fails. libxl_get_version_info() can also return NULL now. Callers of the function libxl_get_version_info() are already prepared to deal with returning NULL on failure of xc_version(). Coverity ID 1351217 Signed-off-by: Harmandeep Kaur --- v2: Change local variable rc to r. Remove xen_version. Better readiblity of blocks of code. --- tools/libxl/libxl.c | 32 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 2d18b8d..771cc40 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -5267,42 +5267,50 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx) xen_platform_parameters_t p_parms; xen_commandline_t xen_commandline; } u; -long xen_version; +long r = 0; libxl_version_info *info = &ctx->version_info; if (info->xen_version_extra != NULL) goto out; -xen_version = xc_version(ctx->xch, XENVER_version, NULL); -info->xen_version_major = xen_version >> 16; -info->xen_version_minor = xen_version & 0xFF; +r = xc_version(ctx->xch, XENVER_version, NULL); +if ( r < 0 ) goto out; +info->xen_version_major = r >> 16; +info->xen_version_minor = r & 0xFF; -xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra); +r = xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra); +if ( r < 0 ) goto out; info->xen_version_extra = libxl__strdup(NOGC, u.xen_extra); -xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc); +r = xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc); +if ( r < 0 ) goto out; info->compiler = libxl__strdup(NOGC, u.xen_cc.compiler); info->compile_by = libxl__strdup(NOGC, u.xen_cc.compile_by); info->compile_domain = libxl__strdup(NOGC, u.xen_cc.compile_domain); info->compile_date = libxl__strdup(NOGC, u.xen_cc.compile_date); -xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps); +r = xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps); +if ( r < 0 ) goto out; info->capabilities = libxl__strdup(NOGC, u.xen_caps); -xc_version(ctx->xch, XENVER_changeset, &u.xen_chgset); +r = xc_version(ctx->xch, XENVER_changeset, &u.xen_chgset); +if ( r < 0 ) goto out; info->changeset = libxl__strdup(NOGC, u.xen_chgset); -xc_version(ctx->xch, XENVER_platform_parameters, &u.p_parms); +r = xc_version(ctx->xch, XENVER_platform_parameters, &u.p_parms); +if ( r < 0 ) goto out; info->virt_start = u.p_parms.virt_start; -info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL); +r = info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL); +if ( r < 0 ) goto out; -xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline); +r = xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline); +if ( r < 0 ) goto out; info->commandline = libxl__strdup(NOGC, u.xen_commandline); out: GC_FREE; -return info; +return r < 0 ? NULL:info; } libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid, -- 2.5.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
On Thu, 11 Feb 2016, Konrad Rzeszutek Wilk wrote: > in the keyhandler.h file. Otherwise on ARM builds if we > just use the keyhandler file - the compile will fail. > > CC: ian.campb...@citrix.com > CC: wei.l...@citrix.com > CC: stefano.stabell...@citrix.com > Signed-off-by: Konrad Rzeszutek Wilk > --- > xen/include/xen/keyhandler.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/include/xen/keyhandler.h b/xen/include/xen/keyhandler.h > index 06c05c8..e361558 100644 > --- a/xen/include/xen/keyhandler.h > +++ b/xen/include/xen/keyhandler.h > @@ -19,6 +19,7 @@ > */ > typedef void (keyhandler_fn_t)(unsigned char key); > > +struct cpu_user_regs; > /* > * Callback type for irq_keyhandler. > * I think that the right fix would be to #include . ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen
On Thu, 11 Feb 2016, Rafael J. Wysocki wrote:
> On Thursday, February 11, 2016 04:04:14 PM Stefano Stabellini wrote:
> > On Wed, 10 Feb 2016, Rafael J. Wysocki wrote:
> > > On Tuesday, February 09, 2016 11:19:02 AM Stefano Stabellini wrote:
> > > > On Mon, 8 Feb 2016, Rafael J. Wysocki wrote:
> > > > > On Monday, February 08, 2016 10:57:01 AM Stefano Stabellini wrote:
> > > > > > On Sat, 6 Feb 2016, Rafael J. Wysocki wrote:
> > > > > > > On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao
> > > > > > > wrote:
> > > > > > > > From: Shannon Zhao
> > > > > > > >
> > > > > > > > ACPI 6.0 introduces a new table STAO to list the devices which
> > > > > > > > are used
> > > > > > > > by Xen and can't be used by Dom0. On Xen virtual platforms, the
> > > > > > > > physical
> > > > > > > > UART is used by Xen. So here it hides UART from Dom0.
> > > > > > > >
> > > > > > > > Signed-off-by: Shannon Zhao
> > > > > > > > Reviewed-by: Stefano Stabellini
> > > > > > > >
> > > > > > >
> > > > > > > Well, this doesn't look right to me.
> > > > > > >
> > > > > > > We need to find a nicer way to achieve what you want.
> > > > > >
> > > > > > I take that you are talking about how to honor the STAO table in
> > > > > > Linux.
> > > > > > Do you have any concrete suggestions?
> > > > >
> > > > > I do.
> > > > >
> > > > > The last hunk of the patch is likely what it needs to be, although I'm
> > > > > not sure if the place it is added to is the right one. That's a
> > > > > minor thing,
> > > > > though.
> > > > >
> > > > > The other part is problematic. Not that as it doesn't work, but
> > > > > because of
> > > > > how it works. With these changes the device will be visible to the
> > > > > OS (in
> > > > > fact to user space even), but will never be "present". I'm not sure
> > > > > if
> > > > > that's what you want?
> > > > >
> > > > > It might be better to add a check to acpi_bus_type_and_status() that
> > > > > will
> > > > > evaluate the "should ignore?" thing and return -ENODEV if this is
> > > > > true. This
> > > > > way the device won't be visible at all.
> > > >
> > > > Something like below? Actually your suggestion is better, thank you!
> > > >
> > > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > > > index 78d5f02..4778c51 100644
> > > > --- a/drivers/acpi/scan.c
> > > > +++ b/drivers/acpi/scan.c
> > > > @@ -1455,6 +1455,9 @@ static int acpi_bus_type_and_status(acpi_handle
> > > > handle, int *type,
> > > > if (ACPI_FAILURE(status))
> > > > return -ENODEV;
> > > >
> > > > + if (acpi_check_device_is_ignored(handle))
> > > > + return -ENODEV;
> > > > +
> > > > switch (acpi_type) {
> > > > case ACPI_TYPE_ANY: /* for ACPI_ROOT_OBJECT */
> > > > case ACPI_TYPE_DEVICE:
> > > >
> > >
> > > I thought about doing that under ACPI_TYPE_DEVICE, because it shouldn't be
> > > applicable to the other types. But generally, yes.
> >
> > I was pondering about it myself. Maybe an ACPI_TYPE_PROCESSOR object
> > could theoretically be hidden with the STAO?
>
> But this patch won't check for it anyway, will it?
>
> It seems to be only checking against the UART address or have I missed
> anything?
You are right, this patch only checks for the UART address, which is
critical.
However the STAO also has a "Name List" field with a list of paths in
ACPI namespace to hide. If not implementing proper "Name List" support,
at least, as part of this patch, it would be nice to check for the
presence of the Name List field in the table, and print a warning such
as "STAO Name List not yet supported" when the field is present.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable-smoke test] 82125: tolerable all pass - PUSHED
flight 82125 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/82125/
Failures :-/ but no regressions.
Tests which did not succeed, but are not blocking:
test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass
test-armhf-armhf-xl 12 migrate-support-checkfail never pass
test-armhf-armhf-xl 13 saverestore-support-checkfail never pass
version targeted for testing:
xen 7ffb9b0cda75479471afa90cf44763af0020493f
baseline version:
xen 1716be47617c1ef18189607448a9242e533cd6f7
Last test of basis82022 2016-02-11 19:04:02 Z0 days
Testing same since82125 2016-02-12 10:13:01 Z0 days1 attempts
People who touched revisions under test:
Andrew Cooper
Doug Goldstein
Jan Beulich
Konrad Rzeszutek Wilk
jobs:
build-amd64 pass
build-armhf pass
build-amd64-libvirt pass
test-armhf-armhf-xl pass
test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
test-amd64-amd64-libvirt pass
sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images
Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs
Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master
Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary
Pushing revision :
+ branch=xen-unstable-smoke
+ revision=7ffb9b0cda75479471afa90cf44763af0020493f
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
getconfig Repos
perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke
7ffb9b0cda75479471afa90cf44763af0020493f
+ branch=xen-unstable-smoke
+ revision=7ffb9b0cda75479471afa90cf44763af0020493f
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
getconfig Repos
perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-unstable-coverity
+ '[' x7ffb9b0cda75479471afa90cf44763af0020493f = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git
+++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src
'[fetch=try]'
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local 'options=[fetch=try]'
getconfig GitCacheProxy
perl -e '
use Osstest;
readglobalconfig();
print $c{"GitCacheProxy"} or die $!;
'
+++ local cache=git://cache:9419/
+++ '[' xgit://
Re: [Xen-devel] [PATCH] vm_event: Remove xc_mem_access_enable_emulate() and friends
On Fri, Feb 12, 2016 at 11:04:34AM +0200, Razvan Cojocaru wrote: > xc_mem_access_enable_emulate() and xc_mem_access_disable_emulate() > are currently no-ops, that is all they do is set a flag that > nobody else checks. The user can already set the EMULATE flags in > the vm_event response if emulation is desired, and having an extra > check above that is not inherently safer, but it does complicate > (currenly unnecessarily) the API. This patch removes these > functions and the corresponding hypervisor code. > > Signed-off-by: Razvan Cojocaru > --- > tools/libxc/include/xenctrl.h | 11 --- > tools/libxc/xc_mem_access.c | 24 The changes to libxc look good to me. I'm waiting for v2 to actually ack it. Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
>>> On 12.02.16 at 12:37, wrote: > On Thu, 11 Feb 2016, Konrad Rzeszutek Wilk wrote: >> --- a/xen/include/xen/keyhandler.h >> +++ b/xen/include/xen/keyhandler.h >> @@ -19,6 +19,7 @@ >> */ >> typedef void (keyhandler_fn_t)(unsigned char key); >> >> +struct cpu_user_regs; >> /* >> * Callback type for irq_keyhandler. >> * > > I think that the right fix would be to #include . I disagree - for one this isn't where the structure gets defined, and then this is the "include everything everywhere" attitude which tends to needlessly slow down builds (avoiding the need to include everything everywhere is actually one of the purposes of such forward declarations, which allows going as far as having the full definition in just a single source file). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 3/6] xen: factor out allocation of page tables into separate function
On 11.02.2016 13:53, Juergen Gross wrote:
> On 11/02/16 13:27, Daniel Kiper wrote:
>> On Thu, Feb 11, 2016 at 08:53:23AM +0100, Juergen Gross wrote:
>>> Do the allocation of page tables in a separate function. This will
>>> allow to do the allocation at different times of the boot preparations
>>> depending on the features the kernel is supporting.
>>>
>>> Signed-off-by: Juergen Gross
>>> ---
>>> grub-core/loader/i386/xen.c | 82
>>> -
>>> 1 file changed, 51 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
>>> index e48cc3f..65cec27 100644
>>> --- a/grub-core/loader/i386/xen.c
>>> +++ b/grub-core/loader/i386/xen.c
>>> @@ -56,6 +56,9 @@ static struct grub_relocator_xen_state state;
>>> static grub_xen_mfn_t *virt_mfn_list;
>>> static struct start_info *virt_start_info;
>>> static grub_xen_mfn_t console_pfn;
>>> +static grub_uint64_t *virt_pgtable;
>>> +static grub_uint64_t pgtbl_start;
>>> +static grub_uint64_t pgtbl_end;
>>
>> Same as in patches #1 and #2.
>
> Yep.
>
>>
>>> #define PAGE_SIZE 4096
>>> #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
>>> @@ -106,17 +109,17 @@ get_pgtable_size (grub_uint64_t total_pages,
>>> grub_uint64_t virt_base)
>>>
>>> static void
>>> generate_page_table (grub_uint64_t *where, grub_uint64_t paging_start,
>>> -grub_uint64_t total_pages, grub_uint64_t virt_base,
>>> -grub_xen_mfn_t *mfn_list)
>>> +grub_uint64_t paging_end, grub_uint64_t total_pages,
>>> +grub_uint64_t virt_base, grub_xen_mfn_t *mfn_list)
>>> {
>>>if (!virt_base)
>>> -total_pages++;
>>> +paging_end++;
>>>
>>>grub_uint64_t lx[NUMBER_OF_LEVELS], lxs[NUMBER_OF_LEVELS];
>>>grub_uint64_t nlx, nls, sz = 0;
>>>int l;
>>>
>>> - nlx = total_pages;
>>> + nlx = paging_end;
>>>nls = virt_base >> PAGE_SHIFT;
>>>for (l = 0; l < NUMBER_OF_LEVELS; l++)
>>> {
>>> @@ -160,7 +163,7 @@ generate_page_table (grub_uint64_t *where,
>>> grub_uint64_t paging_start,
>>>if (pr)
>>> pg += POINTERS_PER_PAGE;
>>>
>>> - for (j = 0; j < total_pages; j++)
>>> + for (j = 0; j < paging_end; j++)
>>> {
>>>if (j >= paging_start && j < lp)
>>> pg[j + lxs[0]] = page2offset (mfn_list[j]) | 5;
>>> @@ -261,24 +264,12 @@ grub_xen_special_alloc (void)
>>> }
>>>
>>> static grub_err_t
>>> -grub_xen_boot (void)
>>> +grub_xen_pt_alloc (void)
>>> {
>>>grub_relocator_chunk_t ch;
>>>grub_err_t err;
>>>grub_uint64_t nr_info_pages;
>>>grub_uint64_t nr_pages, nr_pt_pages, nr_need_pages;
>>> - struct gnttab_set_version gnttab_setver;
>>> - grub_size_t i;
>>> -
>>> - if (grub_xen_n_allocated_shared_pages)
>>> -return grub_error (GRUB_ERR_BUG, "active grants");
>>> -
>>> - err = grub_xen_p2m_alloc ();
>>> - if (err)
>>> -return err;
>>> - err = grub_xen_special_alloc ();
>>> - if (err)
>>> -return err;
>>>
>>>next_start.pt_base = max_addr + xen_inf.virt_base;
>>>state.paging_start = max_addr >> PAGE_SHIFT;
>>> @@ -298,30 +289,59 @@ grub_xen_boot (void)
>>>nr_pages = nr_need_pages;
>>> }
>>>
>>> - grub_dprintf ("xen", "bootstrap domain %llx+%llx\n",
>>> - (unsigned long long) xen_inf.virt_base,
>>> - (unsigned long long) page2offset (nr_pages));
>>> -
>>>err = grub_relocator_alloc_chunk_addr (relocator, &ch,
>>> max_addr, page2offset (nr_pt_pages));
>>>if (err)
>>> return err;
>>>
>>> + virt_pgtable = get_virtual_current_address (ch);
>>> + pgtbl_start = max_addr >> PAGE_SHIFT;
>>> + max_addr += page2offset (nr_pt_pages);
>>> + state.stack = max_addr + STACK_SIZE + xen_inf.virt_base;
>>> + state.paging_size = nr_pt_pages;
>>> + next_start.nr_pt_frames = nr_pt_pages;
>>> + max_addr = page2offset (nr_pages);
>>> + pgtbl_end = nr_pages;
>>> +
>>> + return GRUB_ERR_NONE;
>>> +}
>>> +
>>> +static grub_err_t
>>> +grub_xen_boot (void)
>>> +{
>>> + grub_err_t err;
>>> + grub_uint64_t nr_pages;
>>> + struct gnttab_set_version gnttab_setver;
>>> + grub_size_t i;
>>> +
>>> + if (grub_xen_n_allocated_shared_pages)
>>> +return grub_error (GRUB_ERR_BUG, "active grants");
>>> +
>>> + err = grub_xen_p2m_alloc ();
>>> + if (err)
>>> +return err;
>>> + err = grub_xen_special_alloc ();
>>> + if (err)
>>> +return err;
>>> + err = grub_xen_pt_alloc ();
>>> + if (err)
>>> +return err;
>>
>> Should not we free memory allocated by grub_xen_p2m_alloc() and
>> grub_xen_special_alloc() if grub_xen_pt_alloc() failed?
>
> Hmm, why? If I don't miss anything freeing memory in case of error isn't
> done anywhere (at least not in this source file).
>
If we don't it's a bug and not an excuse to continue doing bad things
> Juergen
>
> .
>
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing lis
Re: [Xen-devel] [PATCH v2 4/6] xen: add capability to load initrd outside of initial mapping
On 11.02.2016 15:13, Juergen Gross wrote:
> On 11/02/16 13:33, Daniel Kiper wrote:
>> On Thu, Feb 11, 2016 at 08:53:24AM +0100, Juergen Gross wrote:
>>> Modern pvops linux kernels support an initrd not covered by the initial
>>> mapping. This capability is flagged by an elf-note.
>>>
>>> In case the elf-note is set by the kernel don't place the initrd into
>>> the initial mapping. This will allow to load larger initrds and/or
>>> support domains with larger memory, as the initial mapping is limited
>>> to 2GB and it is containing the p2m list.
>>>
>>> Signed-off-by: Juergen Gross
>>> ---
>>> grub-core/loader/i386/xen.c| 56
++
>>> grub-core/loader/i386/xen_fileXX.c | 3 ++
>>> include/grub/xen_file.h| 1 +
>>> 3 files changed, 49 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
>>> index 65cec27..0f41048 100644
>>> --- a/grub-core/loader/i386/xen.c
>>> +++ b/grub-core/loader/i386/xen.c
>>> @@ -307,15 +307,14 @@ grub_xen_pt_alloc (void)
>>> }
>>>
>>> static grub_err_t
>>> -grub_xen_boot (void)
>>> +grub_xen_alloc_end (void)
>>> {
>>>grub_err_t err;
>>> - grub_uint64_t nr_pages;
>>> - struct gnttab_set_version gnttab_setver;
>>> - grub_size_t i;
>>> + static int called = 0;
>>>
>>> - if (grub_xen_n_allocated_shared_pages)
>>> -return grub_error (GRUB_ERR_BUG, "active grants");
>>> + if (called)
>>> +return GRUB_ERR_NONE;
>>
>> Why?
>
> Did you look at the function? grub_xen_alloc_end() (some parts of the
> original grub_xen_boot()) is new and may be called multiple times. It
> was much easier to just call it and let it do what must be done only
> at the first time called.
>
What if a boot fails and then fallback kernel is loaded?
>>> + if (grub_xen_n_allocated_shared_pages)
>>> +return grub_error (GRUB_ERR_BUG, "active grants");
>>
>> Why?
>
> That's how it has been before. git just decided to generate the diff
> that way.
>
This is also needed to avoid passing control when some drivers are still
active
>>> + case 16:
>>
>> Could you define this a constant and use it here?
>
> This would be the only case with a constant. All others are numeric
> as well.
>
I'm ok with not insisisting on using constants given current state but
in general constants are preferable (yes, xen code isn't always clean)
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] libxl: fix handling of returns in libxl_get_version_info()
On Fri, Feb 12, 2016 at 05:00:40PM +0530, Harmandeep Kaur wrote:
> Check the return value of xc_version() and return NULL if it
> fails. libxl_get_version_info() can also return NULL now.
> Callers of the function libxl_get_version_info() are already
> prepared to deal with returning NULL on failure of xc_version().
>
> Coverity ID 1351217
>
> Signed-off-by: Harmandeep Kaur
> ---
> v2: Change local variable rc to r. Remove xen_version.
> Better readiblity of blocks of code.
> ---
> tools/libxl/libxl.c | 32
> 1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 2d18b8d..771cc40 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5267,42 +5267,50 @@ const libxl_version_info*
> libxl_get_version_info(libxl_ctx *ctx)
> xen_platform_parameters_t p_parms;
> xen_commandline_t xen_commandline;
> } u;
> -long xen_version;
> +long r = 0;
> libxl_version_info *info = &ctx->version_info;
>
> if (info->xen_version_extra != NULL)
> goto out;
>
> -xen_version = xc_version(ctx->xch, XENVER_version, NULL);
> -info->xen_version_major = xen_version >> 16;
> -info->xen_version_minor = xen_version & 0xFF;
> +r = xc_version(ctx->xch, XENVER_version, NULL);
> +if ( r < 0 ) goto out;
I know you're following Ian's suggestion, but examples in CODING_STYLE
don't have space after "(" and before ")".
> +info->xen_version_major = r >> 16;
> +info->xen_version_minor = r & 0xFF;
>
> -xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra);
> +r = xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra);
> +if ( r < 0 ) goto out;
> info->xen_version_extra = libxl__strdup(NOGC, u.xen_extra);
>
> -xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc);
> +r = xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc);
> +if ( r < 0 ) goto out;
At the beginning of this function it checks if info->xen_version_extra
is not NULL.
You can now get into a state where partial information is cached. This
is buggy.
Not that the original implementation is any better, but if you're going
to fix it, try not to introduce new bug with your fix. :-)
I think you can rollback the caching by freeing up any resources before
returning.
Wei.
> info->compiler = libxl__strdup(NOGC, u.xen_cc.compiler);
> info->compile_by = libxl__strdup(NOGC, u.xen_cc.compile_by);
> info->compile_domain = libxl__strdup(NOGC, u.xen_cc.compile_domain);
> info->compile_date = libxl__strdup(NOGC, u.xen_cc.compile_date);
>
> -xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps);
> +r = xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps);
> +if ( r < 0 ) goto out;
> info->capabilities = libxl__strdup(NOGC, u.xen_caps);
>
> -xc_version(ctx->xch, XENVER_changeset, &u.xen_chgset);
> +r = xc_version(ctx->xch, XENVER_changeset, &u.xen_chgset);
> +if ( r < 0 ) goto out;
> info->changeset = libxl__strdup(NOGC, u.xen_chgset);
>
> -xc_version(ctx->xch, XENVER_platform_parameters, &u.p_parms);
> +r = xc_version(ctx->xch, XENVER_platform_parameters, &u.p_parms);
> +if ( r < 0 ) goto out;
> info->virt_start = u.p_parms.virt_start;
>
> -info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL);
> +r = info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL);
> +if ( r < 0 ) goto out;
>
> -xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline);
> +r = xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline);
> +if ( r < 0 ) goto out;
> info->commandline = libxl__strdup(NOGC, u.xen_commandline);
>
> out:
> GC_FREE;
> -return info;
> +return r < 0 ? NULL:info;
> }
>
> libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
> --
> 2.5.0
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] libxc: fix leak of t_info in xc_tbuf_get_size()
On Fri, Feb 12, 2016 at 04:38:32PM +0530, Harmandeep Kaur wrote: > Avoid leaking the memory mapping of the trace buffer > > Coverity ID 1351228 > > Signed-off-by: Harmandeep Kaur > Reviewed-by: Dario Faggioli Acked-by: Wei Liu Thanks ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
On Fri, 12 Feb 2016, Jan Beulich wrote: > >>> On 12.02.16 at 12:37, wrote: > > On Thu, 11 Feb 2016, Konrad Rzeszutek Wilk wrote: > >> --- a/xen/include/xen/keyhandler.h > >> +++ b/xen/include/xen/keyhandler.h > >> @@ -19,6 +19,7 @@ > >> */ > >> typedef void (keyhandler_fn_t)(unsigned char key); > >> > >> +struct cpu_user_regs; > >> /* > >> * Callback type for irq_keyhandler. > >> * > > > > I think that the right fix would be to #include . > > I disagree - for one this isn't where the structure gets defined, Ah, sorry, it is on ARM (actually to be precise, the header files are asm-arm/arm64/processor.h and asm-arm/arm32/processor.h, included by asm-arm/processor.h depending on the architecture). I see now that the corresponding x86 header is actually arch-x86/xen.h. > and then this is the "include everything everywhere" attitude > which tends to needlessly slow down builds (avoiding the need > to include everything everywhere is actually one of the > purposes of such forward declarations, which allows going as > far as having the full definition in just a single source file). Fair enough, but keyhandler.h directly uses struct cpu_user_regs as parameter in two functions, so I think it would be right to include an header file with the definition. It's too bad that the names for the ARM headers and the x86 headers don't match. Given that, I understand why Konrad went with this solution instead, and I am OK with it. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] vm_event: consolidate hvm_event_fill_regs and p2m_vm_event_fill_regs
On Feb 12, 2016 03:41, "Jan Beulich" wrote:
>
> >>> On 12.02.16 at 11:19, wrote:
> > On 02/12/2016 11:57 AM, Jan Beulich wrote:
> > On 12.02.16 at 01:22, wrote:
> >>> --- a/xen/arch/x86/vm_event.c
> >>> +++ b/xen/arch/x86/vm_event.c
> >>> @@ -122,6 +122,65 @@ void vm_event_set_registers(struct vcpu *v,
> > vm_event_response_t *rsp)
> >>> v->arch.user_regs.eip = rsp->data.regs.x86.rip;
> >>> }
> >>>
> >>> +void vm_event_fill_regs(vm_event_request_t *req)
> >>> +{
> >>> +const struct cpu_user_regs *regs = guest_cpu_user_regs();
> >>> +struct segment_register seg;
> >>> +struct hvm_hw_cpu ctxt;
> >>> +struct vcpu *curr = current;
> >>> +
> >>> +req->data.regs.x86.rax = regs->eax;
> >>> +req->data.regs.x86.rcx = regs->ecx;
> >>> +req->data.regs.x86.rdx = regs->edx;
> >>> +req->data.regs.x86.rbx = regs->ebx;
> >>> +req->data.regs.x86.rsp = regs->esp;
> >>> +req->data.regs.x86.rbp = regs->ebp;
> >>> +req->data.regs.x86.rsi = regs->esi;
> >>> +req->data.regs.x86.rdi = regs->edi;
> >>> +
> >>> +req->data.regs.x86.r8 = regs->r8;
> >>> +req->data.regs.x86.r9 = regs->r9;
> >>> +req->data.regs.x86.r10 = regs->r10;
> >>> +req->data.regs.x86.r11 = regs->r11;
> >>> +req->data.regs.x86.r12 = regs->r12;
> >>> +req->data.regs.x86.r13 = regs->r13;
> >>> +req->data.regs.x86.r14 = regs->r14;
> >>> +req->data.regs.x86.r15 = regs->r15;
> >>> +
> >>> +req->data.regs.x86.rflags = regs->eflags;
> >>> +req->data.regs.x86.rip= regs->eip;
> >>> +
> >>> +if ( !is_hvm_domain(curr->domain) )
> >>> +return;
> >>
> >> No such check existed in either of the two original functions. Why is
> >> it needed all of the sudden? And if it is needed, why do the other
> >> fields not get filled (as far as possible at least) for PV guests?
> >
> > I can't speak for Tamas, but I suspect the check has been placed there
> > because calls to hvm_funcs.save_cpu_ctxt(curr, &ctxt) and
> > hvm_get_segment_register(curr, x86_seg_fs, &seg) follow, and he's put
> > vm_event_fill_regs() in xen/arch/x86/vm_event.c (a previous function was
> > called hvm_event_fill_regs(), in arch/x86/hvm/event.c, so no checking
> > for HVM was needed).
> >
> > I don't think the check is needed for the current codepaths, but since
> > the code has been moved to xen/arch/x86/ the question about future PV
> > events is fair.
That was the idea.
>
> In which case ASSERT(is_hvm_vcpu(curr)) would be the common
> way to document this (at once avoiding the open coding of
> is_hvm_vcpu()).
>
I don't think we need an assert here. The function is fine for pv guests as
well up to that point. Filling in the rest of the registers for pv guests
can be done when pv events are implemented. Maybe a comment saying so is
warranted.
Tamas
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] hvm/vmx: save dr7 during vmx_vmcs_save
On Feb 12, 2016 02:12, "Jan Beulich" wrote: > > >>> On 12.02.16 at 01:22, wrote: > > Sending the dr7 register during vm_events is useful for various applications, > > but the current way the register value is gathered is incorrent. In this > > patch > > we extend vmx_vmcs_save so that we get the correct value. > > > > Suggested-by: Andrew Cooper > > Iirc Andrew suggested ... > > > --- a/xen/arch/x86/hvm/vmx/vmx.c > > +++ b/xen/arch/x86/hvm/vmx/vmx.c > > @@ -490,6 +490,7 @@ static void vmx_vmcs_save(struct vcpu *v, struct hvm_hw_cpu *c) > > __vmread(GUEST_SYSENTER_CS, &c->sysenter_cs); > > __vmread(GUEST_SYSENTER_ESP, &c->sysenter_esp); > > __vmread(GUEST_SYSENTER_EIP, &c->sysenter_eip); > > +__vmread(GUEST_DR7, &c->dr7); > > ... just when v == current. > Would that check really be necessary? It would complicate the code not just here but the caller would need to be aware too that in that case dr7 can be aquired from someplace else. I don't see the harm in just saving dr7 here in both cases. Tamas ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen
On Fri, Feb 12, 2016 at 12:50 PM, Stefano Stabellini
wrote:
> On Thu, 11 Feb 2016, Rafael J. Wysocki wrote:
>> On Thursday, February 11, 2016 04:04:14 PM Stefano Stabellini wrote:
>> > On Wed, 10 Feb 2016, Rafael J. Wysocki wrote:
>> > > On Tuesday, February 09, 2016 11:19:02 AM Stefano Stabellini wrote:
>> > > > On Mon, 8 Feb 2016, Rafael J. Wysocki wrote:
>> > > > > On Monday, February 08, 2016 10:57:01 AM Stefano Stabellini wrote:
>> > > > > > On Sat, 6 Feb 2016, Rafael J. Wysocki wrote:
>> > > > > > > On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao
>> > > > > > > wrote:
>> > > > > > > > From: Shannon Zhao
>> > > > > > > >
>> > > > > > > > ACPI 6.0 introduces a new table STAO to list the devices which
>> > > > > > > > are used
>> > > > > > > > by Xen and can't be used by Dom0. On Xen virtual platforms,
>> > > > > > > > the physical
>> > > > > > > > UART is used by Xen. So here it hides UART from Dom0.
>> > > > > > > >
>> > > > > > > > Signed-off-by: Shannon Zhao
>> > > > > > > > Reviewed-by: Stefano Stabellini
>> > > > > > > >
>> > > > > > >
>> > > > > > > Well, this doesn't look right to me.
>> > > > > > >
>> > > > > > > We need to find a nicer way to achieve what you want.
>> > > > > >
>> > > > > > I take that you are talking about how to honor the STAO table in
>> > > > > > Linux.
>> > > > > > Do you have any concrete suggestions?
>> > > > >
>> > > > > I do.
>> > > > >
>> > > > > The last hunk of the patch is likely what it needs to be, although
>> > > > > I'm
>> > > > > not sure if the place it is added to is the right one. That's a
>> > > > > minor thing,
>> > > > > though.
>> > > > >
>> > > > > The other part is problematic. Not that as it doesn't work, but
>> > > > > because of
>> > > > > how it works. With these changes the device will be visible to the
>> > > > > OS (in
>> > > > > fact to user space even), but will never be "present". I'm not sure
>> > > > > if
>> > > > > that's what you want?
>> > > > >
>> > > > > It might be better to add a check to acpi_bus_type_and_status() that
>> > > > > will
>> > > > > evaluate the "should ignore?" thing and return -ENODEV if this is
>> > > > > true. This
>> > > > > way the device won't be visible at all.
>> > > >
>> > > > Something like below? Actually your suggestion is better, thank you!
>> > > >
>> > > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>> > > > index 78d5f02..4778c51 100644
>> > > > --- a/drivers/acpi/scan.c
>> > > > +++ b/drivers/acpi/scan.c
>> > > > @@ -1455,6 +1455,9 @@ static int acpi_bus_type_and_status(acpi_handle
>> > > > handle, int *type,
>> > > > if (ACPI_FAILURE(status))
>> > > > return -ENODEV;
>> > > >
>> > > > + if (acpi_check_device_is_ignored(handle))
>> > > > + return -ENODEV;
>> > > > +
>> > > > switch (acpi_type) {
>> > > > case ACPI_TYPE_ANY: /* for ACPI_ROOT_OBJECT */
>> > > > case ACPI_TYPE_DEVICE:
>> > > >
>> > >
>> > > I thought about doing that under ACPI_TYPE_DEVICE, because it shouldn't
>> > > be
>> > > applicable to the other types. But generally, yes.
>> >
>> > I was pondering about it myself. Maybe an ACPI_TYPE_PROCESSOR object
>> > could theoretically be hidden with the STAO?
>>
>> But this patch won't check for it anyway, will it?
>>
>> It seems to be only checking against the UART address or have I missed
>> anything?
>
> You are right, this patch only checks for the UART address, which is
> critical.
>
> However the STAO also has a "Name List" field with a list of paths in
> ACPI namespace to hide. If not implementing proper "Name List" support,
> at least, as part of this patch, it would be nice to check for the
> presence of the Name List field in the table, and print a warning such
> as "STAO Name List not yet supported" when the field is present.
That would be fine, but anyway it belongs to the table parsing part IMO.
And the check can be moved when adding proper support for the name list part.
Thanks,
Rafael
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] libxl: fix handling of returns in libxl_get_version_info()
On Fri, 2016-02-12 at 12:31 +, Wei Liu wrote: > On Fri, Feb 12, 2016 at 05:00:40PM +0530, Harmandeep Kaur wrote: > > > > +info->xen_version_major = r >> 16; > > +info->xen_version_minor = r & 0xFF; > > > > -xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra); > > +r = xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra); > > +if ( r < 0 ) goto out; > > info->xen_version_extra = libxl__strdup(NOGC, u.xen_extra); > > > > -xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc); > > +r = xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc); > > +if ( r < 0 ) goto out; > > At the beginning of this function it checks if info- > >xen_version_extra > is not NULL. > > You can now get into a state where partial information is cached. > This > is buggy. > Yep, I saw this, and figured out it is not ideal. I thought that, as you say, original code was bad in this respect already, and that we should fix that independently. However... > Not that the original implementation is any better, but if you're > going > to fix it, try not to introduce new bug with your fix. :-) > > I think you can rollback the caching by freeing up any resources > before > returning. > ...you're right, it's probably simple enough to fix both issues, that we should just take the chance. So, Harmandeep, can you take care of this issue Wei is rising as well? Thanks and Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
On Fri, Feb 12, 2016 at 01:51:28AM -0700, Jan Beulich wrote: > >>> On 12.02.16 at 04:08, wrote: > > in the keyhandler.h file. Otherwise on ARM builds if we > > just use the keyhandler file - the compile will fail. > > > > CC: ian.campb...@citrix.com > > CC: wei.l...@citrix.com > > CC: stefano.stabell...@citrix.com > > Signed-off-by: Konrad Rzeszutek Wilk > > See commit d1d181328d. Weird. When I did git rebase origin/staging it didn't notice that commit. And it just applied it on top - so I had two 'struct cpu_regs;' in the header file. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] xl dev-detach hangs with missing frontends
CC'ing other tools maintainer. On Thu, Feb 11, 2016 at 11:37:49AM +0100, Olaf Hering wrote: > How should libxl__initiate_device_generic_remove deal with devices which I think you meant libxl__initiate_device_remove. There is no function called libxl__initiate_device_generic _remove. > have no frontend driver? Right now it moves "state" from either > XenbusStateInitialising or XenbusStateInitWait to XenbusStateClosing. > Then it expects the backend to move "state" to XenbusStateClosed. This > will never happen, at least for netback and scsiback. The result is a 10 > second delay. > I don't think there is a way to tell whether there is no frontend driver or the frontend driver is just too slow. Do you think the timeout is too long? Wei. > > # xl - network-detach $domU $mac > 2016-02-11 11:33:19 CET [20318] libxl: debug: > libxl.c:4614:libxl_device_nic_remove: ao 0x19b3870: create: how=(nil) > callback=(nil) poller=0x19b0200 > 2016-02-11 11:33:19 CET [20318] libxl: debug: > libxl_event.c:636:libxl__ev_xswatch_register: watch w=0x19b2c40 > wpath=/local/domain/0/backend/vif/10/1/state token=3/0: register slotnum=3 > 2016-02-11 11:33:19 CET [20318] libxl: debug: > libxl.c:4614:libxl_device_nic_remove: ao 0x19b3870: inprogress: > poller=0x19b0200, flags=i > 2016-02-11 11:33:19 CET [20318] libxl: debug: > libxl_event.c:573:watchfd_callback: watch w=0x19b2c40 > wpath=/local/domain/0/backend/vif/10/1/state token=3/0: event > epath=/local/domain/0/backend/vif/10/1/state > 2016-02-11 11:33:19 CET [20318] libxl: debug: > libxl_event.c:878:devstate_callback: backend > /local/domain/0/backend/vif/10/1/state wanted state 6 still waiting state 5 > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_aoutils.c:88:xswait_timeout_callback: backend > /local/domain/0/backend/vif/10/1/state (hoping for state change to 6): xswait > timeout (path=/local/domain/0/backend/vif/10/1/state) > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:673:libxl__ev_xswatch_deregister: watch w=0x19b2c40 > wpath=/local/domain/0/backend/vif/10/1/state token=3/0: deregister slotnum=3 > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:862:devstate_callback: backend > /local/domain/0/backend/vif/10/1/state wanted state 6 timed out > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:686:libxl__ev_xswatch_deregister: watch w=0x19b2c40: deregister > unregistered > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_device.c:939:device_backend_callback: calling device_backend_cleanup > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:686:libxl__ev_xswatch_deregister: watch w=0x19b2c40: deregister > unregistered > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_device.c:945:device_backend_callback: Timeout reached, initiating > forced remove > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:636:libxl__ev_xswatch_register: watch w=0x19b2c40 > wpath=/local/domain/0/backend/vif/10/1/state token=3/1: register slotnum=3 > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:573:watchfd_callback: watch w=0x19b2c40 > wpath=/local/domain/0/backend/vif/10/1/state token=3/1: event > epath=/local/domain/0/backend/vif/10/1/state > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:874:devstate_callback: backend > /local/domain/0/backend/vif/10/1/state wanted state 6 ok > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:673:libxl__ev_xswatch_deregister: watch w=0x19b2c40 > wpath=/local/domain/0/backend/vif/10/1/state token=3/1: deregister slotnum=3 > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_device.c:939:device_backend_callback: calling device_backend_cleanup > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:686:libxl__ev_xswatch_deregister: watch w=0x19b2c40: deregister > unregistered > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_device.c:1036:device_hotplug: calling hotplug script: > /opt/xen/staging-wip/etc/xen/scripts/vif-bridge offline > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_aoutils.c:593:libxl__async_exec_start: forking to execute: > /opt/xen/staging-wip/etc/xen/scripts/vif-bridge offline > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:542:watchfd_callback: watch > epath=/local/domain/0/backend/vif/10/1/state token=3/1: empty slot > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:686:libxl__ev_xswatch_deregister: watch w=0x19b2d40: deregister > unregistered > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_device.c:1023:device_hotplug: No hotplug script to execute > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:686:libxl__ev_xswatch_deregister: watch w=0x19b2d40: deregister > unregistered > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:1869:libxl__ao_complete: ao 0x19b3870: complete, rc=0 > 2016-02-11 11:33:29 CET [20318] libxl: debug: > libxl_event.c:1838:libxl__ao__destroy: ao
Re: [Xen-devel] [PATCH v3 3/5] build: remove .config from /boot when uninstalling.
On 2/11/16 9:08 PM, Konrad Rzeszutek Wilk wrote: > c/s 361b4f9f0f0d4adc19df428e224a7b8fa62cd392 > "build: save generated xen .config" forgot to remove > the config file when uninstalling. > > CC: ian.campb...@citrix.com > CC: jbeul...@suse.com > CC: car...@cardoe.com > CC: ian.jack...@eu.citrix.com > Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Doug Goldstein -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] xen: credit1: avoid boosting vCPUs being "just" migrated
[Yes, replying to myself]
On Fri, 2016-02-12 at 11:50 +0100, Dario Faggioli wrote:
> On Fri, 2016-02-12 at 02:50 -0700, Jan Beulich wrote:
> > > > > On 12.02.16 at 10:37, wrote:
> > > @@ -787,6 +788,16 @@ _csched_cpu_pick(const struct scheduler
> > > *ops,
> > > struct vcpu *vc, bool_t commit)
> > > static int
> > > csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc)
> > > {
> > > +struct csched_vcpu *svc = CSCHED_VCPU(vc);
> > > +
> > > +/*
> > > + * We have been called by vcpu_migrate() (in schedule.c), as
> > > part
> > > + * of the process of seeing if vc can be migrated to another
> > > pcpu.
> > > + * We make a note about this in svc->flags so that later, in
> > > + * csched_vcpu_wake() (still called from vcpu_migrate()) we
> > > won't
> > > + * get boosted, which we don't deserve as we are "only"
> > > migrating.
> > > + */
> > > +set_bit(CSCHED_FLAG_VCPU_MIGRATING, &svc->flags);
> > > return _csched_cpu_pick(ops, vc, 1);
> > > }
> >
> > I think you either want __set_bit() here or ...
> >
> Yes, this is completely serialized by the vcpu's scheduler lock, so I
> indeed want __set_bit(), sorry for the overlook.
>
Which is indeed the case, in the case of this svc->flags, but not for
other cases when svc->flags is used, for manipulating the other two
existing flags (see, for instance
be6507509454adf3bb5a50b9406c88504e996d5a "credit1: Use atomic bit
operations for the flags structure").
So what I want is really the opposite of what I said above: set_bit()
is ok, and I need the atomic test_and_clear().
-ENEEDMORECOFFEEATMORNING :-/
Thanks again and Regards,
Dario
--
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
On Fri, Feb 12, 2016 at 11:26:10AM +, Stefano Stabellini wrote: > On Thu, 11 Feb 2016, Konrad Rzeszutek Wilk wrote: > > Otherwise any code that tries to use Elf_* macros instead of > > Elf32_ or Elf_64 fails to compile. > > > > CC: ian.campb...@citrix.com > > CC: wei.l...@citrix.com > > CC: stefano.stabell...@citrix.com > > Signed-off-by: Konrad Rzeszutek Wilk > > --- > > xen/include/asm-arm/config.h | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h > > index bd832df..4ea66bf 100644 > > --- a/xen/include/asm-arm/config.h > > +++ b/xen/include/asm-arm/config.h > > @@ -15,8 +15,10 @@ > > > > #if defined(CONFIG_ARM_64) > > # define LONG_BYTEORDER 3 > > +# define ELFSIZE 64 > > #else > > # define LONG_BYTEORDER 2 > > +# define ELFSIZE 32 > > #endif > > I wonder if we should use ELF64 on ARM32 too, for simplicity (x86 only > uses ELF64) and because ARM32 is LPAE. Done. And this resolves also the question Jan raised in the design document about ARM32 and the ELF payload declaring the ELF only in 64-bit syntax. Thanks! Updated the patch to be: P.S. It compiles without trouble. >From 7756ddc1e2aa0be487df05ce76577c6fa15a75ce Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Fri, 5 Feb 2016 10:44:45 -0500 Subject: [PATCH] arm/config: Declare ELFSIZE_64. Otherwise any code that tries to use Elf_* macros would require us to use Elf64_* types instead of the more friendly Elf_ one. This is OK to do since 32-bit ARM uses LPAE mode. CC: ian.campb...@citrix.com CC: wei.l...@citrix.com CC: stefano.stabell...@citrix.com Signed-off-by: Konrad Rzeszutek Wilk --- xen/include/asm-arm/config.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h index bd832df..d5321b4 100644 --- a/xen/include/asm-arm/config.h +++ b/xen/include/asm-arm/config.h @@ -15,8 +15,10 @@ #if defined(CONFIG_ARM_64) # define LONG_BYTEORDER 3 +# define ELFSIZE 64 #else # define LONG_BYTEORDER 2 +# define ELFSIZE 64 #endif #define BYTES_PER_LONG (1 << LONG_BYTEORDER) -- 2.4.3 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 4/6] xen: add capability to load initrd outside of initial mapping
On 12/02/16 13:24, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 11.02.2016 15:13, Juergen Gross wrote:
>> On 11/02/16 13:33, Daniel Kiper wrote:
>>> On Thu, Feb 11, 2016 at 08:53:24AM +0100, Juergen Gross wrote:
Modern pvops linux kernels support an initrd not covered by
the initial mapping. This capability is flagged by an
elf-note.
In case the elf-note is set by the kernel don't place the
initrd into the initial mapping. This will allow to load
larger initrds and/or support domains with larger memory, as
the initial mapping is limited to 2GB and it is containing
the p2m list.
Signed-off-by: Juergen Gross ---
grub-core/loader/i386/xen.c| 56
> ++
grub-core/loader/i386/xen_fileXX.c | 3 ++
include/grub/xen_file.h| 1 + 3 files changed, 49
insertions(+), 11 deletions(-)
diff --git a/grub-core/loader/i386/xen.c
b/grub-core/loader/i386/xen.c index 65cec27..0f41048 100644
--- a/grub-core/loader/i386/xen.c +++
b/grub-core/loader/i386/xen.c @@ -307,15 +307,14 @@
grub_xen_pt_alloc (void) }
static grub_err_t -grub_xen_boot (void) +grub_xen_alloc_end
(void) { grub_err_t err; - grub_uint64_t nr_pages; - struct
gnttab_set_version gnttab_setver; - grub_size_t i; + static
int called = 0;
- if (grub_xen_n_allocated_shared_pages) -return
grub_error (GRUB_ERR_BUG, "active grants"); + if (called) +
return GRUB_ERR_NONE;
>>>
>>> Why?
>>
>> Did you look at the function? grub_xen_alloc_end() (some parts of
>> the original grub_xen_boot()) is new and may be called multiple
>> times. It was much easier to just call it and let it do what must
>> be done only at the first time called.
>>
> What if a boot fails and then fallback kernel is loaded?
As already stated before: I'll add a patch handling freeing of memory
in a reset function which will reset the "called" flag as well.
+ if (grub_xen_n_allocated_shared_pages) +return
grub_error (GRUB_ERR_BUG, "active grants");
>>>
>>> Why?
>>
>> That's how it has been before. git just decided to generate the
>> diff that way.
>>
> This is also needed to avoid passing control when some drivers are
> still active
+ case 16:
>>>
>>> Could you define this a constant and use it here?
>>
>> This would be the only case with a constant. All others are
>> numeric as well.
>>
> I'm ok with not insisisting on using constants given current state
> but in general constants are preferable (yes, xen code isn't always
> clean)
Okay, thanks for the confirmation. I'll add a patch (or better: some
patches) which will clean up the xen code by introducing (lots of)
constants.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [distros-debian-jessie test] 38740: tolerable all pass
flight 38740 distros-debian-jessie real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/38740/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-armhf-jessie-netboot-pygrub 12 saverestore-support-check fail never pass test-armhf-armhf-armhf-jessie-netboot-pygrub 11 migrate-support-check fail never pass baseline version: flight 38726 jobs: build-amd64 pass build-armhf pass build-i386 pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-amd64-jessie-netboot-pvgrub pass test-amd64-i386-i386-jessie-netboot-pvgrub pass test-amd64-i386-amd64-jessie-netboot-pygrub pass test-armhf-armhf-armhf-jessie-netboot-pygrub pass test-amd64-amd64-i386-jessie-netboot-pygrub pass sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xs.citrite.net/~osstest/testlogs/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] xl dev-detach hangs with missing frontends
On Fri, Feb 12, Wei Liu wrote: > CC'ing other tools maintainer. > > On Thu, Feb 11, 2016 at 11:37:49AM +0100, Olaf Hering wrote: > > How should libxl__initiate_device_generic_remove deal with devices which > > I think you meant libxl__initiate_device_remove. There is no function > called libxl__initiate_device_generic _remove. Not yet. > > have no frontend driver? Right now it moves "state" from either > > XenbusStateInitialising or XenbusStateInitWait to XenbusStateClosing. > > Then it expects the backend to move "state" to XenbusStateClosed. This > > will never happen, at least for netback and scsiback. The result is a 10 > > second delay. > > > > I don't think there is a way to tell whether there is no frontend driver > or the frontend driver is just too slow. To handle this the code should check the current value of "state". If its XenbusStateInitialising or XenbusStateInitWait nothing should be done. Olaf ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
>>> On 12.02.16 at 13:46, wrote: > On Fri, 12 Feb 2016, Jan Beulich wrote: >> and then this is the "include everything everywhere" attitude >> which tends to needlessly slow down builds (avoiding the need >> to include everything everywhere is actually one of the >> purposes of such forward declarations, which allows going as >> far as having the full definition in just a single source file). > > Fair enough, but keyhandler.h directly uses struct cpu_user_regs as > parameter in two functions, so I think it would be right to include an > header file with the definition. No, full definitions need only be visible when structure instances are used, not when just pointers to them get declared. The sole reason why the forward declaration is needed is because otherwise declaration and definition of the function wouldn't match, as without the forward declaration the scope of the structure is that of the function (instead of the global scope). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [COVERITY ACCESS] Request for access to Coverity
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I agree to the conditions in the XenProject Coverity contribution guidelines [0]. I'm a developer working for Citrix Systems UK, Ltd. I've been active in the Xen community since 2008; I currently maintain Xen on ARM, Xen support for the ARM and ARM64 architectures in Linux and Xen support in QEMU. I would like access to keep an eye on outstanding defects and help fix them whenever I can. - - Stefano Stabellini [0] http://xenproject.org/help/contribution-guidelines.html -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) iQIcBAEBAgAGBQJWvfD/AAoJEIlPj0hw4a6QfnIP/2TaFvzrlvA2s9RH0wWquYu1 UgObtA/gzbQTc2GmpFHKC/n9i//DPkqx7TeQv2l95NLec5djBi9ILeuwKRotye0l tn56jwOVna03jD/N/EWs6dk/TNkt1ZjqB1MUdYTIeQ+a9petqT1h5FahRB7Cb3a4 Tr5YDVXuY3P1x/D7bq23KrsMzTHM+MJLhzunWqqfdiCKe9fwtzOGE6T7N7jO6D2D 6RKlIuR+v6iOHyVJK7ZqD6QdA1xpZUdAYvhBQ/8nC948Khd515VIZIr5O2cwYNxb dhr7aDSSAQO50/CPGIOWwfA4eXQ46PF+1dlWbybNmkixsc/4050SOvazsONDvy2v Leu3E7LheJmD9oEAx7RwVInlo4A5wYtLO27oTUzjFmwyuKIOsbA0oiL2M07KfbIh 76r8mCy89ZQEaRuER/gm29A+ltJhER9xzDiL+Q96ywHew1VxvqfohnhzdsNyoGTq jdB4/X5aUqYi7RbkrkM4330GFaEXL/VtofGrq4R6W3XZH5ulbjSTaMKy9WoSwo00 berbziUj2hY0vUVnNV68tXjE1fJZMQvCyTFBRqJ6O6f4t5VU5VF03xFWJkbEIDHJ 6r4oouNFTXKSPUxKfmHmcDEdu6HIVhCCYZCDpxL5FGOctSjjnQLU2OyEgZwreZgD bjIno21PR2LYOghHdiOp =kULv -END PGP SIGNATURE- ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/5] hypervisor/arm/keyhandler: Declare struct cpu_user_regs;
>>> On 12.02.16 at 15:06, wrote: > On Fri, Feb 12, 2016 at 01:51:28AM -0700, Jan Beulich wrote: >> >>> On 12.02.16 at 04:08, wrote: >> > in the keyhandler.h file. Otherwise on ARM builds if we >> > just use the keyhandler file - the compile will fail. >> > >> > CC: ian.campb...@citrix.com >> > CC: wei.l...@citrix.com >> > CC: stefano.stabell...@citrix.com >> > Signed-off-by: Konrad Rzeszutek Wilk >> >> See commit d1d181328d. > > Weird. When I did git rebase origin/staging it didn't > notice that commit. And it just applied it on top - so I had > two 'struct cpu_regs;' in the header file. That's likely because I moved the declaration down a few lines while applying. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [COVERITY ACCESS] Request for access to Coverity
On 12/02/16 14:53, Stefano Stabellini wrote: > I agree to the conditions in the XenProject Coverity contribution > guidelines [0]. > > I'm a developer working for Citrix Systems UK, Ltd. I've been active > in the Xen community since 2008; I currently maintain Xen on ARM, Xen > support for the ARM and ARM64 architectures in Linux and Xen support in > QEMU. > > I would like access to keep an eye on outstanding defects and help fix > them whenever I can. > > - Stefano Stabellini +1 ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] vm_event: consolidate hvm_event_fill_regs and p2m_vm_event_fill_regs
>>> On 12.02.16 at 13:50, wrote: > On Feb 12, 2016 03:41, "Jan Beulich" wrote: >> In which case ASSERT(is_hvm_vcpu(curr)) would be the common >> way to document this (at once avoiding the open coding of >> is_hvm_vcpu()). >> > > I don't think we need an assert here. The function is fine for pv guests as > well up to that point. Filling in the rest of the registers for pv guests > can be done when pv events are implemented. Maybe a comment saying so is > warranted. I disagree: Either you mean to support PV in the function, in which case all fields should be filled, or you don't, in which case the ASSERT() would at once document that PV is intended to not be supported right now. With the conditional as in your patch any future reader may either think "bug" or get confused as to the actual intentions here. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [qemu-upstream-4.6-testing test] 81798: regressions - FAIL
flight 81798 qemu-upstream-4.6-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/81798/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 77722 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 77722 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 77722 test-amd64-amd64-qemuu-nested-amd 9 debian-hvm-install fail REGR. vs. 77722 test-amd64-i386-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 77722 test-amd64-amd64-qemuu-nested-intel 9 debian-hvm-install fail REGR. vs. 77722 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 77722 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 77722 test-amd64-i386-qemuu-rhel6hvm-intel 9 redhat-installfail REGR. vs. 77722 test-amd64-i386-qemuu-rhel6hvm-amd 9 redhat-install fail REGR. vs. 77722 test-armhf-armhf-libvirt 6 xen-boot fail REGR. vs. 77722 test-amd64-amd64-xl-qemuu-winxpsp3 9 windows-install fail REGR. vs. 77722 test-amd64-i386-xl-qemuu-winxpsp3 9 windows-install fail REGR. vs. 77722 test-amd64-amd64-xl-qemuu-win7-amd64 9 windows-install fail REGR. vs. 77722 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 9 windows-install fail REGR. vs. 77722 test-amd64-i386-xl-qemuu-win7-amd64 9 windows-installfail REGR. vs. 77722 Regressions which are regarded as allowable (not blocking): test-amd64-i386-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail like 77562 test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail like 77562 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 guest-saverestorefail never pass test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 11 guest-start fail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 saverestore-support-checkfail never pass version targeted for testing: qemuu02879be2307dbda16056152d61872ccdadb0feba baseline version: qemuu9e304f572ac98265f5e7433b6490077962acda97 Last test of basis77722 2016-01-10 11:19:40 Z 33 days Failing since 80733 2016-02-05 15:18:36 Z6 days2 attempts Testing same since81798 2016-02-10 13:34:44 Z2 days1 attempts People who touched revisions under test: Gerd Hoffmann Jason Wang John Snow Laszlo Ersek Michael S. Tsirkin P J P Paolo Bonzini Peter Maydell Prasad J Pandit Roger Pau Monne Roger Pau Monné Stefano Stabellini jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64
Re: [Xen-devel] [PATCH v2 2/6] relocator: Do not use memory region if its starta is smaller than size
Applied, thanks
On 20.07.2015 16:35, Daniel Kiper wrote:
> malloc_in_range() should not use memory region if its starta is smaller
> than size. Otherwise target wraps around and points to region which is
> usually not a RAM, e.g.:
>
> loader/multiboot.c:93: segment 0: paddr=0x80, memsz=0x3f80,
> vaddr=0x80
> lib/relocator.c:1241: min_addr = 0x0, max_addr = 0x, target =
> 0x80
> lib/relocator.c:434: trying to allocate in 0x80-0x
> aligned 0x1 size 0x3f80
> lib/relocator.c:434: trying to allocate in 0x0-0x80 aligned 0x1 size
> 0x3f80
> lib/relocator.c:434: trying to allocate in 0x0-0x aligned 0x1
> size 0x3f80
> lib/relocator.c:1188: allocated: 0xc07f+0x3f80
> lib/relocator.c:1277: allocated 0xc07f/0x80
>
> Signed-off-by: Daniel Kiper
> ---
> grub-core/lib/relocator.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/lib/relocator.c b/grub-core/lib/relocator.c
> index f759c7f..4eee0c5 100644
> --- a/grub-core/lib/relocator.c
> +++ b/grub-core/lib/relocator.c
> @@ -748,7 +748,7 @@ malloc_in_range (struct grub_relocator *rel,
> /* Found an usable address. */
> goto found;
> }
> - if (isinsidebefore && !isinsideafter && !from_low_priv)
> + if (isinsidebefore && !isinsideafter && !from_low_priv && starta >=
> size)
> {
> target = starta - size;
> if (target > end - size)
>
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] xen/x86: Fix errors arising from c/s dab76ff
Coverity correctly identifies that the changes in mtrr_attrib_to_str()
introduce dead code. strings[] is a 2d array, rather than an array of
strings, which means that strings[x] will never be a NULL pointer.
Adjust the check to compenstate, by looking for a NUL in strings[x][0]
instead.
Curiously, Coverity did not notice the same error with memory_type_to_str().
There was also a further error; the strings were not NULL terminated, which
made the return type of memory_type_to_str() erronious.
Bump the 2D array to 3 characters, so the strings retain their NUL characters,
and introduce an ASSERT() as requested on one thread of the original patch.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: George Dunlap
---
xen/arch/x86/cpu/mtrr/generic.c | 2 +-
xen/arch/x86/mm/p2m-ept.c | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/cpu/mtrr/generic.c b/xen/arch/x86/cpu/mtrr/generic.c
index 8839f8d..234d2ba 100644
--- a/xen/arch/x86/cpu/mtrr/generic.c
+++ b/xen/arch/x86/cpu/mtrr/generic.c
@@ -98,7 +98,7 @@ static const char *__init mtrr_attrib_to_str(mtrr_type x)
[MTRR_TYPE_WRBACK] = "write-back",
};
- return x < MTRR_NUM_TYPES ? (strings[x] ?: "?") : "?";
+ return (x < ARRAY_SIZE(strings) && strings[x][0]) ? strings[x] : "?";
}
static unsigned int __initdata last_fixed_start;
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 316e3f3..3cb6868 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -1204,7 +1204,7 @@ void ept_p2m_uninit(struct p2m_domain *p2m)
static const char *memory_type_to_str(unsigned int x)
{
-static const char memory_types[8][2] = {
+static const char memory_types[8][3] = {
[MTRR_TYPE_UNCACHABLE] = "UC",
[MTRR_TYPE_WRCOMB] = "WC",
[MTRR_TYPE_WRTHROUGH] = "WT",
@@ -1213,7 +1213,8 @@ static const char *memory_type_to_str(unsigned int x)
[MTRR_NUM_TYPES] = "??"
};
-return x < ARRAY_SIZE(memory_types) ? (memory_types[x] ?: "?") : "?";
+ASSERT(x < ARRAY_SIZE(memory_types));
+return memory_types[x][0] ? memory_types[x] : "?";
}
static void ept_dump_p2m_table(unsigned char key)
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] hvm/vmx: save dr7 during vmx_vmcs_save
>>> On 12.02.16 at 13:57, wrote: > On Feb 12, 2016 02:12, "Jan Beulich" wrote: >> >> >>> On 12.02.16 at 01:22, wrote: >> > Sending the dr7 register during vm_events is useful for various > applications, >> > but the current way the register value is gathered is incorrent. In this >> > patch >> > we extend vmx_vmcs_save so that we get the correct value. >> > >> > Suggested-by: Andrew Cooper >> >> Iirc Andrew suggested ... >> >> > --- a/xen/arch/x86/hvm/vmx/vmx.c >> > +++ b/xen/arch/x86/hvm/vmx/vmx.c >> > @@ -490,6 +490,7 @@ static void vmx_vmcs_save(struct vcpu *v, struct >> > hvm_hw_cpu *c) >> > __vmread(GUEST_SYSENTER_CS, &c->sysenter_cs); >> > __vmread(GUEST_SYSENTER_ESP, &c->sysenter_esp); >> > __vmread(GUEST_SYSENTER_EIP, &c->sysenter_eip); >> > +__vmread(GUEST_DR7, &c->dr7); >> >> ... just when v == current. >> > > Would that check really be necessary? It would complicate the code not just > here but the caller would need to be aware too that in that case dr7 can be > aquired from someplace else. I don't see the harm in just saving dr7 here > in both cases. Maybe the solution then is for the suggested if() to have an "else"? While, as someone said elsewhere, a few more cycles may not be noticable, why make things slower than they need to be. Plus - what guarantees that the VMCS field isn't stale while the guest isn't running (perhaps it got updated but not sync-ed back yet in anticipation for this to happen during vCPU resume)? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
>>> On 12.02.16 at 15:17, wrote: > --- a/xen/include/asm-arm/config.h > +++ b/xen/include/asm-arm/config.h > @@ -15,8 +15,10 @@ > > #if defined(CONFIG_ARM_64) > # define LONG_BYTEORDER 3 > +# define ELFSIZE 64 > #else > # define LONG_BYTEORDER 2 > +# define ELFSIZE 64 > #endif Leaving the question - why twice instead of outside the #ifdef? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/x86: Fix errors arising from c/s dab76ff
>>> On 12.02.16 at 15:59, wrote: > Coverity correctly identifies that the changes in mtrr_attrib_to_str() > introduce dead code. strings[] is a 2d array, rather than an array of > strings, which means that strings[x] will never be a NULL pointer. > > Adjust the check to compenstate, by looking for a NUL in strings[x][0] > instead. > > Curiously, Coverity did not notice the same error with memory_type_to_str(). I agree up to here. > There was also a further error; the strings were not NULL terminated, which > made the return type of memory_type_to_str() erronious. What's erroneous here? I don't think there's any requirement for a function returning char * to always return NUL-terminated strings. > Bump the 2D array to 3 characters, so the strings retain their NUL > characters, I.e. I don't agree with this part of the change, even if the addition of these few bytes doesn't make a whole lot of a difference. They end up being "dead data" now, and if Coverity is smart it should even be able to notice. > and introduce an ASSERT() as requested on one thread of the original patch. Whereas this part is again appreciated. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/x86: Fix errors arising from c/s dab76ff
On 12/02/16 15:13, Jan Beulich wrote:
On 12.02.16 at 15:59, wrote:
>> Coverity correctly identifies that the changes in mtrr_attrib_to_str()
>> introduce dead code. strings[] is a 2d array, rather than an array of
>> strings, which means that strings[x] will never be a NULL pointer.
>>
>> Adjust the check to compenstate, by looking for a NUL in strings[x][0]
>> instead.
>>
>> Curiously, Coverity did not notice the same error with memory_type_to_str().
> I agree up to here.
>
>> There was also a further error; the strings were not NULL terminated, which
>> made the return type of memory_type_to_str() erronious.
> What's erroneous here? I don't think there's any requirement
> for a function returning char * to always return NUL-terminated
> strings.
The name of the function very clearly indicates that it is returning a
string.
>
>> Bump the 2D array to 3 characters, so the strings retain their NUL
>> characters,
> I.e. I don't agree with this part of the change, even if the addition
> of these few bytes doesn't make a whole lot of a difference. They
> end up being "dead data" now, and if Coverity is smart it should
> even be able to notice.
It will produce something wrong if someone introduces a new path doing
something like printk("%s", memory_type_to_str()). 8 extra bytes is a
very small price to pay to make this work properly.
The alternative, const char (*memory_type_to_str(unsigned int x))[2] is
unrecognisable to most C programmers, and can't be used with printk().
~Andrew
>
>> and introduce an ASSERT() as requested on one thread of the original patch.
> Whereas this part is again appreciated.
>
> 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
[Xen-devel] Clang tools build
Hello, Now that Clang 3.5 can build the hypervisor, I was just preparing a patch to README, and encountered this: In file included from xc_altp2m.c:23: In file included from ./xc_private.h:35: In file included from ./include/xenctrl.h:53: /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:203:47: error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes] __align8__ uint64_t evtchn_pending[sizeof(__align8__ uint64_t) * 8]; ^~ /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36: note: expanded from macro '__align8__' # define __align8__ __attribute__((aligned (8))) ^~~ /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:204:44: error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes] __align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8]; ^~ /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36: note: expanded from macro '__align8__' # define __align8__ __attribute__((aligned (8))) ^~~ 2 errors generated. In this case, Clang is complaining that the alignment attribute is wrong for uint64_t. This is correct for 64bit compilations, but wrong for 32bit. On the other hand, I am not sure whether it is sensible to do a blanket disable of -Wignored-attributes. Thoughts? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
On Fri, 12 Feb 2016, Jan Beulich wrote: > >>> On 12.02.16 at 15:17, wrote: > > --- a/xen/include/asm-arm/config.h > > +++ b/xen/include/asm-arm/config.h > > @@ -15,8 +15,10 @@ > > > > #if defined(CONFIG_ARM_64) > > # define LONG_BYTEORDER 3 > > +# define ELFSIZE 64 > > #else > > # define LONG_BYTEORDER 2 > > +# define ELFSIZE 64 > > #endif > > Leaving the question - why twice instead of outside the #ifdef? Right, please move it out of the #ifdef. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [COVERITY ACCESS] Request for access to Coverity
On Fri, Feb 12, 2016 at 02:53:21PM +, Stefano Stabellini wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > I agree to the conditions in the XenProject Coverity contribution > guidelines [0]. +1 > > I'm a developer working for Citrix Systems UK, Ltd. I've been active > in the Xen community since 2008; I currently maintain Xen on ARM, Xen > support for the ARM and ARM64 architectures in Linux and Xen support in > QEMU. > > I would like access to keep an eye on outstanding defects and help fix > them whenever I can. > > - - Stefano Stabellini > > > [0] http://xenproject.org/help/contribution-guidelines.html > -BEGIN PGP SIGNATURE- > Version: GnuPG v1.4.11 (GNU/Linux) > > iQIcBAEBAgAGBQJWvfD/AAoJEIlPj0hw4a6QfnIP/2TaFvzrlvA2s9RH0wWquYu1 > UgObtA/gzbQTc2GmpFHKC/n9i//DPkqx7TeQv2l95NLec5djBi9ILeuwKRotye0l > tn56jwOVna03jD/N/EWs6dk/TNkt1ZjqB1MUdYTIeQ+a9petqT1h5FahRB7Cb3a4 > Tr5YDVXuY3P1x/D7bq23KrsMzTHM+MJLhzunWqqfdiCKe9fwtzOGE6T7N7jO6D2D > 6RKlIuR+v6iOHyVJK7ZqD6QdA1xpZUdAYvhBQ/8nC948Khd515VIZIr5O2cwYNxb > dhr7aDSSAQO50/CPGIOWwfA4eXQ46PF+1dlWbybNmkixsc/4050SOvazsONDvy2v > Leu3E7LheJmD9oEAx7RwVInlo4A5wYtLO27oTUzjFmwyuKIOsbA0oiL2M07KfbIh > 76r8mCy89ZQEaRuER/gm29A+ltJhER9xzDiL+Q96ywHew1VxvqfohnhzdsNyoGTq > jdB4/X5aUqYi7RbkrkM4330GFaEXL/VtofGrq4R6W3XZH5ulbjSTaMKy9WoSwo00 > berbziUj2hY0vUVnNV68tXjE1fJZMQvCyTFBRqJ6O6f4t5VU5VF03xFWJkbEIDHJ > 6r4oouNFTXKSPUxKfmHmcDEdu6HIVhCCYZCDpxL5FGOctSjjnQLU2OyEgZwreZgD > bjIno21PR2LYOghHdiOp > =kULv > -END PGP SIGNATURE- > > ___ > 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 v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
On Fri, Feb 12, 2016 at 03:26:14PM +, Stefano Stabellini wrote: > On Fri, 12 Feb 2016, Jan Beulich wrote: > > >>> On 12.02.16 at 15:17, wrote: > > > --- a/xen/include/asm-arm/config.h > > > +++ b/xen/include/asm-arm/config.h > > > @@ -15,8 +15,10 @@ > > > > > > #if defined(CONFIG_ARM_64) > > > # define LONG_BYTEORDER 3 > > > +# define ELFSIZE 64 > > > #else > > > # define LONG_BYTEORDER 2 > > > +# define ELFSIZE 64 > > > #endif > > > > Leaving the question - why twice instead of outside the #ifdef? > > Right, please move it out of the #ifdef. Done! >From 32a062c119091f2f3f6a4c540a8098e97c273dd2 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Fri, 5 Feb 2016 10:44:45 -0500 Subject: [PATCH] arm/config: Declare ELFSIZE_64. Otherwise any code that tries to use Elf_* macros would require us to use Elf64_* types instead of the more friendly Elf_ one. This is OK to do since 32-bit ARM uses LPAE mode. CC: ian.campb...@citrix.com CC: wei.l...@citrix.com CC: stefano.stabell...@citrix.com Signed-off-by: Konrad Rzeszutek Wilk --- xen/include/asm-arm/config.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h index bd832df..a1b968d 100644 --- a/xen/include/asm-arm/config.h +++ b/xen/include/asm-arm/config.h @@ -25,6 +25,9 @@ /* xen_ulong_t is always 64 bits */ #define BITS_PER_XEN_ULONG 64 +/* And ELF files are also 64-bit. */ +#define ELFSIZE 64 + #define CONFIG_PAGING_ASSISTANCE 1 #define CONFIG_PAGING_LEVELS 3 -- 2.1.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/5] arm/config: Declare ELFSIZE_[32|64] respectively.
On Fri, 12 Feb 2016, Konrad Rzeszutek Wilk wrote: > On Fri, Feb 12, 2016 at 03:26:14PM +, Stefano Stabellini wrote: > > On Fri, 12 Feb 2016, Jan Beulich wrote: > > > >>> On 12.02.16 at 15:17, wrote: > > > > --- a/xen/include/asm-arm/config.h > > > > +++ b/xen/include/asm-arm/config.h > > > > @@ -15,8 +15,10 @@ > > > > > > > > #if defined(CONFIG_ARM_64) > > > > # define LONG_BYTEORDER 3 > > > > +# define ELFSIZE 64 > > > > #else > > > > # define LONG_BYTEORDER 2 > > > > +# define ELFSIZE 64 > > > > #endif > > > > > > Leaving the question - why twice instead of outside the #ifdef? > > > > Right, please move it out of the #ifdef. > > Done! > > >From 32a062c119091f2f3f6a4c540a8098e97c273dd2 Mon Sep 17 00:00:00 2001 > From: Konrad Rzeszutek Wilk > Date: Fri, 5 Feb 2016 10:44:45 -0500 > Subject: [PATCH] arm/config: Declare ELFSIZE_64. > > Otherwise any code that tries to use Elf_* macros would > require us to use Elf64_* types instead of the more > friendly Elf_ one. > > This is OK to do since 32-bit ARM uses LPAE mode. > > CC: ian.campb...@citrix.com > CC: wei.l...@citrix.com > CC: stefano.stabell...@citrix.com > Signed-off-by: Konrad Rzeszutek Wilk Acked-by: Stefano Stabellini > xen/include/asm-arm/config.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h > index bd832df..a1b968d 100644 > --- a/xen/include/asm-arm/config.h > +++ b/xen/include/asm-arm/config.h > @@ -25,6 +25,9 @@ > /* xen_ulong_t is always 64 bits */ > #define BITS_PER_XEN_ULONG 64 > > +/* And ELF files are also 64-bit. */ > +#define ELFSIZE 64 > + > #define CONFIG_PAGING_ASSISTANCE 1 > > #define CONFIG_PAGING_LEVELS 3 > -- > 2.1.0 > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Clang tools build
>>> On 12.02.16 at 16:25, wrote: > Hello, > > Now that Clang 3.5 can build the hypervisor, I was just preparing a > patch to README, and encountered this: > > In file included from xc_altp2m.c:23: > In file included from ./xc_private.h:35: > In file included from ./include/xenctrl.h:53: > /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:203:47: > error: 'aligned' attribute ignored when parsing type > [-Werror,-Wignored-attributes] > __align8__ uint64_t evtchn_pending[sizeof(__align8__ uint64_t) * 8]; > ^~ > /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36: > note: expanded from macro '__align8__' > # define __align8__ __attribute__((aligned (8))) >^~~ > /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:204:44: > error: 'aligned' attribute ignored when parsing type > [-Werror,-Wignored-attributes] > __align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8]; >^~ > /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36: > note: expanded from macro '__align8__' > # define __align8__ __attribute__((aligned (8))) >^~~ > 2 errors generated. > > In this case, Clang is complaining that the alignment attribute is wrong > for uint64_t. This is correct for 64bit compilations, but wrong for > 32bit. If it only complains about it when used as operand to sizeof() I think it's correctly saying so. > On the other hand, I am not sure whether it is sensible to do a > blanket disable of -Wignored-attributes. I agree it's not ideal, but I see no alternative, and I'm not sure there are many cases where a legitimately ignored attribute would be a problem (leaving as a potential issue only cases where the compiler wrongly ignores one). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Clang tools build
On 12/02/16 16:04, Jan Beulich wrote: On 12.02.16 at 16:25, wrote: >> Hello, >> >> Now that Clang 3.5 can build the hypervisor, I was just preparing a >> patch to README, and encountered this: >> >> In file included from xc_altp2m.c:23: >> In file included from ./xc_private.h:35: >> In file included from ./include/xenctrl.h:53: >> /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:203:47: >> error: 'aligned' attribute ignored when parsing type >> [-Werror,-Wignored-attributes] >> __align8__ uint64_t evtchn_pending[sizeof(__align8__ uint64_t) * 8]; >> ^~ >> /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36: >> note: expanded from macro '__align8__' >> # define __align8__ __attribute__((aligned (8))) >>^~~ >> /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:204:44: >> error: 'aligned' attribute ignored when parsing type >> [-Werror,-Wignored-attributes] >> __align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8]; >>^~ >> /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36: >> note: expanded from macro '__align8__' >> # define __align8__ __attribute__((aligned (8))) >>^~~ >> 2 errors generated. >> >> In this case, Clang is complaining that the alignment attribute is wrong >> for uint64_t. This is correct for 64bit compilations, but wrong for >> 32bit. > If it only complains about it when used as operand to sizeof() I > think it's correctly saying so. Turns out that a fix^W gross hack of diff --git a/tools/include/xen-foreign/Makefile b/tools/include/xen-foreign/Makefile index 80a446a..d9f68cd 100644 --- a/tools/include/xen-foreign/Makefile +++ b/tools/include/xen-foreign/Makefile @@ -35,6 +35,7 @@ x86_32.h: mkheader.py structs.py $(ROOT)/arch-x86/xen-x86_32.h $(ROOT)/arch-x86/ x86_64.h: mkheader.py structs.py $(ROOT)/arch-x86/xen-x86_64.h $(ROOT)/arch-x86/xen.h $(ROOT)/xen.h $(PYTHON) $< $* $@ $(filter %.h,$^) + sed 's/(__align8__ uint64_t)/(uint64_t)/g' -i $@ checker.c: mkchecker.py structs.py $(PYTHON) $< $@ $(architectures) Does indeed fix the build. There are problems with sizeof(), and later, an explicit (__align8__ uint64_t) cast. Unfortunately, mkheader is doing dumb translation, and I don't fancy teaching it how to parse C. I will see if there is something neater I can do as a solution. Also, further real issues have surfaces, so I will have to submit a series cleaning up the tools/ compile with Clang. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 05/30] xen/public: Export cpu featureset information in the public API
>>> On 05.02.16 at 14:41, wrote: > +/* Intel-defined CPU features, CPUID level 0x0001.edx, word 0 */ > +#define X86_FEATURE_FPU ( 0*32+ 0) /* Onboard FPU */ Regardless of you limiting the interface to tools only, I'm not convinced exposing constants starting with X86_* here is appropriate. > +#define X86_FEATURE_XMM ( 0*32+25) /* Streaming SIMD Extensions > */ > +#define X86_FEATURE_XMM2 ( 0*32+26) /* Streaming SIMD > Extensions-2 */ > [...] > +/* Intel-defined CPU features, CPUID level 0x0001.ecx, word 1 */ > +#define X86_FEATURE_XMM3 ( 1*32+ 0) /* Streaming SIMD > Extensions-3 */ Apart from that exposing them should be done using canonical instead of Linux-invented names, i.e. s/XMM/SSE/ for the above lines. I've had a need to create a patch to do this just earlier today. > +#define X86_FEATURE_SSSE3 ( 1*32+ 9) /* Supplemental Streaming > SIMD Extensions-3 */ Note how this one and ... > +#define X86_FEATURE_SSE4_1( 1*32+19) /* Streaming SIMD Extensions > 4.1 */ > +#define X86_FEATURE_SSE4_2( 1*32+20) /* Streaming SIMD Extensions > 4.2 */ ... these two already use names matching the SDM. Since canonicalization of the names implies changes elsewhere, I also can't really offer to do the adjustments while committing. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 0/2] xen: sched: Credit1 shouldn't boost vcpus being migrated.
Hi, take 3 of this series. Only change wrt v2 is the atomic-safeness fix in patch 2. While there, I've also added a comment about the need for such atomic-safeness when accessing Credit1's svc->flags. History is here: v2: http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg01750.html v1: http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg01620.html Thanks and Regards, Dario --- Dario Faggioli (2): xen: credit1: trace vCPU boost/unboost xen: credit1: avoid boosting vCPUs being "just" migrated xen/common/sched_credit.c| 38 ++ xen/include/xen/perfc_defn.h |1 + 2 files changed, 35 insertions(+), 4 deletions(-) -- <> (Raistlin Majere) --- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 1/2] xen: credit1: trace vCPU boost/unboost
Add tracepoints and a performance counter for
boosting and unboosting in Credit1.
Note that they (the trace points) do not cover
the case of the idle vCPU being boosted to run
a tasklet, as there already is
TRC_CSCHED_SCHED_TASKLET for that.
Signed-off-by: Dario Faggioli
---
Cc: George Dunlap
---
xen/common/sched_credit.c|8
xen/include/xen/perfc_defn.h |1 +
2 files changed, 9 insertions(+)
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 671bbee..5708701 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -126,6 +126,8 @@
#define TRC_CSCHED_STOLEN_VCPU TRC_SCHED_CLASS_EVT(CSCHED, 4)
#define TRC_CSCHED_PICKED_CPUTRC_SCHED_CLASS_EVT(CSCHED, 5)
#define TRC_CSCHED_TICKLETRC_SCHED_CLASS_EVT(CSCHED, 6)
+#define TRC_CSCHED_BOOST_START TRC_SCHED_CLASS_EVT(CSCHED, 7)
+#define TRC_CSCHED_BOOST_END TRC_SCHED_CLASS_EVT(CSCHED, 8)
/*
@@ -856,7 +858,11 @@ csched_vcpu_acct(struct csched_private *prv, unsigned int
cpu)
* amount of CPU resources and should no longer be boosted.
*/
if ( svc->pri == CSCHED_PRI_TS_BOOST )
+{
svc->pri = CSCHED_PRI_TS_UNDER;
+TRACE_2D(TRC_CSCHED_BOOST_END, svc->sdom->dom->domain_id,
+ svc->vcpu->vcpu_id);
+}
/*
* Update credits
@@ -1023,6 +1029,8 @@ csched_vcpu_wake(const struct scheduler *ops, struct vcpu
*vc)
if ( svc->pri == CSCHED_PRI_TS_UNDER &&
!test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
{
+TRACE_2D(TRC_CSCHED_BOOST_START, vc->domain->domain_id, vc->vcpu_id);
+SCHED_STAT_CRANK(vcpu_boost);
svc->pri = CSCHED_PRI_TS_BOOST;
}
diff --git a/xen/include/xen/perfc_defn.h b/xen/include/xen/perfc_defn.h
index 76ee803..21c1e0b 100644
--- a/xen/include/xen/perfc_defn.h
+++ b/xen/include/xen/perfc_defn.h
@@ -40,6 +40,7 @@ PERFCOUNTER(acct_reorder, "csched: acct_reorder")
PERFCOUNTER(acct_min_credit,"csched: acct_min_credit")
PERFCOUNTER(acct_vcpu_active, "csched: acct_vcpu_active")
PERFCOUNTER(acct_vcpu_idle, "csched: acct_vcpu_idle")
+PERFCOUNTER(vcpu_boost, "csched: vcpu_boost")
PERFCOUNTER(vcpu_park, "csched: vcpu_park")
PERFCOUNTER(vcpu_unpark,"csched: vcpu_unpark")
PERFCOUNTER(load_balance_idle, "csched: load_balance_idle")
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 2/2] xen: credit1: avoid boosting vCPUs being "just" migrated
Moving a vCPU to a different pCPU means offlining it and
then waking it up, on the new pCPU. Credit1 grants BOOST
priority to vCPUs that wakes up, with the aim of improving
I/O latency. The net effect of this all is that vCPUs get
boosted when migrating, which shouldn't happen.
For instance, this causes scheduling anomalies and,
potentially, performance problems, as reported here:
http://lists.xen.org/archives/html/xen-devel/2015-10/msg02851.html
This patch fixes this by noting down (by means of a flag)
the fact that the vCPU is about to undergo a migration.
This way we can tell, later, during a wakeup, whether the
vCPU is migrating or unblocking, and decide whether or
not to apply the boosting.
Note that it is important that atomic-safe bit operations
are used when manipulating vCPUs' flags. Take the chance
and add a comment about this.
Signed-off-by: Dario Faggioli
---
Cc: George Dunlap
Cc: Jan Beulich
---
Changes from v2:
* test_and_clear() is necessary when accessing svc->flags;
* added a comment about such need at the top, where the flags
are defined.
Changes from v1:
* rewritten, following suggestion got during review: there
are no wakeup flags any longer, and all is done in sched_credit.c
by setting a flag in csched_cpu_pick() and testing (and
cleating) it in csched_vcpu_wake().
---
xen/common/sched_credit.c | 30 ++
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 5708701..597a784 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -63,9 +63,14 @@
/*
* Flags
+ *
+ * Note that svc->flags (where these flags live) is protected by an
+ * inconsistent set of locks. Therefore atomic-safe bit operations must
+ * be used for accessing it.
*/
#define CSCHED_FLAG_VCPU_PARKED0x0 /* VCPU over capped credits */
#define CSCHED_FLAG_VCPU_YIELD 0x1 /* VCPU yielding */
+#define CSCHED_FLAG_VCPU_MIGRATING 0x2 /* VCPU may have moved to a new pcpu */
/*
@@ -787,6 +792,16 @@ _csched_cpu_pick(const struct scheduler *ops, struct vcpu
*vc, bool_t commit)
static int
csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc)
{
+struct csched_vcpu *svc = CSCHED_VCPU(vc);
+
+/*
+ * We have been called by vcpu_migrate() (in schedule.c), as part
+ * of the process of seeing if vc can be migrated to another pcpu.
+ * We make a note about this in svc->flags so that later, in
+ * csched_vcpu_wake() (still called from vcpu_migrate()) we won't
+ * get boosted, which we don't deserve as we are "only" migrating.
+ */
+set_bit(CSCHED_FLAG_VCPU_MIGRATING, &svc->flags);
return _csched_cpu_pick(ops, vc, 1);
}
@@ -1022,11 +1037,18 @@ csched_vcpu_wake(const struct scheduler *ops, struct
vcpu *vc)
* more CPU resource intensive VCPUs without impacting overall
* system fairness.
*
- * The one exception is for VCPUs of capped domains unpausing
- * after earning credits they had overspent. We don't boost
- * those.
+ * There are two cases, when we don't want to boost:
+ * - VCPUs that are waking up after a migration, rather than
+ *after having block;
+ * - VCPUs of capped domains unpausing after earning credits
+ *they had overspent.
+ *
+ * Note that checking whether we are "only" migrating must be
+ * done up front, as we do not want the clearing of the bit we
+ * set in csched_cpu_pick() to be short-circuited away.
*/
-if ( svc->pri == CSCHED_PRI_TS_UNDER &&
+if ( !test_and_clear_bit(CSCHED_FLAG_VCPU_MIGRATING, &svc->flags) &&
+ svc->pri == CSCHED_PRI_TS_UNDER &&
!test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
{
TRACE_2D(TRC_CSCHED_BOOST_START, vc->domain->domain_id, vc->vcpu_id);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 06/30] xen/x86: Script to automatically process featureset information
>>> On 05.02.16 at 14:41, wrote: > This script consumes include/public/arch-x86/cpufeatureset.h and generates a > single include/asm-x86/cpuid-autogen.h containing all the processed > information. > > Signed-off-by: Andrew Cooper Acked-by: Jan Beulich albeit ... > --- /dev/null > +++ b/xen/tools/gen-cpuid.py > @@ -0,0 +1,191 @@ ... I can't really comment on this. Am I at least right in understanding that all it does at this point is produce FEATURESET_NR_ENTRIES? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 07/30] xen/x86: Collect more cpuid feature leaves
>>> On 05.02.16 at 14:42, wrote: > New words are: > * 0x8007.edx - Contains Invarient TSC > * 0x8008.ebx - Newly used for AMD Zen processors > > In addition, replace some open-coded ITSC and EFRO manipulation. > > Signed-off-by: Andrew Cooper Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [ovmf test] 81861: regressions - FAIL
flight 81861 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/81861/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail REGR. vs. 65543 version targeted for testing: ovmf 1095b4323dab2e64e71bf7896ea88a0eb5d64706 baseline version: ovmf 5ac96e3a28dd26eabee421919f67fa7c443a47f1 Last test of basis65543 2015-12-08 08:45:15 Z 66 days Failing since 65593 2015-12-08 23:44:51 Z 65 days 69 attempts Testing same since81861 2016-02-10 22:29:53 Z1 days1 attempts People who touched revisions under test: "Samer El-Haj-Mahmoud" "Yao, Jiewen" Andrew Fish Ard Biesheuvel Arthur Crippa Burigo Cecil Sheng Chao Zhang Charles Duffy Cinnamon Shia Dandan Bi Daocheng Bu Daryl McDaniel Eric Dong Eric Dong Eugene Cohen Evan Lloyd Feng Tian Fu Siyuan Hao Wu Hess Chen Heyi Guo Jaben Carsey Jeff Fan Jiaxin Wu Jim Dailey Jordan Justen Karyne Mayer Larry Hauch Laszlo Ersek Leekha Shaveta Leif Lindholm Liming Gao Mark Rutland Michael Kinney Michael Thomas Paolo Bonzini Paulo Alcantara Paulo Alcantara Cavalcanti Qin Long Qiu Shumin Rodrigo Dias Correa Ruiyu Ni Ryan Harkin Samer El-Haj-Mahmoud Samer El-Haj-Mahmoud Star Zeng Supreeth Venkatesh Tapan Shah Vladislav Vovchenko Yao Jiewen Yao, Jiewen Ye Ting Yonghong Zhu Zhang Lubo jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 fail sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 8401 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH net-next v1 0/8] xen-netback: support toeplitz hashing
From: Paul Durrant Date: Fri, 12 Feb 2016 11:07:50 + > Windows *requires* use of Teoplitz so your position completely rules > out being able to support receive side scaling in Windows PV > frontends on Linux kernel backends, not only for Xen but for any > other hypervisor, which I think is totally unreasonable. We have strong reason to believe that Toeplitz has properties that make it very weak if not exploitable, therefore doing software RSS with Jenkins is superior. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
