Re: [Xen-devel] pvgrub "Error 9: Unknown boot failure" booting Debian Jessie kernel (Was: Re: [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported)

[Juergen Gross]

On 01/12/15 08:15, Juergen Gross wrote:
> On 30/11/15 17:56, Ian Campbell wrote:
>> On Mon, 2015-11-30 at 16:25 +, Ian Campbell wrote:
>>> (d54) Pinning the boot page table pfn 4be3 / mfn 1bfd71/1bfd71
>>> (d54) pin_table: MFN 1bfd71
>>> (XEN) mm.c:2417:d54v0 Bad type (saw 1401 != exp 
>>> 7000) for mfn 165b81 (pfn 4d81)
>>
>> I added a "BUG_ON(*pt_pfn == 0x4d81);" to mini-os's new_pt_frame, which
>> after some messing with gdb to decode produced this stack trace:
>>
>> 716e7: arch_do_exit + 9 in section .text
>> 66176: do_exit + 28 in section .text
>> 6ff68: new_pt_frame + 134 in section .text
>> 70401: need_pgt + 410 in section .text
>> 706ec: do_map_frames + 284 in section .text
>> 66e72: sbrk + 130 in section .text
>> 7768e: _sbrk_r + 30 in section .text
>> 74fa3: _malloc_r + 1219 in section .text
>> 76f3f: _realloc_r + 511 in section .text
>> 31035: unsafe_flush + 46 in section .text
>> 38bc7: unxz + 480 in section .text
>> 310fa: xc_dom_decompress_unsafe + 110 in section .text
>> 38cec: xc_try_xz_decode + 45 in section .text
>> 286ff: xc_dom_probe_bzimage_kernel + 891 in section .text
>> 24613: xc_dom_find_loader + 89 in section .text
>> 24d83: xc_dom_parse_image + 58 in section .text
>> 19d06: kexec + 1012 in section .text
>> 03c27: pv_boot + 97 in section .text
>> 08e4b: boot_func + 52 in section .text
>> 0ab16: run_script + 294 in section .text
>> 10848: run_menu + 3133 in section .text
>> 10fb2: cmain + 1444 in section .text
>> 04447: main + 303 in section .text
>> 66991: call_main + 581 in section .text
>> 03423: thread_starter + 9 in section .text
>>
>> I'm not quite sure what to make of this, in particular I don't see anything
>> in kexec.c which obviously looks after unmapping the heap or brk areas.
> 
> Nah, this backtrace shows a normal allocation path while
> uncompressing the kernel image. I'd expect something like that.
> Why shouldn't mini-os make use of pfn 4d81 somewhere?
> 
> I guess there is something wrong either in mini-os's memory
> allocator (not very likely) or in kexec_allocate(). I'll try to
> check those.

Hmm, kexec_allocate() seems to be a little bit fishy.

I suspect a problem in the loop for the case new_pfn == i. I think
in this case the p2m list will be written with a wrong entry.

Ian, could you verify via:

diff --git a/stubdom/grub/kexec.c b/stubdom/grub/kexec.c
index 8fd9ff9..9421023 100644
--- a/stubdom/grub/kexec.c
+++ b/stubdom/grub/kexec.c
@@ -131,6 +131,8 @@ int kexec_allocate(struct xc_dom_image *dom)
/* Store destination PFN of currently requested page. */
pages_moved2pfns[i] = new_pfn;

+   BUG_ON(new_pfn == i);
+
/* Put old page at new PFN */
dom->p2m_host[new_pfn] = old_mfn;


Juergen

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

Re: [Xen-devel] [PATCH] libxc: try to find last used pfn when migrating

[Juergen Gross]

On 27/11/15 18:16, Andrew Cooper wrote:
> On 27/11/15 15:53, Juergen Gross wrote:
>> On 27/11/15 16:33, Wei Liu wrote:
>>> On Fri, Nov 27, 2015 at 03:50:53PM +0100, Juergen Gross wrote:
 For migration the last used pfn of a guest is needed to size the
 logdirty bitmap and as an upper bound of the page loop. Unfortunately
 there are pv-kernels advertising a much higher maximum pfn as they
 are really using in order to support memory hotplug. This will lead
 to allocation of much more memory in Xen tools during migration as
 really needed.

 Try to find the last used guest pfn of a pv-domu by scanning the p2m
 tree from the last entry towards it's start and search for an entry
 not being invalid.

 Normally the mid pages of the p2m tree containing all invalid entries
 are being reused, so we can just scan the top page for identical
 entries and skip them but the first one.

 Signed-off-by: Juergen Gross 
 ---
  tools/libxc/xc_sr_save.c|  8 
  tools/libxc/xc_sr_save_x86_pv.c | 34 +++---
  2 files changed, 35 insertions(+), 7 deletions(-)

 diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
 index 0c12e56..22b3f18 100644
 --- a/tools/libxc/xc_sr_save.c
 +++ b/tools/libxc/xc_sr_save.c
 @@ -677,6 +677,10 @@ static int setup(struct xc_sr_context *ctx)
  DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
  >save.dirty_bitmap_hbuf);
  
 +rc = ctx->save.ops.setup(ctx);
 +if ( rc )
 +goto err;
 +
  dirty_bitmap = xc_hypercall_buffer_alloc_pages(
 xch, dirty_bitmap, 
 NRPAGES(bitmap_size(ctx->save.p2m_size)));
  ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
 @@ -692,10 +696,6 @@ static int setup(struct xc_sr_context *ctx)
  goto err;
  }
  
>>> p2m_size is set in two places for PV guest.  Since we are now relying on
>>> ops.setup to set p2m_size, the invocation to xc_domain_nr_gpfns in
>>> xc_domain_save should be removed, so that we only have one place to set
>>> p2m_size.
>> I wasn't sure this is needed in save case only. If it is I can make
>> the change, of course. Andrew?

I got this one wrong, sorry. I was relating to the xc_domain_nr_gpfns()
call in x86_pv_domain_info(). And here the question really applies:

Do we need this call at all? I don't think it is needed for the
restore operation. At least in my tests the upper pfn bound was always
taken from the data stream, not from xc_domain_nr_gpfns().

> That is most likely a byproduct of this codes somewhat-organic growth
> over 18 months.
> 
> The check in xc_domain_save() is a check left over from legacy
> migration.  It was present in legacy migration as legacy merged the page
> type into the upper 4 bits of the unsigned long pfn, meaning that a pfn
> of (2^28)-1 was the INVALID_PFN representation in the 32bit stream.
> 
> The check is less important for migration v2, but I left it in as an
> upper sanity check.
> 
> I am not aversed to removing the check if we are no longer going to
> believe the result of XENMEM_maximum_gpfn, and requiring that setup()
> get and sanity check the domain size.

As I'm planning to support migration of larger domains in the future
I think the check has to go away. Why not now.

> (Incidently, it seems crazy that we make a XENMEM_maximum_gpfn hypercall
> to read a value from the shared info page, which is also (about to be)
> mapped locally.)

Indeed. It should be necessary for hvm guests only.


Juergen

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

Re: [Xen-devel] boot xen use legacy bios

[Jan Beulich]

>>> On 29.11.15 at 12:26,  wrote:
> I got the problem of multiple cores CPU cannot be fully used when using UEFI 
> mode. It is suggested that I should be boot the xen in legacy mode. How is it 
> done? 

For BIOSes that don't have a Legacy Support Module, booting in
non-EFI mode just won't work. See my other reply. For BIOSes
that do have an LSM, you may need to find the setup option to
turn it on if it's not on by default.

But generally questions like this belong on xen-users, not xen-devel.

Jan


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

Re: [Xen-devel] Page Fault

[Jan Beulich]

>>> On 29.11.15 at 19:19,  wrote:
> Inside the page fault handler for shadow page tables (sh_page_fault
> function in multi.c) where is the code for swapping in a page from disk?

There is no swapping in from disk in that code. You probably think
of memory-paging, which has nothing to do with shadow code.

Jan


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

Re: [Xen-devel] [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported

[Juergen Gross]

On 30/11/15 11:51, Ian Campbell wrote:
> On Mon, 2015-11-30 at 11:47 +0100, Juergen Gross wrote:
>> On 30/11/15 11:34, Ian Campbell wrote:
>>> On Mon, 2015-11-30 at 11:23 +0100, Juergen Gross wrote:
 On 30/11/15 11:20, Wei Liu wrote:
> On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross wrote:
>>  
>>  /* initrd parameters as specified in start_info page */
>> -unsigned long initrd_start;
>> -unsigned long initrd_len;
>> +uint64_t initrd_start;
>> +uint64_t initrd_len;
>>  
>
> I think these should be of type xen_vaddr_t. Doesn't make a
> difference
> in the end though.

 xen_vaddr_t seems not to be appropriate. It can be either a virtual
 address or a pfn.
>>>
>>> Did you mean a virtual address or a physical _address_? Potentially
>>> mixing
>>> addresses and frame numbers in a single variable seems liable to be
>>> confusing, at best.
>>
>> No, it's really a pfn. And this is part of the stable interface between
>> hypervisor and the pv-domU since more than 5 years now.
> 
> Including the virtual address bit?
> 
> That's a shame.

Before commit 9f41c22a559a7ec039a195f6dc8bca32c66fcd5a it could only be
a virtual address. This commit added the pfn possibility.

I think it was done via pfn rather than physical address as this would
enable the feature to be used for 32 bit domains as well without having
to limit the initrd position to the first 4 GB or having to change the
structure layout.


Juergen


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

[Xen-devel] [PATCH] x86_emulate: Always truncate %eip in 32bit mode

[Andrew Cooper]

_regs.eip needs to be truncated after having size added to it, or emulating an
instruction which crosses the 4GB boundary causes _regs.eip to become invalid,
and fail vmentry checks when returning back to the guest.

The comment /* real hardware doesn't truncate */ seems to appear in c/s
ddef8e16 "Tweak x86 emulator interface." without any justification.

I have not been able to find any information to prove or disprove the claim,
and have not experimented to find the behaviour of real hardware, but
emulating oneself into a vmentry failure is definitely the wrong thing to do.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/arch/x86/x86_emulate/x86_emulate.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c 
b/xen/arch/x86/x86_emulate/x86_emulate.c
index f1454ce..56328f4 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -570,8 +570,9 @@ do{ asm volatile (  
\
 /* Fetch next part of the instruction being emulated. */
 #define insn_fetch_bytes(_size) \
 ({ unsigned long _x = 0, _eip = _regs.eip;  \
-   if ( !mode_64bit() ) _eip = (uint32_t)_eip; /* ignore upper dword */ \
-   _regs.eip += (_size); /* real hardware doesn't truncate */   \
+   _regs.eip += (_size);\
+   if ( !mode_64bit() ) { /* Truncate eip in 32bit mode. */ \
+   _eip = (uint32_t)_eip; _regs.eip = (uint32_t) _regs.eip; }   \
generate_exception_if((uint8_t)(_regs.eip -  \
ctxt->regs->eip) > MAX_INST_LEN, \
  EXC_GP, 0);\
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported

[Ian Campbell]

On Mon, 2015-11-30 at 12:03 +0100, Juergen Gross wrote:
> On 30/11/15 11:52, Ian Campbell wrote:
> > On Mon, 2015-11-30 at 10:51 +, Ian Campbell wrote:
> > > On Mon, 2015-11-30 at 11:47 +0100, Juergen Gross wrote:
> > > > On 30/11/15 11:34, Ian Campbell wrote:
> > > > > On Mon, 2015-11-30 at 11:23 +0100, Juergen Gross wrote:
> > > > > > On 30/11/15 11:20, Wei Liu wrote:
> > > > > > > On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross
> > > > > > > wrote:
> > > > > > > >  
> > > > > > > >  /* initrd parameters as specified in start_info page
> > > > > > > > */
> > > > > > > > -unsigned long initrd_start;
> > > > > > > > -unsigned long initrd_len;
> > > > > > > > +uint64_t initrd_start;
> > > > > > > > +uint64_t initrd_len;
> > > > > > > >  
> > > > > > > 
> > > > > > > I think these should be of type xen_vaddr_t. Doesn't make a
> > > > > > > difference
> > > > > > > in the end though.
> > > > > > 
> > > > > > xen_vaddr_t seems not to be appropriate. It can be either a
> > > > > > virtual
> > > > > > address or a pfn.
> > > > > 
> > > > > Did you mean a virtual address or a physical _address_?
> > > > > Potentially
> > > > > mixing
> > > > > addresses and frame numbers in a single variable seems liable to
> > > > > be
> > > > > confusing, at best.
> > > > 
> > > > No, it's really a pfn. And this is part of the stable interface
> > > > between
> > > > hypervisor and the pv-domU since more than 5 years now.
> > > 
> > > Including the virtual address bit?
> > > 
> > > That's a shame.
> > 
> > ... and that being the case would you mind adding a comment here
> > explaining
> > the two forms of these variables and the flag which indicates which one
> > is
> > "in force" at a given moment.
> 
> The comment in the struct already tells us that initrd_start and
> initrd_len are in the very same format as in the start_info page.
> Both fields are meant to be opaque to most of the domain builder
> parts.
> 
> The only function dealing with the differences is xc_dom_build_image()
> which already contains the appropriate flag. I added this on your
> request. You acked the resulting patch. So why do you want to add
> another comment now?

I hadn't realised at the time that the semantics of these fields was so,
uh, interesting.

Ian.


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

Re: [Xen-devel] [PATCH] x86/PV: hide features dependent on XSAVE when booted with "no-xsave"

[Jan Beulich]

>>> On 30.11.15 at 12:10,  wrote:
> On 30/11/15 11:08, Jan Beulich wrote:
> On 30.11.15 at 11:46,  wrote:
>>> On 30/11/15 10:01, Jan Beulich wrote:
>>> On 27.11.15 at 16:05,  wrote:
> On 27/11/15 11:05, Jan Beulich wrote:
>> ... or when the guest has the XSAVE feature hidden by CPUID policy.
>> Not doing so is at best confusing to guests.
>>
>> Signed-off-by: Jan Beulich 
> These changes here are an improvement (so I don't object to taking them
> ahead of my fullblown levelling series), but they are incomplete.
>
> xsaveopt, xsavec, xsetbv1, xsaves, avx and mpx depend on xsave.
> fma, fma4, f16c, avx2 and xop depend on avx.
 I think the dependencies here are a little fuzzy, and hence I'd
 prefer us to not enforce more strict rules than are truly necessary:

 FMA: Neither Intel's SDM nor AMD's PM state any dependency on AVX.

 FMA4, XOP: AMD's PM doesn't state any dependency on AVX.

 AVX2: While Intel's SDM doesn't say so here either, I agree in this case.

 I.e. my view is that FMA{,4} and XOP are all pretty much
 independent of AVX, and they e.g. imply by themselves presence of
 YMM registers.
>>> The AVX feature means several things, and in this case support for VEX
>>> encoded instructions.
>> Yet I don't think "VEX encoding" == "AVX". See the various VEX-
>> encoded non-SIMD instructions.
>>
>>> Per SDM Vol 2, Table 2-18, any VEX encoded instruction will
>>> unconditionally #UD fault if XCR0[2:1] != '11b' or CR0.OSXSAVE = 0.
>> I.e. a dependency on XSAVE, but not on AVX.
> 
> XCR0[2:1] are the AVX and SSE bits.

You mean the YMM and XMM ones (the latter one is  mis-named in
our code base). It's not well defined whether YMM register presence
correlates to AVX, or is simply flagged by the respective XSTATE
CPUID bit (or a mixture of both). The minimal (and imo more natural)
dependency is just the XSTATE bit.

Jan


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

[Xen-devel] [PATCH OSSTEST] target_fetchurl: Handle undefined $c{HttpProxy}

[Ian Campbell]

Avoiding a usage of a potentially undefined variable.

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

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 18b03b4..28ac572 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -1823,7 +1823,7 @@ END
 sub target_fetchurl($$$;$) {
 my ($ho, $url, $path, $timeo) = @_;
 $timeo ||= 2000;
-my $useproxy = "export http_proxy=$c{HttpProxy};" if $c{HttpProxy};
+my $useproxy = $c{HttpProxy} ? "export http_proxy=$c{HttpProxy};" : "";
 target_cmd_root($ho, <

Re: [Xen-devel] xen can only detect one core of multiple cores cpu

[quizy_jo...@outlook.com]

>>> On 28.11.15 at 20:24,  wrote:
> On 28/11/15 17:23, quizyjones wrote:
>> I'm using a  Intel E5-2603 v3 @ 1.60GHz CPU of 6 cores. However, the
>> dom0 can only find one core. here are some information that may helps
>> in analyzing.
> 
> From `xl dmesg`
> 
> (XEN) ACPI Error (tbxfroot-0218): A valid RSDP was not found [20070126]
> ...
> (XEN) SMP motherboard not detected.
> 
> Xen cannot find any ACPI tables, and finds no secondary CPUs.  As a
> knock-on effect, dom0 only gets one.
> 
> How is your BIOS configured?  Does booting Linux natively work?

Presumably this is set to UEFI mode, in which case booting via grub2
requires chainload to be used, or grub2 to be avoided.

Jan

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

Re: [Xen-devel] [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported

[Ian Campbell]

On Mon, 2015-11-30 at 11:23 +0100, Juergen Gross wrote:
> On 30/11/15 11:20, Wei Liu wrote:
> > On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross wrote:
> > >  
> > >  /* initrd parameters as specified in start_info page */
> > > -unsigned long initrd_start;
> > > -unsigned long initrd_len;
> > > +uint64_t initrd_start;
> > > +uint64_t initrd_len;
> > >  
> > 
> > I think these should be of type xen_vaddr_t. Doesn't make a difference
> > in the end though.
> 
> xen_vaddr_t seems not to be appropriate. It can be either a virtual
> address or a pfn.

Did you mean a virtual address or a physical _address_? Potentially mixing
addresses and frame numbers in a single variable seems liable to be
confusing, at best.

Ian.

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

Re: [Xen-devel] [PATCH v9 6/9] libxc/xen: introduce a start info structure for HVMlite guests

[Jan Beulich]

>>> On 27.11.15 at 14:43,  wrote:
> This structure contains the physical address of the command line, as well as
> the physical address of the list of loaded modules. The physical address of
> this structure is passed to the guest at boot time in the %ebx register.
> 
> Signed-off-by: Roger Pau Monné 
> Reviewed-by: Andrew Cooper 
> Acked-by: Wei Liu 

Public interface part
Acked-by: Jan Beulich 

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

Re: [Xen-devel] [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported

[Ian Campbell]

On Mon, 2015-11-30 at 11:47 +0100, Juergen Gross wrote:
> On 30/11/15 11:34, Ian Campbell wrote:
> > On Mon, 2015-11-30 at 11:23 +0100, Juergen Gross wrote:
> > > On 30/11/15 11:20, Wei Liu wrote:
> > > > On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross wrote:
> > > > >  
> > > > >  /* initrd parameters as specified in start_info page */
> > > > > -unsigned long initrd_start;
> > > > > -unsigned long initrd_len;
> > > > > +uint64_t initrd_start;
> > > > > +uint64_t initrd_len;
> > > > >  
> > > > 
> > > > I think these should be of type xen_vaddr_t. Doesn't make a
> > > > difference
> > > > in the end though.
> > > 
> > > xen_vaddr_t seems not to be appropriate. It can be either a virtual
> > > address or a pfn.
> > 
> > Did you mean a virtual address or a physical _address_? Potentially
> > mixing
> > addresses and frame numbers in a single variable seems liable to be
> > confusing, at best.
> 
> No, it's really a pfn. And this is part of the stable interface between
> hypervisor and the pv-domU since more than 5 years now.

Including the virtual address bit?

That's a shame.

Ian.

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

Re: [Xen-devel] [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported

[Juergen Gross]

On 30/11/15 11:52, Ian Campbell wrote:
> On Mon, 2015-11-30 at 10:51 +, Ian Campbell wrote:
>> On Mon, 2015-11-30 at 11:47 +0100, Juergen Gross wrote:
>>> On 30/11/15 11:34, Ian Campbell wrote:
 On Mon, 2015-11-30 at 11:23 +0100, Juergen Gross wrote:
> On 30/11/15 11:20, Wei Liu wrote:
>> On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross wrote:
>>>  
>>>  /* initrd parameters as specified in start_info page */
>>> -unsigned long initrd_start;
>>> -unsigned long initrd_len;
>>> +uint64_t initrd_start;
>>> +uint64_t initrd_len;
>>>  
>>
>> I think these should be of type xen_vaddr_t. Doesn't make a
>> difference
>> in the end though.
>
> xen_vaddr_t seems not to be appropriate. It can be either a virtual
> address or a pfn.

 Did you mean a virtual address or a physical _address_? Potentially
 mixing
 addresses and frame numbers in a single variable seems liable to be
 confusing, at best.
>>>
>>> No, it's really a pfn. And this is part of the stable interface between
>>> hypervisor and the pv-domU since more than 5 years now.
>>
>> Including the virtual address bit?
>>
>> That's a shame.
> 
> ... and that being the case would you mind adding a comment here explaining
> the two forms of these variables and the flag which indicates which one is
> "in force" at a given moment.

The comment in the struct already tells us that initrd_start and
initrd_len are in the very same format as in the start_info page.
Both fields are meant to be opaque to most of the domain builder
parts.

The only function dealing with the differences is xc_dom_build_image()
which already contains the appropriate flag. I added this on your
request. You acked the resulting patch. So why do you want to add
another comment now?


Juergen


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

Re: [Xen-devel] [PATCH] x86/PV: hide features dependent on XSAVE when booted with "no-xsave"

[Jan Beulich]

>>> On 30.11.15 at 11:46,  wrote:
> On 30/11/15 10:01, Jan Beulich wrote:
> On 27.11.15 at 16:05,  wrote:
>>> On 27/11/15 11:05, Jan Beulich wrote:
 ... or when the guest has the XSAVE feature hidden by CPUID policy.
 Not doing so is at best confusing to guests.

 Signed-off-by: Jan Beulich 
>>> These changes here are an improvement (so I don't object to taking them
>>> ahead of my fullblown levelling series), but they are incomplete.
>>>
>>> xsaveopt, xsavec, xsetbv1, xsaves, avx and mpx depend on xsave.
>>> fma, fma4, f16c, avx2 and xop depend on avx.
>> I think the dependencies here are a little fuzzy, and hence I'd
>> prefer us to not enforce more strict rules than are truly necessary:
>>
>> FMA: Neither Intel's SDM nor AMD's PM state any dependency on AVX.
>>
>> FMA4, XOP: AMD's PM doesn't state any dependency on AVX.
>>
>> AVX2: While Intel's SDM doesn't say so here either, I agree in this case.
>>
>> I.e. my view is that FMA{,4} and XOP are all pretty much
>> independent of AVX, and they e.g. imply by themselves presence of
>> YMM registers.
> 
> The AVX feature means several things, and in this case support for VEX
> encoded instructions.

Yet I don't think "VEX encoding" == "AVX". See the various VEX-
encoded non-SIMD instructions.

> Per SDM Vol 2, Table 2-18, any VEX encoded instruction will
> unconditionally #UD fault if XCR0[2:1] != '11b' or CR0.OSXSAVE = 0.

I.e. a dependency on XSAVE, but not on AVX.

Jan


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

Re: [Xen-devel] [PATCH] x86/PV: hide features dependent on XSAVE when booted with "no-xsave"

[Andrew Cooper]

On 30/11/15 11:08, Jan Beulich wrote:
 On 30.11.15 at 11:46,  wrote:
>> On 30/11/15 10:01, Jan Beulich wrote:
>> On 27.11.15 at 16:05,  wrote:
 On 27/11/15 11:05, Jan Beulich wrote:
> ... or when the guest has the XSAVE feature hidden by CPUID policy.
> Not doing so is at best confusing to guests.
>
> Signed-off-by: Jan Beulich 
 These changes here are an improvement (so I don't object to taking them
 ahead of my fullblown levelling series), but they are incomplete.

 xsaveopt, xsavec, xsetbv1, xsaves, avx and mpx depend on xsave.
 fma, fma4, f16c, avx2 and xop depend on avx.
>>> I think the dependencies here are a little fuzzy, and hence I'd
>>> prefer us to not enforce more strict rules than are truly necessary:
>>>
>>> FMA: Neither Intel's SDM nor AMD's PM state any dependency on AVX.
>>>
>>> FMA4, XOP: AMD's PM doesn't state any dependency on AVX.
>>>
>>> AVX2: While Intel's SDM doesn't say so here either, I agree in this case.
>>>
>>> I.e. my view is that FMA{,4} and XOP are all pretty much
>>> independent of AVX, and they e.g. imply by themselves presence of
>>> YMM registers.
>> The AVX feature means several things, and in this case support for VEX
>> encoded instructions.
> Yet I don't think "VEX encoding" == "AVX". See the various VEX-
> encoded non-SIMD instructions.
>
>> Per SDM Vol 2, Table 2-18, any VEX encoded instruction will
>> unconditionally #UD fault if XCR0[2:1] != '11b' or CR0.OSXSAVE = 0.
> I.e. a dependency on XSAVE, but not on AVX.

XCR0[2:1] are the AVX and SSE bits.

~Andrew

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

Re: [Xen-devel] [PATCH v2 01/11] xen/arm: vgic-v2: Implement correctly ICFGR{0, 1} read-only

[Julien Grall]

Hi Ian,

On 25/11/15 12:29, Ian Campbell wrote:
> On Tue, 2015-11-24 at 17:14 +, Ian Campbell wrote:
>> @@ -507,10 +507,12 @@ static int vgic_v2_distr_mmio_write(struct vcpu
>>> *v,
>>> mmio_info_t *info,
>>>  
>>>  case GICD_ICFGR: /* SGIs */
>>>  goto write_ignore_32;
>>> -case GICD_ICFGR + 1: /* PPIs */
>>> +
>>> +case GICD_ICFGR1:
>>
>> This should have a /* PPIs */ comment I think?
>>
>> I think I could add that on commit if you agree.
> 
> With you being away this week I decided this wasn't worth holding up 11
> patches over and committed without. Instead:

FWIW, I'm fine with that as it was a mistake while I wrote the patch.

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH] libxc: try to find last used pfn when migrating

[Juergen Gross]

On 30/11/15 10:47, Andrew Cooper wrote:
> On 30/11/15 08:17, Juergen Gross wrote:
>> On 27/11/15 18:16, Andrew Cooper wrote:
>>> On 27/11/15 15:53, Juergen Gross wrote:
 On 27/11/15 16:33, Wei Liu wrote:
> On Fri, Nov 27, 2015 at 03:50:53PM +0100, Juergen Gross wrote:
>> For migration the last used pfn of a guest is needed to size the
>> logdirty bitmap and as an upper bound of the page loop. Unfortunately
>> there are pv-kernels advertising a much higher maximum pfn as they
>> are really using in order to support memory hotplug. This will lead
>> to allocation of much more memory in Xen tools during migration as
>> really needed.
>>
>> Try to find the last used guest pfn of a pv-domu by scanning the p2m
>> tree from the last entry towards it's start and search for an entry
>> not being invalid.
>>
>> Normally the mid pages of the p2m tree containing all invalid entries
>> are being reused, so we can just scan the top page for identical
>> entries and skip them but the first one.
>>
>> Signed-off-by: Juergen Gross 
>> ---
>>  tools/libxc/xc_sr_save.c|  8 
>>  tools/libxc/xc_sr_save_x86_pv.c | 34 +++---
>>  2 files changed, 35 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
>> index 0c12e56..22b3f18 100644
>> --- a/tools/libxc/xc_sr_save.c
>> +++ b/tools/libxc/xc_sr_save.c
>> @@ -677,6 +677,10 @@ static int setup(struct xc_sr_context *ctx)
>>  DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
>>  >save.dirty_bitmap_hbuf);
>>  
>> +rc = ctx->save.ops.setup(ctx);
>> +if ( rc )
>> +goto err;
>> +
>>  dirty_bitmap = xc_hypercall_buffer_alloc_pages(
>> xch, dirty_bitmap, 
>> NRPAGES(bitmap_size(ctx->save.p2m_size)));
>>  ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
>> @@ -692,10 +696,6 @@ static int setup(struct xc_sr_context *ctx)
>>  goto err;
>>  }
>>  
> p2m_size is set in two places for PV guest.  Since we are now relying on
> ops.setup to set p2m_size, the invocation to xc_domain_nr_gpfns in
> xc_domain_save should be removed, so that we only have one place to set
> p2m_size.
 I wasn't sure this is needed in save case only. If it is I can make
 the change, of course. Andrew?
>> I got this one wrong, sorry. I was relating to the xc_domain_nr_gpfns()
>> call in x86_pv_domain_info(). And here the question really applies:
>>
>> Do we need this call at all? I don't think it is needed for the
>> restore operation. At least in my tests the upper pfn bound was always
>> taken from the data stream, not from xc_domain_nr_gpfns().
> 
> x86_pv_domain_info() was the result of attempting to consolidate all the
> random checks for PV guests, for both the save and restore sides,
> although most of the callsites subsequently vanished.
> 
> The function as a whole gets used several times on the restore side, and
> once of save side.  Unilaterally rewriting ctx->x86_pv.max_pfn is not
> the best option, so would probably be best to disband
> x86_pv_domain_info() altogether and hoist the width/p2m checks into the
> relevant callers.

Hmm, this would mean to have the remaining code of x86_pv_domain_info()
two or three times. I think it's better to keep it without the call of
xc_domain_nr_gpfns().

> 
>>
>>> That is most likely a byproduct of this codes somewhat-organic growth
>>> over 18 months.
>>>
>>> The check in xc_domain_save() is a check left over from legacy
>>> migration.  It was present in legacy migration as legacy merged the page
>>> type into the upper 4 bits of the unsigned long pfn, meaning that a pfn
>>> of (2^28)-1 was the INVALID_PFN representation in the 32bit stream.
>>>
>>> The check is less important for migration v2, but I left it in as an
>>> upper sanity check.
>>>
>>> I am not aversed to removing the check if we are no longer going to
>>> believe the result of XENMEM_maximum_gpfn, and requiring that setup()
>>> get and sanity check the domain size.
>> As I'm planning to support migration of larger domains in the future
>> I think the check has to go away. Why not now.
> 
> Ideally, there should still be a sanity check.  Ian's idea of the caller
> passing the expected size seems like a good candidate.

But which size is expected? I think it would make more sense to limit
the needed dom0 memory to some fraction of the guest size. We could use
a default which might be configurable globally or per domain.


Juergen

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

Re: [Xen-devel] [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported

[Juergen Gross]

On 30/11/15 12:23, Ian Campbell wrote:
> On Mon, 2015-11-30 at 12:03 +0100, Juergen Gross wrote:
>> On 30/11/15 11:52, Ian Campbell wrote:
>>> On Mon, 2015-11-30 at 10:51 +, Ian Campbell wrote:
 On Mon, 2015-11-30 at 11:47 +0100, Juergen Gross wrote:
> On 30/11/15 11:34, Ian Campbell wrote:
>> On Mon, 2015-11-30 at 11:23 +0100, Juergen Gross wrote:
>>> On 30/11/15 11:20, Wei Liu wrote:
 On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross
 wrote:
>  
>  /* initrd parameters as specified in start_info page
> */
> -unsigned long initrd_start;
> -unsigned long initrd_len;
> +uint64_t initrd_start;
> +uint64_t initrd_len;
>  

 I think these should be of type xen_vaddr_t. Doesn't make a
 difference
 in the end though.
>>>
>>> xen_vaddr_t seems not to be appropriate. It can be either a
>>> virtual
>>> address or a pfn.
>>
>> Did you mean a virtual address or a physical _address_?
>> Potentially
>> mixing
>> addresses and frame numbers in a single variable seems liable to
>> be
>> confusing, at best.
>
> No, it's really a pfn. And this is part of the stable interface
> between
> hypervisor and the pv-domU since more than 5 years now.

 Including the virtual address bit?

 That's a shame.
>>>
>>> ... and that being the case would you mind adding a comment here
>>> explaining
>>> the two forms of these variables and the flag which indicates which one
>>> is
>>> "in force" at a given moment.
>>
>> The comment in the struct already tells us that initrd_start and
>> initrd_len are in the very same format as in the start_info page.
>> Both fields are meant to be opaque to most of the domain builder
>> parts.
>>
>> The only function dealing with the differences is xc_dom_build_image()
>> which already contains the appropriate flag. I added this on your
>> request. You acked the resulting patch. So why do you want to add
>> another comment now?
> 
> I hadn't realised at the time that the semantics of these fields was so,
> uh, interesting.

:-)

I guess due to the lack of a comment? ;-)

Okay, I'll add one when submitting the patch after (hopefully) Boris
confirmed it is fixing his problem.


Juergen


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

Re: [Xen-devel] [PATCH] x86/PV: hide features dependent on XSAVE when booted with "no-xsave"

[Andrew Cooper]

On 30/11/15 10:01, Jan Beulich wrote:
 On 27.11.15 at 16:05,  wrote:
>> On 27/11/15 11:05, Jan Beulich wrote:
>>> ... or when the guest has the XSAVE feature hidden by CPUID policy.
>>> Not doing so is at best confusing to guests.
>>>
>>> Signed-off-by: Jan Beulich 
>> These changes here are an improvement (so I don't object to taking them
>> ahead of my fullblown levelling series), but they are incomplete.
>>
>> xsaveopt, xsavec, xsetbv1, xsaves, avx and mpx depend on xsave.
>> fma, fma4, f16c, avx2 and xop depend on avx.
> I think the dependencies here are a little fuzzy, and hence I'd
> prefer us to not enforce more strict rules than are truly necessary:
>
> FMA: Neither Intel's SDM nor AMD's PM state any dependency on AVX.
>
> FMA4, XOP: AMD's PM doesn't state any dependency on AVX.
>
> AVX2: While Intel's SDM doesn't say so here either, I agree in this case.
>
> I.e. my view is that FMA{,4} and XOP are all pretty much
> independent of AVX, and they e.g. imply by themselves presence of
> YMM registers.

The AVX feature means several things, and in this case support for VEX
encoded instructions.

Per SDM Vol 2, Table 2-18, any VEX encoded instruction will
unconditionally #UD fault if XCR0[2:1] != '11b' or CR0.OSXSAVE = 0.

FMA, FMA4 and XOP may only be VEX encoded, so do explicitly depend on
AVX support.

(Both the Intel and the AMD manuals are poor at explicitly listing
dependencies.  I have spent far too much time attempting to reverse
engineer the real dependency trees from the information provided.)

~Andrew

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

Re: [Xen-devel] [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported

[Juergen Gross]

On 30/11/15 11:34, Ian Campbell wrote:
> On Mon, 2015-11-30 at 11:23 +0100, Juergen Gross wrote:
>> On 30/11/15 11:20, Wei Liu wrote:
>>> On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross wrote:
  
  /* initrd parameters as specified in start_info page */
 -unsigned long initrd_start;
 -unsigned long initrd_len;
 +uint64_t initrd_start;
 +uint64_t initrd_len;
  
>>>
>>> I think these should be of type xen_vaddr_t. Doesn't make a difference
>>> in the end though.
>>
>> xen_vaddr_t seems not to be appropriate. It can be either a virtual
>> address or a pfn.
> 
> Did you mean a virtual address or a physical _address_? Potentially mixing
> addresses and frame numbers in a single variable seems liable to be
> confusing, at best.

No, it's really a pfn. And this is part of the stable interface between
hypervisor and the pv-domU since more than 5 years now.


Juergen


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

[Xen-devel] Xen Security Advisory 162 (CVE-2015-7504) - heap buffer overflow vulnerability in pcnet emulator

[Xen . org security team]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Xen Security Advisory CVE-2015-7504 / XSA-162
  version 2

 heap buffer overflow vulnerability in pcnet emulator

UPDATES IN VERSION 2


Public release.

Correct cut and paste reference to bootloaders in "DEPLOYMENT DURING
EMBARGO" section, which should have instead referred to the
configuration changes.

ISSUE DESCRIPTION
=

The QEMU security team has predisclosed the following advisory:

The AMD PC-Net II emulator(hw/net/pcnet.c), while receiving
packets in loopback mode, appends CRC code to the receive
buffer. If the data size given is same as the buffer size(4096),
the appended CRC code overwrites 4 bytes after the s->buffer,
making the adjacent 's->irq' object point to a new location.

IMPACT
==

A guest which has access to an emulated PCNET network device
(e.g. with "model=pcnet" in their VIF configuration) can exploit this
vulnerability to take over the qemu process elevating its privilege to
that of the qemu process.

VULNERABLE SYSTEMS
==

All Xen systems running x86 HVM guests without stubdomains which have
been configured to use the PCNET emulated driver model are
vulnerable.

The default configuration is NOT vulnerable (because it does not
emulate PCNET NICs).

Systems running only PV guests are NOT vulnerable.

Systems using qemu-dm stubdomain device models (for example, by
specifying "device_model_stubdomain_override=1" in xl's domain
configuration files) are NOT vulnerable.

Both the traditional "qemu-xen" or upstream qemu device models are
potentially vulnerable.

ARM systems are NOT vulnerable.

MITIGATION
==

Avoiding the use of emulated network devices altogether, by specifying
a PV only VIF in the domain configuration file will avoid this
issue.

Avoiding the use of the PCNET device in favour of other emulations
will also avoid this issue.

Enabling stubdomains will mitigate this issue, by reducing the
escalation to only those privileges accorded to the service domain.

qemu-dm stubdomains are only available with the traditional "qemu-xen"
version.

RESOLUTION
==

The QEMU security team have supplied the attached xsa162-qemuu.patch
which it is believed will resolve the issue. However this patch has
not undergone the usual reviews and has not yet been accepted by QEMU
upstream.

The backports were created by the Xen Project security team on the same
basis.

xsa162-qemuu.patch   qemu upstream, Xen unstable, 4.6.x, 4.5.x, 4.4.x
xsa162-qemuu-4.3.patch   Xen 4.3.x
xsa162-qemut-4.3.patch   qemu-xen-traditional, Xen unstable, 4.5.x, 4.4.x, 
4.3.x

$ sha256sum xsa162*
5844debcfdf606030aaa98f32a5920bc64c659dfae6062f24ab98e9008d8bf86  
xsa162-qemut.patch
73e5857570b7464a2118a3ae6a8f424e01effd684c67773fada22a8411199238  
xsa162-qemuu.patch
4a0ded68cc20d64752ef72e12983b20a4b14fef9b14e8774d889cfa34201909d  
xsa162-qemuu-4.3.patch
$


CREDITS
===

This issue was discovered by Qinghao Tang of the Qihoo 360 Marvel
Team.


DEPLOYMENT DURING EMBARGO
=

Deployment of the patch described above (or others which are
substantially similar) is permitted during the embargo, even on
public-facing systems with untrusted guest users and administrators.

However deployment of the mitigations described above is not permitted
(except where all the affected systems and VMs are administered and
used only by organisations which are members of the Xen Project
Security Issues Predisclosure List).  Specifically, deployment on
public cloud systems is NOT permitted.

This is because in all cases the configuration change may be visible
to the guest which could lead to the rediscovery of the vulnerability.

But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.


(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.12 (GNU/Linux)

iQEcBAEBAgAGBQJWXCrJAAoJEIP+FMlX6CvZEtkIAJJYN60maax4jOLKUNGJZcmO
MLTxucr4P2ffw5sNyNYJDHo7Ui5qTdx62uPQHAuYc8mt7x7g9+zhWH39XfFe/9KR
ZVqWAQeoFVT030dQXkuWQkr1ryXzWF/xIUzFsD4F0d3pXY3WNxTH5hKjmXxCUQzT
jM3h3hc4a2+BdTxL527liAiiG31z0sLMqop2V7346yqM5g+HK83DxN2hNackFWZx
PijuBIFO/L9FZiXvcsMtBllaHVko089MBtTF7nnOav1hJefn4yGDBdoj0D+r8PiB
6376dASIwznXV6YZcg62N2HxbKn4tnjr6HumM5kWlXUM7+f2eG3kfM4re7A3ry8=
=6xNZ
-END PGP SIGNATURE-


xsa162-qemut.patch

Re: [Xen-devel] [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported

[Ian Campbell]

On Mon, 2015-11-30 at 10:51 +, Ian Campbell wrote:
> On Mon, 2015-11-30 at 11:47 +0100, Juergen Gross wrote:
> > On 30/11/15 11:34, Ian Campbell wrote:
> > > On Mon, 2015-11-30 at 11:23 +0100, Juergen Gross wrote:
> > > > On 30/11/15 11:20, Wei Liu wrote:
> > > > > On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross wrote:
> > > > > >  
> > > > > >  /* initrd parameters as specified in start_info page */
> > > > > > -unsigned long initrd_start;
> > > > > > -unsigned long initrd_len;
> > > > > > +uint64_t initrd_start;
> > > > > > +uint64_t initrd_len;
> > > > > >  
> > > > > 
> > > > > I think these should be of type xen_vaddr_t. Doesn't make a
> > > > > difference
> > > > > in the end though.
> > > > 
> > > > xen_vaddr_t seems not to be appropriate. It can be either a virtual
> > > > address or a pfn.
> > > 
> > > Did you mean a virtual address or a physical _address_? Potentially
> > > mixing
> > > addresses and frame numbers in a single variable seems liable to be
> > > confusing, at best.
> > 
> > No, it's really a pfn. And this is part of the stable interface between
> > hypervisor and the pv-domU since more than 5 years now.
> 
> Including the virtual address bit?
> 
> That's a shame.

... and that being the case would you mind adding a comment here explaining
the two forms of these variables and the flag which indicates which one is
"in force" at a given moment.

Ian.

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

[Xen-devel] [distros-debian-sid test] 38388: trouble: blocked/broken/pass

[Platform Team regression test user]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-pvops 3 host-install(3) broken REGR. vs. 38323
 build-amd64   3 host-install(3) broken REGR. vs. 38323
 build-i3863 host-install(3) broken REGR. vs. 38323
 build-i386-pvops  3 host-install(3) broken REGR. vs. 38323
 build-amd64-pvops 3 host-install(3) broken REGR. vs. 38323

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

baseline version:
 flight   38323

jobs:
 build-amd64  broken  
 build-armhf  pass
 build-i386   broken  
 build-amd64-pvopsbroken  
 build-armhf-pvopsbroken  
 build-i386-pvops broken  
 test-amd64-amd64-amd64-sid-netboot-pvgrubblocked 
 test-amd64-i386-i386-sid-netboot-pvgrub  blocked 
 test-amd64-i386-amd64-sid-netboot-pygrub blocked 
 test-armhf-armhf-armhf-sid-netboot-pygrubblocked 
 test-amd64-amd64-i386-sid-netboot-pygrub blocked 



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] [PATCH] x86_emulate: Always truncate %eip in 32bit mode

[Jan Beulich]

>>> On 30.11.15 at 12:07,  wrote:
> _regs.eip needs to be truncated after having size added to it, or emulating an
> instruction which crosses the 4GB boundary causes _regs.eip to become invalid,
> and fail vmentry checks when returning back to the guest.
> 
> The comment /* real hardware doesn't truncate */ seems to appear in c/s
> ddef8e16 "Tweak x86 emulator interface." without any justification.

Considering how the code looked like before this commit, ...

> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -570,8 +570,9 @@ do{ asm volatile (
>   \
>  /* Fetch next part of the instruction being emulated. */
>  #define insn_fetch_bytes(_size) \
>  ({ unsigned long _x = 0, _eip = _regs.eip;  \
> -   if ( !mode_64bit() ) _eip = (uint32_t)_eip; /* ignore upper dword */ \
> -   _regs.eip += (_size); /* real hardware doesn't truncate */   \
> +   _regs.eip += (_size);\
> +   if ( !mode_64bit() ) { /* Truncate eip in 32bit mode. */ \
> +   _eip = (uint32_t)_eip; _regs.eip = (uint32_t) _regs.eip; }   \

... don't you think we would better switch back to
_register_address_increment()? Afaik in a 16-bit code segment
only the lower 16 bits actually get looked at.

Jan


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

[Xen-devel] [PATCH OSSTEST] ts-debian-di-install: Don't set runvars for netboot kernel+ramdisk as outputs

[Ian Campbell]

Currently these runvars are either URLs provided by the definition
(e.g. make-flight) or output controller-relative paths created by the
execution (in the case where they aren't from the definition).

This wierd dual-semantics is confusing and wrong, and in particular is
broken if the test step is rerun (e.g. in standalone mode).

In the case where they are outputs only  these paths is information
only. The information is already available in the full logs so
dropping the runvars here merely removes the information from the
summary table. It's not so useful that this is an issue.

Signed-off-by: Ian Campbell 
---
 ts-debian-di-install | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/ts-debian-di-install b/ts-debian-di-install
index a9b3b20..96acd0e 100755
--- a/ts-debian-di-install
+++ b/ts-debian-di-install
@@ -163,9 +163,6 @@ sub setup_netboot($$$)
 
target_putfile_root($ho, 60, $kernel, "$didir/kernel_${suite}_${arch}");
target_putfile_root($ho, 60, $ramdisk, 
"$didir/ramdisk_${suite}_${arch}");
-
-   store_runvar("$gho->{Guest}_netboot_kernel", $kernel);
-   store_runvar("$gho->{Guest}_netboot_ramdisk", $ramdisk);
 }
 
 return <

Re: [Xen-devel] [PATCH v9 5/9] xen/x86: allow HVM guests to use hypercalls to bring up vCPUs

[Julien Grall]

Hi Roger,

On 27/11/15 13:43, Roger Pau Monne wrote:
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index f56b7ff..3690eec 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1208,11 +1208,34 @@ void unmap_vcpu_info(struct vcpu *v)
>  put_page_and_type(mfn_to_page(mfn));
>  }
>  
> +int default_initalize_vcpu(struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)

For here and everywhere this function is called:

s/initalize/initialize/

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v2 02/11] xen/arm: vgic-v3: Don't try to emulate IROUTER which doesn't exist in the spec

[Julien Grall]

Hi Ian,

On 24/11/15 17:17, Ian Campbell wrote:
> On Wed, 2015-11-18 at 17:27 +, Julien Grall wrote:
> 
> Subject: ... which do not exist in the ...
> 
>> The range of valid IROUTER are n = 32 - 1019 (see 8.9.13 in IHI 0069A)
>> which correspond to the offset 0x6100-0x7FD8.
>>
>> Other offset are invalid and therefore should not be emulated.
> 
> "offsets"
> 
>>
>> Also remove the now unused label read_as_zero_64 and write_ignore_64.
>>
>> Note that GICD_IROUTER is kept to accomadate the GICv3 drivers which has
> 
> "accommodate"
> 
>> been in part taken from Linux.
>>
>> Signed-off-by: Julien Grall 
> 
> Accesses to 0x6000-0x60FF are going to be noisy now, I suppose that is OK.

Well, those registers are reserved so the guests are not supposed to
access them.

> Acked-by: Ian Campbell 

Thank you!

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] unhandled word causes Xen crash with recent Linux kernels, was: Re: [PATCH v2 05/11] xen/arm: vgic: Properly emulate the full register

[Julien Grall]

Hi Stefano,

On 25/11/15 12:15, Stefano Stabellini wrote:
> Hi Shannon,
> 
> On Wed, 25 Nov 2015, Shannon Zhao wrote:
>> Upstream Linux kernel applies below patch which will write
>> GICD_ICACTIVER. But since Xen doesn't support it, so it will cause Dom0
>> initializes GIC failed.
>>
>> 0eece2b22849c90b730815c893425a36b9d10fd5 (irqchip/gic: Make sure all
>> interrupts are deactivated at boot)
>>
>> (XEN) d0v0: vGICD: unhandled word write 0x to ICACTIVER4
>> (XEN) traps.c:2447:d0v0 HSR=0x93860046 pc=0xffc0008d63f0
>> gva=0xff804384 gpa=0x002f000384
>> (XEN) DOM0: Unhandled fault: ttbr address size fault (0x9600) at
>> 0xff804384
>> (XEN) DOM0: Internal error: : 9600 [#1] PREEMPT SMP
>> (XEN) DOM0: Modules linked in:
>> (XEN) DOM0: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.4.0-rc2+ #364
>> (XEN) DOM0: Hardware name: (null) (DT)
>> (XEN) DOM0: task: ffc000969970 ti: ffc00095c000 task.ti:
>> ffc00095c000
>> (XEN) DOM0: PC is at gic_dist_config+0x78/0xa0
>> (XEN) DOM0: LR is at __gic_init_bases+0x240/0x2bc
>>
>> Do we have a plan to fix this?
> 
> Thanks for the reporting the issue, I can reproduce the problem.  Given
> that this is a very serious regression and that we cannot really "fix"
> the Linux side because Linux is not doing anything wrong, I think we
> have to go with a very simple change, something we can easily backport
> to all past Xen releases.
> 
> I suggest we turn the "unhandled word write" into a write_ignore, see
> below:
> 
> ---
> 
> xen/arm: ignore GICD_ICACTIVER writes


This need more rational in the commit message to explain why you decided
to implement write ignore.

> 
> Signed-off-by: Stefano Stabellini 
> 
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index f7d784b..8585c44 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -332,11 +332,8 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
> mmio_info_t *info,
>  return 0;
>  
>  case GICD_ICACTIVER ... GICD_ICACTIVERN:
> -if ( dabt.size != DABT_WORD ) goto bad_width;
> -printk(XENLOG_G_ERR
> -   "%pv: vGICD: unhandled word write %#"PRIregister" to 
> ICACTIVER%d\n",
> -   v, r, gicd_reg - GICD_ICACTIVER);implementing write ignore is 
> fine.
> -return 0;

I would prefer if you retain the printk, it helps the guest developer to
know that we don't support GICD_I*ACTIVER registers.

Maybe you can turn it to a XENLOG_G_DEBUG.

> +/* we should really be implementing this */
> +goto write_ignore_32;
>  
>  case GICD_ITARGETSR ... GICD_ITARGETSR + 7:
>  /* SGI/PPI target is read only */
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index b5249ff..6d77373 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -421,11 +421,8 @@ static int __vgic_v3_distr_common_mmio_write(const char 
> *name, struct vcpu *v,
>  return 0;
>  
>  case GICD_ICACTIVER ... GICD_ICACTIVERN:
> -if ( dabt.size != DABT_WORD ) goto bad_width;
> -printk(XENLOG_G_ERR
> -   "%pv: %s: unhandled word write %#"PRIregister" to 
> ICACTIVER%d\n",
> -   v, name, r, reg - GICD_ICACTIVER);

Ditto

> -return 0;
> +/* we should really be implementing this */
> +goto write_ignore_32;
>  
>  case GICD_IPRIORITYR ... GICD_IPRIORITYRN:
>  if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto 
> bad_width;
> 

Regards,

-- 
Julien Grall

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

[Xen-devel] [RFC PATCH] libxl: add screendump API

[Chunyan Liu]

Currently libvirt kvm can support domain screenshot but libxl
cannot. This patch is trying to add screendump API in libxl
by calling qmp 'screendump' command, so to support screenshot
for domains.

Signed-off-by: Chunyan Liu 
---
 tools/libxl/libxl.c  | 36 
 tools/libxl/libxl.h  |  9 +
 tools/libxl/libxl_internal.h |  2 ++
 tools/libxl/libxl_qmp.c  |  8 
 4 files changed, 55 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index bd3aac8..712ea5a 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -6824,6 +6824,42 @@ out:
 return rc;
 }
 
+int libxl_domain_screendump(libxl_ctx *ctx, uint32_t domid,
+const char *filename)
+{
+GC_INIT(ctx);
+char *pid;
+int rc, dm_present;
+
+switch (libxl__domain_type(gc, domid)) {
+case LIBXL_DOMAIN_TYPE_HVM:
+if (!libxl_get_stubdom_id(ctx, domid))
+dm_present = 1;
+else
+dm_present = 0;
+break;
+case LIBXL_DOMAIN_TYPE_PV:
+pid = libxl__xs_read(gc, XBT_NULL, 
GCSPRINTF("/local/domain/%d/image/device-model-pid", domid));
+dm_present = (pid != NULL);
+break;
+case LIBXL_DOMAIN_TYPE_INVALID:
+default:
+rc = ERROR_FAIL;
+goto out;
+}
+
+if (dm_present) {
+rc = libxl__qmp_screendump(gc, domid, filename);
+} else {
+LOG(ERROR, "Not supported");
+rc = ERROR_FAIL;
+}
+
+out:
+GC_FREE;
+return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 6b73848..f1fb5b7 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -218,6 +218,12 @@
 #define LIBXL_HAVE_SOFT_RESET 1
 
 /*
+ * LIBXL_HAVE_SCREENDUMP indicates that libxl supports taking
+ * screenshot for domains.
+ */
+#define LIBXL_HAVE_SCREENDUMP 1
+
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
@@ -1657,6 +1663,9 @@ int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
 int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
 int libxl_send_debug_keys(libxl_ctx *ctx, char *keys);
 
+int libxl_domain_screendump(libxl_ctx *ctx, uint32_t domid,
+const char *filename);
+
 typedef struct libxl__xen_console_reader libxl_xen_console_reader;
 
 libxl_xen_console_reader *
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 58d07cd..230da23 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1734,6 +1734,8 @@ typedef struct libxl__qmp_handler libxl__qmp_handler;
  */
 _hidden libxl__qmp_handler *libxl__qmp_initialize(libxl__gc *gc,
   uint32_t domid);
+_hidden int libxl__qmp_screendump(libxl__gc *gc, int domid,
+  const char *filename);
 /* ask to QEMU the serial port information and store it in xenstore. */
 _hidden int libxl__qmp_query_serial(libxl__qmp_handler *qmp);
 _hidden int libxl__qmp_pci_add(libxl__gc *gc, int d, libxl_device_pci *pcidev);
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index f798de7..835b43d 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -968,6 +968,14 @@ int libxl__qmp_cpu_add(libxl__gc *gc, int domid, int idx)
 return qmp_run_command(gc, domid, "cpu-add", args, NULL, NULL);
 }
 
+int libxl__qmp_screendump(libxl__gc *gc, int domid, const char *filename)
+{
+libxl__json_object *args = NULL;
+
+qmp_parameters_add_string(gc, , "filename", filename);
+return qmp_run_command(gc, domid, "screendump", args, NULL, NULL);
+}
+
 int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
const libxl_domain_config *guest_config)
 {
-- 
2.1.4


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

[Xen-devel] [OSSTEST PATCH] cs-bisection-step: Limit size of revision log included in reports

[Ian Jackson]

There is a limit in cr-daily-branch, but none in cs-bisection-step.

adhoc-revtuple-generator could usefully have this built in but that's
not so simple, so do it again here.  We already slurp the whole thing
into core so from a resource usage point of view we might as well do
the length check here too.

Reported-by: David Vrabel 
Signed-off-by: Ian Jackson 
---
 cs-bisection-step |5 +
 1 file changed, 5 insertions(+)

diff --git a/cs-bisection-step b/cs-bisection-step
index e51babd..928a147 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -898,6 +898,11 @@ END
"./adhoc-revtuple-generator -S @revtuplegenargs $rts";
 my $revinfo= `$revrune`;
 if (!$?) {
+   if (length($revinfo) > 24000) {
+   $revinfo = <

Re: [Xen-devel] pvgrub "Error 9: Unknown boot failure" booting Debian Jessie kernel (Was: Re: [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported)

[Ian Campbell]

On Mon, 2015-11-30 at 13:16 +, Ian Campbell wrote:
> On Mon, 2015-11-30 at 13:59 +0100, Juergen Gross wrote:
> > On 30/11/15 13:35, Ian Campbell wrote:
> > > FYI attempting to upgrade osstest to use Debian Jessie in the guest
> > > seems
> > > to have exposed another issue here.
> > > 
> > > http://logs.test-lab.xenproject.org/osstest/logs/65172/test-amd64-amd
> > > 64-amd64-pvgrub/info.html
> > > 
> > >   Booting 'Debian GNU/Linux, kernel 3.16.0-4-amd64'
> > > 
> > > root  (hd0,0)
> > >  Filesystem type is ext2fs, partition type 0x83
> > > kernel  /boot/vmlinuz-3.16.0-4-amd64 root=UUID=12447529-e85a-
> > > 4b41-86b6-3e83ccfc
> > > 1377 ro 
> > > initrd  /boot/initrd.img-3.16.0-4-amd64
> > > 
> > > = Init TPM Front 
> > > Tpmfront:Error Unable to read device/vtpm/0/backend-id during
> > > tpmfront initialization! error = ENOENT
> > > Tpmfront:Info Shutting down tpmfront
> > > pin_table(x) returned 1357193
> > > close(3)
> > > 
> > > Error 9: Unknown boot failure
> > > 
> > > Press any key to continue...
> > > 
> > > xen.git 6853c9bf9ff0 is OK, whereas 713b7e4ef2aa is not. Adding your
> > > two
> > > outstanding patches:
> > > libxc: correct domain builder for 64 bit guest with 32 bit tools
> > > libxc: use correct return type for do_memory_op()
> > > 
> > > Doesn't appear to have helped. Anyway, I was in the process of
> > > investigating/bisecting etc but since I was mailing you any way I
> > > thought
> > > I'd mention it. I'll start a fresh thread once I have some more to go
> > > on.
> > 
> > When something was wrong with pvgrub in my tests of the patches it died
> > right away and didn't show random errors later. I don't think the
> > problem you are seeing is related to my recent changes. OTOH I have
> > been
> > wrong before. :-(
> 
> Bisection has blamed 06954411ee14 "xen: add generic flag to elf_dom_parms
> indicating support of unmapped initrd" which I'm struggling to explain
> right now...

And trying again it has blamed ea7c8a3d0e82 "libxc: reorganize domain
builder guest memory allocator" which is far more likely.

I double checked clean rebuilds twice this time and ea7c8a3d0e82 is bad
while 6853c9bf9ff0 "MINIOS_UPSTREAM_REVISION Update" is good.

Now, the question is why...

Ian.

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

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

[Ian Campbell]

On Mon, 2015-11-30 at 13:32 +, Julien Grall wrote:
> Hi Ian,
> 
> On 25/11/15 11:37, Ian Campbell wrote:
> > On Wed, 2015-11-18 at 16:42 +, Julien Grall wrote:
> > > Xen is currently directly storing the value of GICD_ITARGETSR
> > > register
> > > (for GICv2) and GICD_IROUTER (for GICv3) in the rank. This makes the
> > > emulation of the registers access very simple but makes the code to
> > > get
> > > the target vCPU for a given vIRQ more complex.
> > > 
> > > While the target vCPU of an vIRQ is retrieved every time an vIRQ is
> > > injected to the guest, the access to the register occurs less often.
> > > 
> > > So the data structure should be optimized for the most common case
> > > rather than the inverse.
> > > 
> > > This patch introduces the usage of an array to store the target vCPU
> > > for
> > > every interrupt in the rank. This will make the code to get the
> > > target
> > > very quick. The emulation code will now have to generate the
> > > GICD_ITARGETSR
> > > and GICD_IROUTER register for read access and split it to store in a
> > > convenient way.
> > > 
> > > With the new way to store the target vCPU, the structure
> > > vgic_irq_rank
> > > is shrunk down from 320 bytes to 92 bytes. This is saving about 228
> > > bytes of memory allocated separately per vCPU.
> > > 
> > > Note that with these changes, any read to those register will list
> > > only
> > > the target vCPU used by Xen. As the spec is not clear whether this is
> > > a
> > > valid choice or not, OSes which have a different interpretation of
> > > the
> > > spec (i.e OSes which perform read-modify-write operations on these
> > > registers) may not boot anymore on Xen. Although, I think this is
> > > fair
> > > trade between memory usage in Xen (1KB less on a domain using 4 vCPUs
> > > with no SPIs) and a strict interpretation of the spec (though all the
> > > cases are not clearly defined).
> > > 
> > > Furthermore, the implementation of the callback get_target_vcpu is
> > > now
> > > exactly the same. Consolidate the implementation in the common vGIC
> > > code
> > > and drop the callback.
> > > 
> > > Finally take the opportunity to fix coding style and replace "irq" by
> > > "virq" to make clear that we are dealing with virtual IRQ in section
> > > we
> > > are modifying.
> > > 
> > > Signed-off-by: Julien Grall 
> > 
> > Acked-by: Ian Campbell 
> > 
> > I have one clarifying question, which may or may not be worth a
> > followup:
> > 
> > > + * Fetch an ITARGETSR register based on the offset from ITARGETSR0.
> > 
> > Is the offset here in terms of bytes or in terms of entire ITARGETSR
> > registers (i.e. 4 bytes)?
> 
> The offset is in term of bytes.
> 
> > Might be worth clarifying the comment?
> 
> I'm not sure, I think it's implicit with the following sentence in the
> comment:
> 
> "Note the offset will be aligned to the appropriate boundary."

It's very implicit, since without knowing the answer it's not clear what an
appropriate boundary is. 

How about: "Note the byte offset will be aligned to an ITARGETSR"
boundary" ?


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

Re: [Xen-devel] [OSSTEST PATCH] cs-bisection-step: Limit size of revision log included in reports

[Ian Campbell]

On Mon, 2015-11-30 at 13:41 +, Ian Jackson wrote:
> There is a limit in cr-daily-branch, but none in cs-bisection-step.
> 
> adhoc-revtuple-generator could usefully have this built in but that's
> not so simple, so do it again here.  We already slurp the whole thing
> into core so from a resource usage point of view we might as well do
> the length check here too.
> 
> Reported-by: David Vrabel 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 

> +(Revision log too long, ommitted.)

My MUA's spell checker thinks it is "omitted".

Ian.

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

Re: [Xen-devel] [PATCH OSSTEST] target_fetchurl: Handle undefined $c{HttpProxy}

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST] target_fetchurl: Handle undefined 
$c{HttpProxy}"):
> Avoiding a usage of a potentially undefined variable.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH OSSTEST] ts-debian-di-install: Don't set runvars for netboot kernel+ramdisk as outputs

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST] ts-debian-di-install: Don't set runvars 
for netboot kernel+ramdisk as outputs"):
> Currently these runvars are either URLs provided by the definition
> (e.g. make-flight) or output controller-relative paths created by the
> execution (in the case where they aren't from the definition).
> 
> This wierd dual-semantics is confusing and wrong, and in particular is
> broken if the test step is rerun (e.g. in standalone mode).
> 
> In the case where they are outputs only  these paths is information
> only. The information is already available in the full logs so
> dropping the runvars here merely removes the information from the
> summary table. It's not so useful that this is an issue.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCHv6] 01/28] build: import Kbuild/Kconfig from Linux 4.2

[Jan Beulich]

>>> On 24.11.15 at 18:51,  wrote:
> --- /dev/null
> +++ b/xen/include/linux/kconfig.h
> @@ -0,0 +1,54 @@
> +#ifndef __LINUX_KCONFIG_H
> +#define __LINUX_KCONFIG_H

Neither placement in the source tree nor guard variable should say
"Linux".

> --- /dev/null
> +++ b/xen/scripts/Makefile.host
> @@ -0,0 +1,128 @@

We already have ways to build host programs. Do we really need a
second mechanism, the more an abstract one? I'd prefer a more
minimalistic approach (confined to the xen/scripts/kconfig/ subtree,
which imo actually should be xen/tools/kconfig/). In any even I'd
expect, for everything outside of the actual kconfig/ subtree, a
few words in the commit message towards the rationale for including
those pieces. That'll help future cleanup by clarifying what certain
pieces are actually needed for.

> --- /dev/null
> +++ b/xen/scripts/kconfig/.gitignore
> @@ -0,0 +1,22 @@

Does the top level one (perhaps suitably adjusted) not fit the needs?

> --- /dev/null
> +++ b/xen/scripts/kconfig/Makefile.linux
> @@ -0,0 +1,317 @@

This doesn't seem to be referenced anywhere.

> --- /dev/null
> +++ b/xen/scripts/kconfig/POTFILES.in
> @@ -0,0 +1,12 @@

Do we really need this?

> +void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const 
> char *), void *data, int prevtoken)
> +{
> + if (!e) {
> + fn(data, NULL, "y");
> + return;
> + }
> +
> + if (expr_compare_type(prevtoken, e->type) > 0)
> + fn(data, NULL, "(");
> + switch (e->type) {
> + case E_SYMBOL:
> + if (e->left.sym->name)
> + fn(data, e->left.sym, e->left.sym->name);
> + else
> + fn(data, NULL, "");
> + break;
> + case E_NOT:
> + fn(data, NULL, "!");
> + expr_print(e->left.expr, fn, data, E_NOT);
> + break;
> + case E_EQUAL:
> + if (e->left.sym->name)
> + fn(data, e->left.sym, e->left.sym->name);
> + else
> + fn(data, NULL, "");
> + fn(data, NULL, "=");
> + fn(data, e->right.sym, e->right.sym->name);
> + break;
> + case E_LEQ:
> + case E_LTH:
> + if (e->left.sym->name)
> + fn(data, e->left.sym, e->left.sym->name);
> + else
> + fn(data, NULL, "");
> + fn(data, NULL, e->type == E_LEQ ? "<=" : "<");
> + fn(data, e->right.sym, e->right.sym->name);
> + break;
> + case E_GEQ:
> + case E_GTH:
> + if (e->left.sym->name)
> + fn(data, e->left.sym, e->left.sym->name);
> + else
> + fn(data, NULL, "");
> + fn(data, NULL, e->type == E_LEQ ? ">=" : ">");

Please eliminate the copy-and-paste mistake here right away (see
Linux commit f6aad2615c).

> --- /dev/null
> +++ b/xen/scripts/kconfig/gconf.c
> @@ -0,0 +1,1521 @@

I think I had asked before, and with us not wanting any user visible
prompts for now I wonder even more: Do we really need [gmnq]conf?
All I think we really need is conf.

Jan


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

Re: [Xen-devel] [PATCH] xen/arm: implement GICD_ICACTIVER read/write

[Julien Grall]

Hi Stefano,

On 25/11/15 16:40, Stefano Stabellini wrote:
> Implement GICD_ICACTIVER and GICD_ISACTIVER reads by looking for the
> GIC_IRQ_GUEST_ACTIVE bit in the relevant struct pending_irq. However
> given that the pending to active transaction for irqs in LRs in done in
> hardware, the GIC_IRQ_GUEST_ACTIVE bit might be out of date. We'll have
> to live with that.

Looking to the Linux tree again, I found this commit:

commit 1c7d4dd46ee048afe19e1294634df6fa66862519
Author: Marc Zyngier 
Date:   Mon Nov 16 19:13:28 2015 +

irqchip/gic: Add save/restore of the active state

When using EOImode==1, we may mark interrupts as being forwarded
to a virtual machine. In that case, the interrupt is left active
while being passed to the VM.

If we suspend the system before the VM has deactivated the interrupt,
the active state will be lost (which may be very annoying, as this
may result in spurious interrupts and a confused guest).

To avoid this, save and restore the active state together with the
rest of the GIC registers.

Signed-off-by: Marc Zyngier 
Cc: 
Cc: Jason Cooper 
Cc: Russell King 
Link:
http://lkml.kernel.org/r/1447701208-18150-5-git-send-email-marc.zyng...@arm.com
Signed-off-by: Thomas Gleixner 

So, we have to handle properly active/deactivate in order to get
suspend/resume working correctly.

Regards,

-- 
Julien Grall

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

[Xen-devel] [xen-4.6-testing test] 65241: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 16 
guest-localmigrate/x10 fail in 65210 REGR. vs. 63449

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-rtds 15 guest-start.2  fail in 65227 pass in 65241
 test-amd64-amd64-rumpuserxen-amd64 15 
rumpuserxen-demo-xenstorels/xenstorels.repeat fail in 65227 pass in 65241
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 13 guest-localmigrate 
fail pass in 65210
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 13 guest-localmigrate 
fail pass in 65227

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 15 guest-localmigrate.2 
fail in 65227 like 63379
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail   like 63449

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail 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   9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-i386-libvirt-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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail 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-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail 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  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-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

version targeted for testing:
 xen  78833c04250416f1870c458309d3ac0e5cf915fd
baseline version:
 xen  40d7a7454835c2f7c639c78f6c09e7b6f0e4a4e2

Last test of basis63449  2015-11-01 10:09:20 Z   29 days
Failing since 64055  2015-11-10 11:39:11 Z   20 days   18 attempts
Testing same since64935  2015-11-20 02:51:37 Z   10 days   12 attempts


People who touched revisions under test:
  Andrew Cooper 
  Ian Campbell 
  Ian Jackson 
  Jan Beulich 
  Kevin Tian 

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

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

[Julien Grall]

On 30/11/15 13:55, Ian Campbell wrote:
> On Mon, 2015-11-30 at 13:32 +, Julien Grall wrote:
>> Hi Ian,
>>
>> On 25/11/15 11:37, Ian Campbell wrote:
>>> On Wed, 2015-11-18 at 16:42 +, Julien Grall wrote:
 Xen is currently directly storing the value of GICD_ITARGETSR
 register
 (for GICv2) and GICD_IROUTER (for GICv3) in the rank. This makes the
 emulation of the registers access very simple but makes the code to
 get
 the target vCPU for a given vIRQ more complex.

 While the target vCPU of an vIRQ is retrieved every time an vIRQ is
 injected to the guest, the access to the register occurs less often.

 So the data structure should be optimized for the most common case
 rather than the inverse.

 This patch introduces the usage of an array to store the target vCPU
 for
 every interrupt in the rank. This will make the code to get the
 target
 very quick. The emulation code will now have to generate the
 GICD_ITARGETSR
 and GICD_IROUTER register for read access and split it to store in a
 convenient way.

 With the new way to store the target vCPU, the structure
 vgic_irq_rank
 is shrunk down from 320 bytes to 92 bytes. This is saving about 228
 bytes of memory allocated separately per vCPU.

 Note that with these changes, any read to those register will list
 only
 the target vCPU used by Xen. As the spec is not clear whether this is
 a
 valid choice or not, OSes which have a different interpretation of
 the
 spec (i.e OSes which perform read-modify-write operations on these
 registers) may not boot anymore on Xen. Although, I think this is
 fair
 trade between memory usage in Xen (1KB less on a domain using 4 vCPUs
 with no SPIs) and a strict interpretation of the spec (though all the
 cases are not clearly defined).

 Furthermore, the implementation of the callback get_target_vcpu is
 now
 exactly the same. Consolidate the implementation in the common vGIC
 code
 and drop the callback.

 Finally take the opportunity to fix coding style and replace "irq" by
 "virq" to make clear that we are dealing with virtual IRQ in section
 we
 are modifying.

 Signed-off-by: Julien Grall 
>>>
>>> Acked-by: Ian Campbell 
>>>
>>> I have one clarifying question, which may or may not be worth a
>>> followup:
>>>
 + * Fetch an ITARGETSR register based on the offset from ITARGETSR0.
>>>
>>> Is the offset here in terms of bytes or in terms of entire ITARGETSR
>>> registers (i.e. 4 bytes)?
>>
>> The offset is in term of bytes.
>>
>>> Might be worth clarifying the comment?
>>
>> I'm not sure, I think it's implicit with the following sentence in the
>> comment:
>>
>> "Note the offset will be aligned to the appropriate boundary."
> 
> It's very implicit, since without knowing the answer it's not clear what an
> appropriate boundary is. 
> 
> How about: "Note the byte offset will be aligned to an ITARGETSR"
> boundary" ?

Ok. I will do the same for the comment on top of vgic_*_irouter.

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [OSSTEST PATCH] cs-bisection-step: Limit size of revision log included in reports

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH] cs-bisection-step: Limit size of 
revision log included in reports"):
> Acked-by: Ian Campbell 
> 
> > +(Revision log too long, ommitted.)
> 
> My MUA's spell checker thinks it is "omitted".

Thanks.  I have ommittedd the spurrious lettter.

Ian.

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

Re: [Xen-devel] [PATCH] xen/grant-table: constify gnttab_ops structure

[David Vrabel]

On 28/11/15 14:28, Julia Lawall wrote:
> The gnttab_ops structure is never modified, so declare it as const.
> 
> Done with the help of Coccinelle.

Applied to for-linus-4.5, thanks.

David

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

Re: [Xen-devel] [PATCH 1/3] xen/gntdev: constify mmu_notifier_ops structures

[David Vrabel]

On 29/11/15 22:02, Julia Lawall wrote:
> This mmu_notifier_ops structure is never modified, so declare it as
> const, like the other mmu_notifier_ops structures.
> 
> Done with the help of Coccinelle.

Applied to for-linus-4.5, thanks.

David

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

[Xen-devel] [ovmf test] 65244: all pass - PUSHED

[osstest service owner]

flight 65244 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/65244/

Perfect :-)
All tests in this flight passed
version targeted for testing:
 ovmf 81438fe8d0fc63f9bc9fcee0113baf6bd395f29c
baseline version:
 ovmf ec613395d114ed6f7c13249a199d1e9cc0025326

Last test of basis65181  2015-11-28 08:28:52 Z2 days
Testing same since65244  2015-11-30 02:12:34 Z0 days1 attempts


People who touched revisions under test:
  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  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=ovmf
+ revision=81438fe8d0fc63f9bc9fcee0113baf6bd395f29c
+ . ./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 ovmf 
81438fe8d0fc63f9bc9fcee0113baf6bd395f29c
+ branch=ovmf
+ revision=81438fe8d0fc63f9bc9fcee0113baf6bd395f29c
+ . ./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=ovmf
+ xenbranch=xen-unstable
+ '[' xovmf = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.6-testing
+ '[' x81438fe8d0fc63f9bc9fcee0113baf6bd395f29c = 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://cache:9419/ '!=' x ']'
+++ echo 
'git://cache:9419/https://github.com/rumpkernel/rumpkernel-netbsd-src%20[fetch=try]'
++ : 

[Xen-devel] Beginner Information: Final 2015 Xen Project Document Day is Wednesday Dec 2

[Russ Pavlicek]

OUR THEME OF THE MONTH: Beginner Information

We've seen an increase of interest from potential new users recently
and we need to make sure that people can successfully get started with
4.6 using our current set of Wiki pages.  We have been slowly
replacing the "xm" references with "xl", but we could use people to
read over many of the existing pages to make sure they still make
sense with the latest release.

Pages of special interest include:

* Xen Project Beginners Guide (recently revised, but could use a read-through)
* Getting Started
* Xen Project Best Practices
* Xen Common Problems

And, of course, any other page which needs to reflect the realities of
working with 4.6.

More detailed information can be found in the TODO document (below).
And, as always, feel free to add any other documentation which you
believe to be necessary.

All the information you need to participate in Document Day is here:

http://wiki.xenproject.org/wiki/Xen_Document_Days

Also take a look at the current TODO list to see other items which
need attention:

http://wiki.xenproject.org/wiki/Xen_Document_Days/TODO

Please think about how you can help out.  If you haven't requested
to be made a Wiki editor, save time and do it now so you are ready to
go on Document Day.  Just fill out the form below:

http://xenproject.org/component/content/article/100-misc/145-request-to-be-made-a-wiki-editor.html

We hope to see you Wednesday in #xendocs!

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

Re: [Xen-devel] HVM domains crash after upgrade from XEN 4.5.1 to 4.5.2

[Jan Beulich]

>>> On 27.11.15 at 23:51,  wrote:
> Am 24.11.15 um 11:43 schrieb Jan Beulich:
>> Taking a random object out of that log, I can't see any non-standard
>> option passed to the compiler, so I have to assume this is its default
>> behavior (i.e. determined at build time, or established by extra
>> patches). Did you check the result of a random, non-trivial C file from
>> other than the Xen tree?
> Hi Jan,
> today I update a few packages like net-tools, curl, lipcre, and python 
> and none of these packages did have any strange compiler options. It 
> appeared that actually all had "-march=native -O2 -pipe 
> -fomit-frame-pointer" which is the globally defined standard on my 
> system and thus fully expected. A few obviously had package specific 
> additions (i.e. python added -fPIC -fwrapv), but nothing really strange 
> in my view.
> 
> Any more information you require or would you want to see any such build 
> log for yourself?

I definitely do not want to see any build logs; I also do not _require_
and more information: I think I've provided enough guidance for you
to investigate the odd compiler behavior at your end.

Jan


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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install fail REGR. vs. 64579
 test-amd64-i386-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
64579

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail 
REGR. vs. 64579
 test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail baseline 
untested
 test-armhf-armhf-xl-rtds 11 guest-start  fail   like 64579

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-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
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-libvirt-raw  9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   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-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-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-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-libvirt-vhd  9 debian-di-installfail   never pass

version targeted for testing:
 qemuu714487515dbe0c65d5904251e796cd3a5b3579fb
baseline version:
 qemuu9be060f5278dc0d732ebfcf2bf0a293f88b833eb

Last test of basis64579  2015-11-17 15:37:49 Z   12 days
Failing since 64797  2015-11-19 03:03:30 Z   11 days   10 attempts
Testing same since65167  2015-11-27 19:59:25 Z2 days2 attempts


People who touched revisions under test:
  "Dr. David Alan Gilbert" 
  "Eugene (jno) Dvurechenski" 
  Alberto Garcia 
  Alistair Francis 
  Andreas Färber 
  Andrew Baumann 
  Anthony PERARD 
  Bandan Das 
  Daniel P. Berrange 
  Denis V. Lunev 
  Dr. David Alan Gilbert 
  Eduardo Habkost 
  Eric Blake 
  Eugene (jno) Dvurechenski 
  Fam Zheng 
  François Baldassari 
  Gerd Hoffmann 
  Greg Kurz 
  Ildar Isaev 
  James Hogan 
  Jason J. Herne 
  Jason Wang 
  John Arbuckle 
  John Clarke 
  John Snow 
  Juan Quintela 
  Kevin Wolf 
  Leon Alrae 
  Marc-André Lureau 
  Markus Armbruster 
  Max Reitz 
  Michael Roth 
  Michael S. Tsirkin 
  Paolo Bonzini 

Re: [Xen-devel] unhandled word causes Xen crash with recent Linux kernels, was: Re: [PATCH v2 05/11] xen/arm: vgic: Properly emulate the full register

[Julien Grall]

Hi Ian,

On 25/11/15 12:26, Ian Campbell wrote:
> On Wed, 2015-11-25 at 12:15 +, Stefano Stabellini wrote:
>> On Wed, 25 Nov 2015, Shannon Zhao wrote:
>>> Upstream Linux kernel applies below patch which will write
>>> GICD_ICACTIVER. But since Xen doesn't support it, so it will cause Dom0
>>> initializes GIC failed.
>>>
>>> 0eece2b22849c90b730815c893425a36b9d10fd5 (irqchip/gic: Make sure all
>>> interrupts are deactivated at boot)
>>>
>>> (XEN) d0v0: vGICD: unhandled word write 0x to ICACTIVER4
>>> (XEN) traps.c:2447:d0v0 HSR=0x93860046 pc=0xffc0008d63f0
>>> gva=0xff804384 gpa=0x002f000384
>>> (XEN) DOM0: Unhandled fault: ttbr address size fault (0x9600) at
>>> 0xff804384
>>> (XEN) DOM0: Internal error: : 9600 [#1] PREEMPT SMP
>>> (XEN) DOM0: Modules linked in:
>>> (XEN) DOM0: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.4.0-rc2+ #364
>>> (XEN) DOM0: Hardware name: (null) (DT)
>>> (XEN) DOM0: task: ffc000969970 ti: ffc00095c000 task.ti:
>>> ffc00095c000
>>> (XEN) DOM0: PC is at gic_dist_config+0x78/0xa0
>>> (XEN) DOM0: LR is at __gic_init_bases+0x240/0x2bc
>>>
>>> Do we have a plan to fix this?
>>
>> Thanks for the reporting the issue, I can reproduce the problem.  Given
>> that this is a very serious regression and that we cannot really "fix"
>> the Linux side because Linux is not doing anything wrong, I think we
>> have to go with a very simple change, something we can easily backport
>> to all past Xen releases.
>>
>> I suggest we turn the "unhandled word write" into a write_ignore, see
>> below:
> 
> As discussed IRL this might be tolerable as a patch intended for
> backporting purposes, but I would want to see it in a series along with one
> or more not-for-backport patches which actually makes the register work as
> it should.

I have the feeling that fixing properly GICD_I*ACTIVER will take
sometimes as we also need to take into consideration hardware interrupt
routed to a guest.

As this is preventing Linux upstream to run on the latest, can we get a
simple fix for now?

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH] xen/arm: implement GICD_ICACTIVER read/write

[Julien Grall]

Hi Stefano,

On 25/11/15 16:40, Stefano Stabellini wrote:
> Implement GICD_ICACTIVER and GICD_ISACTIVER reads by looking for the
> GIC_IRQ_GUEST_ACTIVE bit in the relevant struct pending_irq. However
> given that the pending to active transaction for irqs in LRs in done in
> hardware, the GIC_IRQ_GUEST_ACTIVE bit might be out of date. We'll have
> to live with that.

Is it acceptable? The guest may rely on it to save the active state of
an IRQ.

> Implement GICD_ICACTIVER writes by checking the state of the irq in our
> queues: if the irq is present in an LR, remove the hardware ACTIVE bit.
> If the irq is present in an LR of another vcpu, send an IPI. Set the
> GIC_IRQ_GUEST_DEACTIVATE bit to tell the receiving vcpu that the active
> bit needs to be deactivated.

I would explain that the write bits of GICD_IACTIVER is left unimplemented.

> Signed-off-by: Stefano Stabellini 
> ---
>  xen/arch/arm/gic.c |   40 +++
>  xen/arch/arm/vgic-v2.c |   45 
> ++--
>  xen/arch/arm/vgic-v3.c |   44 ++-
>  xen/include/asm-arm/gic.h  |1 +
>  xen/include/asm-arm/vgic.h |4 
>  5 files changed, 123 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 1e1e5ba..75c1f52 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -414,6 +414,15 @@ static void gic_update_one_lr(struct vcpu *v, int i)
>  gic_hw_ops->read_lr(i, _val);
>  irq = lr_val.virq;
>  p = irq_to_pending(v, irq);
> +
> +if ( test_and_clear_bit(GIC_IRQ_GUEST_DEACTIVATE, >status) &&
> + (lr_val.state & GICH_LR_ACTIVE) )
> +{
> +clear_bit(GIC_IRQ_GUEST_ACTIVE, >status);
> +lr_val.state &= ~GICH_LR_ACTIVE;
> +gic_hw_ops->write_lr(i, _val);

You also have to handle the deactivation of an hardware IRQ route to a
guest.

> +}
> +
>  if ( lr_val.state & GICH_LR_ACTIVE )
>  {
>  set_bit(GIC_IRQ_GUEST_ACTIVE, >status);
> @@ -489,6 +498,37 @@ void gic_clear_lrs(struct vcpu *v)
>  spin_unlock_irqrestore(>arch.vgic.lock, flags);
>  }
>  
> +/* called with rank lock held */

Please add an ASSERT in the code to check this assumption.

> +void gic_deactivate_irq(struct vcpu *v, unsigned int irq)
> +{
> +unsigned long flags;
> +struct pending_irq *p;
> +struct vcpu *v_target = v->domain->arch.vgic.handler->get_target_vcpu(v, 
> irq);
> +
> +spin_lock_irqsave(_target->arch.vgic.lock, flags);
> +
> +p = irq_to_pending(v_target, irq);
> +/* the interrupt is not even in an LR */
> +if ( list_empty(>inflight) || !list_empty(>lr_queue) )

I think this can be simplified by testing p->lr == GIC_INVALID_LR

> +{
> +spin_unlock_irqrestore(_target->arch.vgic.lock, flags);
> +return;
> +}
> +
> +/* it is in an LR, let's check */
> +set_bit(GIC_IRQ_GUEST_DEACTIVATE, >status);
> +if ( v_target == current )
> +{
> +gic_update_one_lr(v_target, p->lr);
> +spin_unlock_irqrestore(_target->arch.vgic.lock, flags);
> +} else {
> +spin_unlock_irqrestore(_target->arch.vgic.lock, flags);
> +vcpu_unblock(v_target);

Why do you need to unblock the vCPU?

> +if (v_target->is_running )
> +smp_send_event_check_mask(cpumask_of(v_target->processor));
> +}
> +}
> +
>  static void gic_restore_pending_irqs(struct vcpu *v)
>  {
>  int lr = 0;
> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> index f7d784b..9042062 100644
> --- a/xen/arch/arm/vgic-v2.c
> +++ b/xen/arch/arm/vgic-v2.c
> @@ -126,8 +126,31 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, 
> mmio_info_t *info,
>  /* Read the active status of an IRQ via GICD is not supported */
>  case GICD_ISACTIVER ... GICD_ISACTIVERN:
>  case GICD_ICACTIVER ... GICD_ICACTIVERN:
> -goto read_as_zero;
> -
> +{
> +unsigned int i = 0, irq = 0;
> +struct pending_irq *p;
> +if ( dabt.size != DABT_WORD ) goto bad_width;
> +rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICACTIVER, DABT_WORD);
> +if ( rank == NULL) goto read_as_zero;
> +vgic_lock_rank(v, rank, flags);
> +*r = 0;
> +irq = (gicd_reg - GICD_ICACTIVER) << 3;
> +for (i = 0; i < 32; i++)
> +{
> +p = irq_to_pending(v, i + irq);
> +/*
> + * This information is likely out of date because we don't
> + * actually know which interrupts have become ACTIVE from
> + * PENDING in the LRs of other processors at it happens
> + * transparently in hardware.  We would have to interrupt
> + * all other running vcpus to get an accurate snapshot.
> + * Let's not do that.
> + */
> +*r |= test_bit(GIC_IRQ_GUEST_ACTIVE, >status) ? (1 << i) : 0;
> +}

Re: [Xen-devel] [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported

[Ian Campbell]

On Mon, 2015-11-30 at 13:20 +0100, Juergen Gross wrote:
> On 30/11/15 12:23, Ian Campbell wrote:
> > On Mon, 2015-11-30 at 12:03 +0100, Juergen Gross wrote:
> > > On 30/11/15 11:52, Ian Campbell wrote:
> > > > On Mon, 2015-11-30 at 10:51 +, Ian Campbell wrote:
> > > > > On Mon, 2015-11-30 at 11:47 +0100, Juergen Gross wrote:
> > > > > > On 30/11/15 11:34, Ian Campbell wrote:
> > > > > > > On Mon, 2015-11-30 at 11:23 +0100, Juergen Gross wrote:
> > > > > > > > On 30/11/15 11:20, Wei Liu wrote:
> > > > > > > > > On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross
> > > > > > > > > wrote:
> > > > > > > > > >  
> > > > > > > > > >  /* initrd parameters as specified in start_info
> > > > > > > > > > page
> > > > > > > > > > */
> > > > > > > > > > -unsigned long initrd_start;
> > > > > > > > > > -unsigned long initrd_len;
> > > > > > > > > > +uint64_t initrd_start;
> > > > > > > > > > +uint64_t initrd_len;
> > > > > > > > > >  
> > > > > > > > > 
> > > > > > > > > I think these should be of type xen_vaddr_t. Doesn't make
> > > > > > > > > a
> > > > > > > > > difference
> > > > > > > > > in the end though.
> > > > > > > > 
> > > > > > > > xen_vaddr_t seems not to be appropriate. It can be either a
> > > > > > > > virtual
> > > > > > > > address or a pfn.
> > > > > > > 
> > > > > > > Did you mean a virtual address or a physical _address_?
> > > > > > > Potentially
> > > > > > > mixing
> > > > > > > addresses and frame numbers in a single variable seems liable
> > > > > > > to
> > > > > > > be
> > > > > > > confusing, at best.
> > > > > > 
> > > > > > No, it's really a pfn. And this is part of the stable interface
> > > > > > between
> > > > > > hypervisor and the pv-domU since more than 5 years now.
> > > > > 
> > > > > Including the virtual address bit?
> > > > > 
> > > > > That's a shame.
> > > > 
> > > > ... and that being the case would you mind adding a comment here
> > > > explaining
> > > > the two forms of these variables and the flag which indicates which
> > > > one
> > > > is
> > > > "in force" at a given moment.
> > > 
> > > The comment in the struct already tells us that initrd_start and
> > > initrd_len are in the very same format as in the start_info page.
> > > Both fields are meant to be opaque to most of the domain builder
> > > parts.
> > > 
> > > The only function dealing with the differences is
> > > xc_dom_build_image()
> > > which already contains the appropriate flag. I added this on your
> > > request. You acked the resulting patch. So why do you want to add
> > > another comment now?
> > 
> > I hadn't realised at the time that the semantics of these fields was
> > so,
> > uh, interesting.
> 
> :-)
> 
> I guess due to the lack of a comment? ;-)

;-)

> Okay, I'll add one when submitting the patch after (hopefully) Boris
> confirmed it is fixing his problem.

Thanks!

FYI attempting to upgrade osstest to use Debian Jessie in the guest seems
to have exposed another issue here.

http://logs.test-lab.xenproject.org/osstest/logs/65172/test-amd64-amd64-amd64-pvgrub/info.html

  Booting 'Debian GNU/Linux, kernel 3.16.0-4-amd64'

root  (hd0,0)
 Filesystem type is ext2fs, partition type 0x83
kernel  /boot/vmlinuz-3.16.0-4-amd64 
root=UUID=12447529-e85a-4b41-86b6-3e83ccfc
1377 ro 
initrd  /boot/initrd.img-3.16.0-4-amd64

= Init TPM Front 
Tpmfront:Error Unable to read device/vtpm/0/backend-id during tpmfront 
initialization! error = ENOENT
Tpmfront:Info Shutting down tpmfront
pin_table(x) returned 1357193
close(3)

Error 9: Unknown boot failure

Press any key to continue...

xen.git 6853c9bf9ff0 is OK, whereas 713b7e4ef2aa is not. Adding your two
outstanding patches:
libxc: correct domain builder for 64 bit guest with 32 bit tools
libxc: use correct return type for do_memory_op()

Doesn't appear to have helped. Anyway, I was in the process of
investigating/bisecting etc but since I was mailing you any way I thought
I'd mention it. I'll start a fresh thread once I have some more to go on.

Ian.

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

Re: [Xen-devel] unhandled word causes Xen crash with recent Linux kernels, was: Re: [PATCH v2 05/11] xen/arm: vgic: Properly emulate the full register

[Ian Campbell]

On Mon, 2015-11-30 at 12:22 +, Julien Grall wrote:
> Hi Ian,
> 
> On 25/11/15 12:26, Ian Campbell wrote:
> > On Wed, 2015-11-25 at 12:15 +, Stefano Stabellini wrote:
> > > On Wed, 25 Nov 2015, Shannon Zhao wrote:
> > > > Upstream Linux kernel applies below patch which will write
> > > > GICD_ICACTIVER. But since Xen doesn't support it, so it will cause
> > > > Dom0
> > > > initializes GIC failed.
> > > > 
> > > > 0eece2b22849c90b730815c893425a36b9d10fd5 (irqchip/gic: Make sure
> > > > all
> > > > interrupts are deactivated at boot)
> > > > 
> > > > (XEN) d0v0: vGICD: unhandled word write 0x to ICACTIVER4
> > > > (XEN) traps.c:2447:d0v0 HSR=0x93860046 pc=0xffc0008d63f0
> > > > gva=0xff804384 gpa=0x002f000384
> > > > (XEN) DOM0: Unhandled fault: ttbr address size fault (0x9600)
> > > > at
> > > > 0xff804384
> > > > (XEN) DOM0: Internal error: : 9600 [#1] PREEMPT SMP
> > > > (XEN) DOM0: Modules linked in:
> > > > (XEN) DOM0: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.4.0-rc2+
> > > > #364
> > > > (XEN) DOM0: Hardware name: (null) (DT)
> > > > (XEN) DOM0: task: ffc000969970 ti: ffc00095c000 task.ti:
> > > > ffc00095c000
> > > > (XEN) DOM0: PC is at gic_dist_config+0x78/0xa0
> > > > (XEN) DOM0: LR is at __gic_init_bases+0x240/0x2bc
> > > > 
> > > > Do we have a plan to fix this?
> > > 
> > > Thanks for the reporting the issue, I can reproduce the
> > > problem.  Given
> > > that this is a very serious regression and that we cannot really
> > > "fix"
> > > the Linux side because Linux is not doing anything wrong, I think we
> > > have to go with a very simple change, something we can easily
> > > backport
> > > to all past Xen releases.
> > > 
> > > I suggest we turn the "unhandled word write" into a write_ignore, see
> > > below:
> > 
> > As discussed IRL this might be tolerable as a patch intended for
> > backporting purposes, but I would want to see it in a series along with
> > one
> > or more not-for-backport patches which actually makes the register work
> > as
> > it should.
> 
> I have the feeling that fixing properly GICD_I*ACTIVER will take
> sometimes as we also need to take into consideration hardware interrupt
> routed to a guest.
> 
> As this is preventing Linux upstream to run on the latest, can we get a
> simple fix for now?

With a suitable commit message explaining the interim/backportability
nature of this patch and the intention to do it properly I'd be willing to
accept it.

Ian.

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

Re: [Xen-devel] [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported

[Juergen Gross]

On 30/11/15 13:35, Ian Campbell wrote:
> On Mon, 2015-11-30 at 13:20 +0100, Juergen Gross wrote:
>> On 30/11/15 12:23, Ian Campbell wrote:
>>> On Mon, 2015-11-30 at 12:03 +0100, Juergen Gross wrote:
 On 30/11/15 11:52, Ian Campbell wrote:
> On Mon, 2015-11-30 at 10:51 +, Ian Campbell wrote:
>> On Mon, 2015-11-30 at 11:47 +0100, Juergen Gross wrote:
>>> On 30/11/15 11:34, Ian Campbell wrote:
 On Mon, 2015-11-30 at 11:23 +0100, Juergen Gross wrote:
> On 30/11/15 11:20, Wei Liu wrote:
>> On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross
>> wrote:
>>>  
>>>  /* initrd parameters as specified in start_info
>>> page
>>> */
>>> -unsigned long initrd_start;
>>> -unsigned long initrd_len;
>>> +uint64_t initrd_start;
>>> +uint64_t initrd_len;
>>>  
>>
>> I think these should be of type xen_vaddr_t. Doesn't make
>> a
>> difference
>> in the end though.
>
> xen_vaddr_t seems not to be appropriate. It can be either a
> virtual
> address or a pfn.

 Did you mean a virtual address or a physical _address_?
 Potentially
 mixing
 addresses and frame numbers in a single variable seems liable
 to
 be
 confusing, at best.
>>>
>>> No, it's really a pfn. And this is part of the stable interface
>>> between
>>> hypervisor and the pv-domU since more than 5 years now.
>>
>> Including the virtual address bit?
>>
>> That's a shame.
>
> ... and that being the case would you mind adding a comment here
> explaining
> the two forms of these variables and the flag which indicates which
> one
> is
> "in force" at a given moment.

 The comment in the struct already tells us that initrd_start and
 initrd_len are in the very same format as in the start_info page.
 Both fields are meant to be opaque to most of the domain builder
 parts.

 The only function dealing with the differences is
 xc_dom_build_image()
 which already contains the appropriate flag. I added this on your
 request. You acked the resulting patch. So why do you want to add
 another comment now?
>>>
>>> I hadn't realised at the time that the semantics of these fields was
>>> so,
>>> uh, interesting.
>>
>> :-)
>>
>> I guess due to the lack of a comment? ;-)
> 
> ;-)
> 
>> Okay, I'll add one when submitting the patch after (hopefully) Boris
>> confirmed it is fixing his problem.
> 
> Thanks!
> 
> FYI attempting to upgrade osstest to use Debian Jessie in the guest seems
> to have exposed another issue here.
> 
> http://logs.test-lab.xenproject.org/osstest/logs/65172/test-amd64-amd64-amd64-pvgrub/info.html
> 
>   Booting 'Debian GNU/Linux, kernel 3.16.0-4-amd64'
> 
> root  (hd0,0)
>  Filesystem type is ext2fs, partition type 0x83
> kernel  /boot/vmlinuz-3.16.0-4-amd64 
> root=UUID=12447529-e85a-4b41-86b6-3e83ccfc
> 1377 ro 
> initrd  /boot/initrd.img-3.16.0-4-amd64
> 
> = Init TPM Front 
> Tpmfront:Error Unable to read device/vtpm/0/backend-id during tpmfront 
> initialization! error = ENOENT
> Tpmfront:Info Shutting down tpmfront
> pin_table(x) returned 1357193
> close(3)
> 
> Error 9: Unknown boot failure
> 
> Press any key to continue...
> 
> xen.git 6853c9bf9ff0 is OK, whereas 713b7e4ef2aa is not. Adding your two
> outstanding patches:
> libxc: correct domain builder for 64 bit guest with 32 bit tools
> libxc: use correct return type for do_memory_op()
> 
> Doesn't appear to have helped. Anyway, I was in the process of
> investigating/bisecting etc but since I was mailing you any way I thought
> I'd mention it. I'll start a fresh thread once I have some more to go on.

When something was wrong with pvgrub in my tests of the patches it died
right away and didn't show random errors later. I don't think the
problem you are seeing is related to my recent changes. OTOH I have been
wrong before. :-(


Juergen


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

[Xen-devel] pvgrub "Error 9: Unknown boot failure" booting Debian Jessie kernel (Was: Re: [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported)

[Ian Campbell]

On Mon, 2015-11-30 at 13:59 +0100, Juergen Gross wrote:
> On 30/11/15 13:35, Ian Campbell wrote:
> > FYI attempting to upgrade osstest to use Debian Jessie in the guest
> > seems
> > to have exposed another issue here.
> > 
> > http://logs.test-lab.xenproject.org/osstest/logs/65172/test-amd64-amd64-amd64-pvgrub/info.html
> > 
> >   Booting 'Debian GNU/Linux, kernel 3.16.0-4-amd64'
> > 
> > root  (hd0,0)
> >  Filesystem type is ext2fs, partition type 0x83
> > kernel  /boot/vmlinuz-3.16.0-4-amd64 
> > root=UUID=12447529-e85a-4b41-86b6-3e83ccfc
> > 1377 ro 
> > initrd  /boot/initrd.img-3.16.0-4-amd64
> > 
> > = Init TPM Front 
> > Tpmfront:Error Unable to read device/vtpm/0/backend-id during tpmfront 
> > initialization! error = ENOENT
> > Tpmfront:Info Shutting down tpmfront
> > pin_table(x) returned 1357193
> > close(3)
> > 
> > Error 9: Unknown boot failure
> > 
> > Press any key to continue...
> > 
> > xen.git 6853c9bf9ff0 is OK, whereas 713b7e4ef2aa is not. Adding your two
> > outstanding patches:
> > libxc: correct domain builder for 64 bit guest with 32 bit tools
> > libxc: use correct return type for do_memory_op()
> > 
> > Doesn't appear to have helped. Anyway, I was in the process of
> > investigating/bisecting etc but since I was mailing you any way I thought
> > I'd mention it. I'll start a fresh thread once I have some more to go on.
> 
> When something was wrong with pvgrub in my tests of the patches it died
> right away and didn't show random errors later. I don't think the
> problem you are seeing is related to my recent changes. OTOH I have been
> wrong before. :-(

Bisection has blamed 06954411ee14 "xen: add generic flag to elf_dom_parms
indicating support of unmapped initrd" which I'm struggling to explain
right now...

Ian.

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

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

[osstest service owner]

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

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  4c6cd64519f9bc270a7278128c94e4b66e3d2077
baseline version:
 xen  b1d398b67781140d1c6efd05778d0ad4103b2a32

Last test of basis65138  2015-11-26 18:01:11 Z3 days
Testing same since65255  2015-11-30 12:02:18 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Len Brown 

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=4c6cd64519f9bc270a7278128c94e4b66e3d2077
+ . ./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 
4c6cd64519f9bc270a7278128c94e4b66e3d2077
+ branch=xen-unstable-smoke
+ revision=4c6cd64519f9bc270a7278128c94e4b66e3d2077
+ . ./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
+ '[' x4c6cd64519f9bc270a7278128c94e4b66e3d2077 = 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 

Re: [Xen-devel] [PATCH] x86/PV: hide features dependent on XSAVE when booted with "no-xsave"

[Andrew Cooper]

On 30/11/15 11:30, Jan Beulich wrote:
 On 30.11.15 at 12:10,  wrote:
>> On 30/11/15 11:08, Jan Beulich wrote:
>> On 30.11.15 at 11:46,  wrote:
 On 30/11/15 10:01, Jan Beulich wrote:
 On 27.11.15 at 16:05,  wrote:
>> On 27/11/15 11:05, Jan Beulich wrote:
>>> ... or when the guest has the XSAVE feature hidden by CPUID policy.
>>> Not doing so is at best confusing to guests.
>>>
>>> Signed-off-by: Jan Beulich 
>> These changes here are an improvement (so I don't object to taking them
>> ahead of my fullblown levelling series), but they are incomplete.
>>
>> xsaveopt, xsavec, xsetbv1, xsaves, avx and mpx depend on xsave.
>> fma, fma4, f16c, avx2 and xop depend on avx.
> I think the dependencies here are a little fuzzy, and hence I'd
> prefer us to not enforce more strict rules than are truly necessary:
>
> FMA: Neither Intel's SDM nor AMD's PM state any dependency on AVX.
>
> FMA4, XOP: AMD's PM doesn't state any dependency on AVX.
>
> AVX2: While Intel's SDM doesn't say so here either, I agree in this case.
>
> I.e. my view is that FMA{,4} and XOP are all pretty much
> independent of AVX, and they e.g. imply by themselves presence of
> YMM registers.
 The AVX feature means several things, and in this case support for VEX
 encoded instructions.
>>> Yet I don't think "VEX encoding" == "AVX". See the various VEX-
>>> encoded non-SIMD instructions.
>>>
 Per SDM Vol 2, Table 2-18, any VEX encoded instruction will
 unconditionally #UD fault if XCR0[2:1] != '11b' or CR0.OSXSAVE = 0.
>>> I.e. a dependency on XSAVE, but not on AVX.
>> XCR0[2:1] are the AVX and SSE bits.
> You mean the YMM and XMM ones (the latter one is  mis-named in
> our code base).

True - I will put it on my TODO list when making XSAVE state safe for
migration (which requires other changes in this area).

> It's not well defined whether YMM register presence
> correlates to AVX, or is simply flagged by the respective XSTATE
> CPUID bit (or a mixture of both).

It is indeed not well defined, which is what makes this area of
functionality so hard to level safely.

> The minimal (and imo more natural) dependency is just the XSTATE bit.

But it is wrong.

Any VEX encoded SIMD operation unconditionally works on YMM state.  In
the case that XMM registers are encoded with a VEX prefix, the upper 128
bits of the YMM register are zeroed (SDM Vol 2, 2.3.10).  This is
contrary to legacy SSE instructions which preserve the upper 128 bits.

Therefore, FMA, FMA4 and XOP do have a strict dependency on AVX.

~Andrew

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

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

[Julien Grall]

Hi Ian,

On 25/11/15 11:37, Ian Campbell wrote:
> On Wed, 2015-11-18 at 16:42 +, Julien Grall wrote:
>> Xen is currently directly storing the value of GICD_ITARGETSR register
>> (for GICv2) and GICD_IROUTER (for GICv3) in the rank. This makes the
>> emulation of the registers access very simple but makes the code to get
>> the target vCPU for a given vIRQ more complex.
>>
>> While the target vCPU of an vIRQ is retrieved every time an vIRQ is
>> injected to the guest, the access to the register occurs less often.
>>
>> So the data structure should be optimized for the most common case
>> rather than the inverse.
>>
>> This patch introduces the usage of an array to store the target vCPU for
>> every interrupt in the rank. This will make the code to get the target
>> very quick. The emulation code will now have to generate the
>> GICD_ITARGETSR
>> and GICD_IROUTER register for read access and split it to store in a
>> convenient way.
>>
>> With the new way to store the target vCPU, the structure vgic_irq_rank
>> is shrunk down from 320 bytes to 92 bytes. This is saving about 228
>> bytes of memory allocated separately per vCPU.
>>
>> Note that with these changes, any read to those register will list only
>> the target vCPU used by Xen. As the spec is not clear whether this is a
>> valid choice or not, OSes which have a different interpretation of the
>> spec (i.e OSes which perform read-modify-write operations on these
>> registers) may not boot anymore on Xen. Although, I think this is fair
>> trade between memory usage in Xen (1KB less on a domain using 4 vCPUs
>> with no SPIs) and a strict interpretation of the spec (though all the
>> cases are not clearly defined).
>>
>> Furthermore, the implementation of the callback get_target_vcpu is now
>> exactly the same. Consolidate the implementation in the common vGIC code
>> and drop the callback.
>>
>> Finally take the opportunity to fix coding style and replace "irq" by
>> "virq" to make clear that we are dealing with virtual IRQ in section we
>> are modifying.
>>
>> Signed-off-by: Julien Grall 
> 
> Acked-by: Ian Campbell 
> 
> I have one clarifying question, which may or may not be worth a followup:
> 
>> + * Fetch an ITARGETSR register based on the offset from ITARGETSR0.
> 
> Is the offset here in terms of bytes or in terms of entire ITARGETSR
> registers (i.e. 4 bytes)?

The offset is in term of bytes.

> Might be worth clarifying the comment?

I'm not sure, I think it's implicit with the following sentence in the
comment:

"Note the offset will be aligned to the appropriate boundary."

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH 4/4] xen/arm: p2m: Remove translation table when it's empty

[Julien Grall]

Hi Ian,

On 25/11/15 12:40, Ian Campbell wrote:
> On Wed, 2015-11-18 at 15:49 +, Julien Grall wrote:
>> Currently, the translation table is left in place even if no entries is
>> inuse. Because of how the p2m code has been implemented, replacing a
>> translation table by a block (i.e superpage) is not supported. Therefore,
>> any mapping of a superpage size will be split in smaller chunks making
>> the translation less efficient.
>>
>> Replacing a table by a block when a new mapping is added would be too
>> complicated because it requires to check if all the upper levels are not
>> inuse and free them if necessary.
>>
>> Instead, we will remove the empty translation table when the mapping are
>> removed. To avoid going through all the table checking if no entry is
>> inuse, a counter representing the number of entry currently inuse is
>> kept per table translation and updated when an entry change state (i.e
>> valid <-> invalid).
>>
>> As Xen allocates a page for each translation table, it's possible to
>> store the counter in the struct page_info.The field type_info has been
>> choosen for this purpose as the p2m owns the page and nobody should used
> 
> "chosen"
> 
>> @@ -936,6 +938,16 @@ static int apply_one_level(struct domain *d,
>>  BUG(); /* Should never get here */
>>  }
>>  
>> +static void update_reference_mapping(struct page_info *page,
>> + lpae_t old_entry,
>> + lpae_t new_entry)
>> +{
>> +if ( p2m_valid(old_entry) && !p2m_valid(new_entry) )
>> +page->u.inuse.type_info--;
>> +else if ( !p2m_valid(old_entry) && !p2m_valid(new_entry) )
>> +page->u.inuse.type_info++;
>> +}
> 
> Is there some suitable locking in place for page here?
> 
> I think the argument is that this page is part of the p2m and therefore the
> p2m lock is the thing which protects it, and we certainly hold that
> everywhere around here.

Correct. I can add a comment in the code to explain that.

> type_info is not actually a bare integer, it has some flags at the top (see
> PGT_*) which are sometimes used (e.g. share_xen_page_with_guest).
> 
> I think for consistency we should probably add a PGT_p2m and use that (and
> perhaps audit some of the other PGT_* which don't all seem to be completely
> obviously not-x86).
> 
> Presumably we would then want {get,put}_page_type to actually do something
> and to use them.
> 
> If we don't want to do that (I'm leaning towards we should, but I might be
> convinceable otherwise) then I think we should avoid the use of type_info
> as a bare counter, which would imply using another member of the inuse
> union (p2m_refcount or some such).

I think using correctly the field type_count would require lots of faff
on ARM as we don't use it for now.

So I would go with introducing a new member of inuse union.

> 
>>  if ( ret != P2M_ONE_DESCEND ) break;
>> @@ -1099,6 +1118,45 @@ static int apply_p2m_changes(struct domain *d,
>>  }
>>  /* else: next level already valid */
>>  }
>> +
>> +BUG_ON(level > 3);
>> +
>> +if ( op == REMOVE )
>> +{
>> +   for ( ; level > P2M_ROOT_LEVEL; level-- )
>> +{
> 
> Something is up with the indentation here.

Hmmm will give a look.

> 
>> +lpae_t old_entry;
>> +lpae_t *entry;
>> +unsigned int offset;
>> +
>> +pg = pages[level];
>> +
>> +/*
>> + * No need to try the previous level if the current one
>> + * still contains some mappings
>> + */
>> +if ( pg->u.inuse.type_info )
>> +break;
>> +
>> +offset = offsets[level - 1];
>> +entry = [level - 1][offset];
>> +old_entry = *entry;
>> +
>> +page_list_del(pg, >pages);
>> +
>> +p2m_remove_pte(entry, flush_pt);
> 
> This made me wonder how the existing pte clear path (which you refactored
> into this function) gets away without freeing the page, are we just leaking
> it onto the p2m->pages list?

Because the existing pte clear path is only called on the leaf of the
page tables.

The intermediate table will left linked and empty.

> p2m_put_l3_page (at the other call site) only takes care of foreign
> mappings, which aren't on that list.
> 
> Maybe there is a latent bug here? And if so perhaps the fix involves making
> p2m_remove_pte take care of various bits and bobs of the book keeping which
> is done here?
> 
>> +
>> +p2m->stats.mappings[level - 1]--;
>> +update_reference_mapping(pages[level - 1], old_entry, 
>> *entry);
>> +
>> +/*
>> + * We can't free the page now because it may be present
>> + * in the guest TLB. Queue it and free it after the TLB
>> + * has been flushed.
>> + */

Re: [Xen-devel] [PATCH 4/4] xen/arm: p2m: Remove translation table when it's empty

[Ian Campbell]

On Mon, 2015-11-30 at 14:26 +, Julien Grall wrote:
> +
> > > +p2m->stats.mappings[level - 1]--;
> > > +update_reference_mapping(pages[level - 1],
> > > old_entry, *entry);
> > > +
> > > +/*
> > > + * We can't free the page now because it may be
> > > present
> > > + * in the guest TLB. Queue it and free it after the
> > > TLB
> > > + * has been flushed.
> > > + */
> > > +page_list_add(pg, _pages);
> > 
> > You could have used page_list_move instead of del+add, but I can't
> > quite
> > convince myself it matters.
> 
> Are you sure?

No.

>  AFAICT, page_list_move move the head of the list from one
> variable to another. So all the list is moved.

I think you are probably right.

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

Re: [Xen-devel] [PATCHv6] 02/28] build: build Kconfig and config rules

[Jan Beulich]

>>> On 24.11.15 at 18:51,  wrote:
> --- a/.gitignore
> +++ b/.gitignore
> @@ -217,6 +217,11 @@ tools/xentrace/tbctl
>  tools/xentrace/xenctx
>  tools/xentrace/xentrace
>  xen/.banner
> +xen/.config
> +xen/.config.old
> +xen/defconfig
> +xen/**/*.cmd
> +xen/**/modules.order

The last two seem rather dubious to me in our environment.

> @@ -239,6 +244,9 @@ xen/include/headers++.chk
>  xen/include/asm
>  xen/include/asm-*/asm-offsets.h
>  xen/include/compat/*
> +xen/include/config.h

Linux doesn't seem to generate a file like this. Is this perhaps a
stale entry you're copying over?

> --- /dev/null
> +++ b/xen/Kconfig
> @@ -0,0 +1,26 @@
> +#
> +# For a description of the syntax of this configuration file,
> +# see Documentation/kbuild/kconfig-language.txt from the Linux

This path needs fixing.

> +# kernel source tree.
> +#
> +mainmenu "Xen/$SRCARCH $XEN_FULLVERSION Configuration"
> +
> +config SRCARCH
> + string
> + option env="SRCARCH"
> +
> +config ARCH
> + string
> + option env="ARCH"
> +
> +source "arch/$SRCARCH/Kconfig"
> +
> +config XEN_FULLVERSION
> + string
> + option env="XEN_FULLVERSION"
> +
> +config DEFCONFIG_LIST
> + string
> + option defconfig_list
> + default "$ARCH_DEFCONFIG"
> + default "arch/$SRCARCH/defconfig"

Linux'es variant has just SRCARCH and the source directive. Why
do we need so much more here? In any even I again think you
should at least briefly explain any extensions you do in the commit
message.

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -17,6 +17,9 @@ export XEN_ROOT := $(BASEDIR)/..
>  
>  EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
>  
> +# Don't break if the build process wasn't called from the top level
> +XEN_TARGET_ARCH ?= $(shell uname -m)

I don't see the need for this. We already require setting this on the
command line when invoking a build process bypassing the top level
directory. Later in the series, when actually using the produced
xen/.config, I could see this becoming a nice enhancement (by
reading the value from the file).

> @@ -216,3 +220,16 @@ FORCE:
>  
>  %/: FORCE
>   $(MAKE) -f $(BASEDIR)/Rules.mk -C $* built_in.o built_in_bin.o
> +
> +kconfig := silentoldconfig oldconfig config menuconfig defconfig \
> + nconfig xconfig gconfig savedefconfig listnewconfig olddefconfig
> +.PHONY: $(kconfig)
> +$(kconfig):
> + $(MAKE) -f $(BASEDIR)/scripts/kconfig/Makefile ARCH=$(XEN_TARGET_ARCH) 
> $@
> +
> +$(BASEDIR)/include/config/%.conf: $(BASEDIR)/include/config/auto.conf.cmd
> + $(Q)$(MAKE) -f $(BASEDIR)/scripts/kconfig/Makefile 
> ARCH=$(XEN_TARGET_ARCH) silentoldconfig

Okay, I have found the Linux original to this and it's the same there,
but I'd like to ask anyway: How can this work without $* getting
passed on?

> +# Allow people to just run `make` as before and not force them to configure
> +$(BASEDIR)/.config $(BASEDIR)/include/config/auto.conf.cmd: ;
> + $(Q)$(MAKE) -f $(BASEDIR)/scripts/kconfig/Makefile 
> ARCH=$(XEN_TARGET_ARCH) defconfig

Is this correct? If I have xen/.config but no
xen/include/config/auto.conf.cmd I surely don't want "defconfig" to
be run (unless "defconfig" honors xen/.config, in which case the
goal's name would be questionable)?

Also do you really need all the explicit $(BASEDIR) references in the
rules above?

> --- /dev/null
> +++ b/xen/arch/arm/configs/arm32_defconfig
> @@ -0,0 +1 @@
> +CONFIG_64BIT=n
> diff --git a/xen/arch/arm/configs/arm64_defconfig 
> b/xen/arch/arm/configs/arm64_defconfig
> new file mode 100644
> index 000..e69de29

As said before I'm not really up to speed with defconfigs, but the
arm64 variant being empty but the arm32 one disabling 64BIT
seems backwards: You don't even have a choice on arm32.

> --- /dev/null
> +++ b/xen/arch/x86/Kconfig
> @@ -0,0 +1,18 @@
> +config X86_64
> + def_bool y
> +
> +config X86
> + def_bool y
> + select HAS_GDBSX

Didn't you mean to convert HAS_* in subsequent patches?

Also - not 64BIT here, yet this - if added to ARM - should be added
here too so it can be used in common code.

> +config ARCH_DEFCONFIG
> + string
> + default "arch/x86/configs/x86_64_defconfig"

x86_defconfig perhaps?

> --- /dev/null
> +++ b/xen/scripts/kconfig/Makefile
> @@ -0,0 +1,77 @@
> +# xen/scripts/kconfig
> +
> +MAKEFLAGS += -rR

Why?

> +# default rule to do nothing
> +all:
> +
> +XEN_ROOT = $(CURDIR)/..
> +
> +# Xen doesn't have a silent build flag
> +quiet := silent_
> +Q := @
> +kecho := :
> +
> +# eventually you'll want to do out of tree builds
> +srctree = $(XEN_ROOT)/xen
> +objtree = $(srctree)
> +src := scripts/kconfig
> +obj := $(src)
> +KBUILD_SRC :=
> +
> +# handle functions (most of these lifted from different Linux makefiles
> +dot-target = $(dir $@).$(notdir $@)
> +depfile = $(subst $(comma),,$(dot-target).d)
> +basetarget = $(basename $(notdir $@))
> +cmd = $(cmd_$(1))
> +if_changed = $(if y, \
> + @set -e; \
> + $(cmd_$(1)); \
> + )
> +
> +if_changed_dep = $(if y, \
> + 

Re: [Xen-devel] [PATCHv6] 03/28] build: use generated Kconfig options for Xen

[Jan Beulich]

>>> On 24.11.15 at 18:51,  wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -26,6 +26,9 @@ default: build
>  .PHONY: dist
>  dist: install
>  
> +.PHONY: build
> +build:: $(BASEDIR)/include/config/auto.conf
> +
>  .PHONY: build install uninstall clean distclean cscope TAGS tags MAP gtags

I do not see why you need to add build to PHONY's dependencies
another time.

> @@ -227,9 +230,14 @@ kconfig := silentoldconfig oldconfig config menuconfig 
> defconfig \
>  $(kconfig):
>   $(MAKE) -f $(BASEDIR)/scripts/kconfig/Makefile ARCH=$(XEN_TARGET_ARCH) 
> $@
>  
> -$(BASEDIR)/include/config/%.conf: $(BASEDIR)/include/config/auto.conf.cmd
> +$(BASEDIR)/include/config/%.conf: $(BASEDIR)/include/config/auto.conf.cmd 
> $(BASEDIR)/.config
>   $(Q)$(MAKE) -f $(BASEDIR)/scripts/kconfig/Makefile 
> ARCH=$(XEN_TARGET_ARCH) silentoldconfig
>  
>  # Allow people to just run `make` as before and not force them to configure
> -$(BASEDIR)/.config $(BASEDIR)/include/config/auto.conf.cmd: ;
> +$(BASEDIR)/.config:
>   $(Q)$(MAKE) -f $(BASEDIR)/scripts/kconfig/Makefile 
> ARCH=$(XEN_TARGET_ARCH) defconfig

This should be one of the oldconfig targets now, shouldn't it?

> +# Break the dependency chain for the first run
> +$(BASEDIR)/include/config/auto.conf.cmd: ;
> +
> +-include $(BASEDIR)/include/config/auto.conf.cmd

The comment is quite a bit different in Linux, and seems to make more
sense. Also note how Linux has an empty rule for $(KCONFIG_CONFIG),
a variable which iirc you defined in an earlier patch and hence perhaps
you should be using here.

> --- a/xen/include/xen/config.h
> +++ b/xen/include/xen/config.h
> @@ -12,6 +12,8 @@
>  #endif
>  #include 
>  
> +#include 

First thing perhaps?

Jan


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

Re: [Xen-devel] [PATCH v3 07/62] arm/acpi: Add arch_acpi_os_map_memory helper function for ARM

[Julien Grall]

Hi,

On 23/11/15 11:37, Stefano Stabellini wrote:
> On Tue, 17 Nov 2015, shannon.z...@linaro.org wrote:
>> From: Shannon Zhao 

> could you please add a couple of lines to the commit message mentioning
> why __va(phys) is an acceptable implementation of arch_acpi_os_map_memory?

FWIW, I already asked this question multiple time on the previous series
without clear answer.

__va should only be used when the memory is direct-mapped to Xen (i.e
accessible directly). On ARM64, this is only the case for the RAM. Can
someone confirm the ACPI will always reside to the RAM?

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCHv6] 04/28] build: convert HAS_PASSTHROUGH use to Kconfig

[Jan Beulich]

>>> On 24.11.15 at 18:51,  wrote:
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -3,6 +3,7 @@ config X86_64
>  
>  config X86
>   def_bool y
> + select HAS_PASSTHROUGH
>   select HAS_GDBSX

Please get these sorted alphabetically from the beginning.

> --- /dev/null
> +++ b/xen/drivers/passthrough/Kconfig
> @@ -0,0 +1,4 @@
> +
> +# Select HAS_PASSTHROUGH if PCI pass through is supported
> +config HAS_PASSTHROUGH
> + bool

s/PCI/device/ in the comment please.

> @@ -348,9 +348,9 @@ static XSM_INLINE int xsm_deassign_device(XSM_DEFAULT_ARG 
> struct domain *d, uint
>  return xsm_default_action(action, current->domain, d);
>  }
>  
> -#endif /* HAS_PASSTHROUGH && HAS_PCI */
> +#endif /* CONFIG_HAS_PASSTHROUGH && HAS_PCI */

I don't think fiddling with trailing comments like this one is really
useful - the connection is clear also without CONFIG_ prefix.

Jan


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