Re: [Xen-devel] [PATCH 3/3] Significant changes to decision making; some new roles and minor changes

[Tim Deegan]

Hi,

At 14:55 + on 15 Aug (1471272946), Lars Kurth wrote:
> But I see your point. The text should really have said something like...
> -
> In situations where the entire Xen Project community becomes paralysed,
> the project leaderships team or project lead should work with the
> community 
> manager or advisory board to find a way forward.
> -

Sure.  I think that's good.

> I think we have two options:
> A) A delete this bullet entirely
> B) Replace it with something clearer - even though, the location
> for such a paragraph is wrong.
> 
> My gut feel is to just go for A.

Sounds good to me.

Cheers,

Tim.

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

[Xen-devel] [qemu-mainline baseline-only test] 67537: tolerable FAIL

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 67537 qemu-mainline real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/67537/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail like 66974
 test-amd64-amd64-amd64-pvgrub 10 guest-start  fail  like 66974

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

version targeted for testing:
 qemuu60ac136102098a70b97ab0c07bc7bf53131c
baseline version:
 qemuu28b874429ba16e71e0caa46453f3a3e31efb3c51

Last test of basis66974  2016-08-12 09:50:38 Z3 days
Testing same since67537  2016-08-15 17:14:39 Z0 days1 attempts


People who touched revisions under test:
  Peter Maydell 
  Pranith Kumar 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass
 

Re: [Xen-devel] [RFC v3 00/13] linux: generalize sections, ranges and linker tables

[Alan Cox]

> This is the module tag ... it says what licence the module is under,
> not the licence for the module combined with the kernel, which is
> always GPLv2 because the stricter licence rules.

Because if I build a BSD licensed module against the kernel, give you
the binaries and refuse to give you the source I am conforming to the
BSD licence in letter. So to use it with the kernel it needs to be GPL
with additional rights (eg BSD including the source...)

Alan
##SELECTION_END##___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [xen-4.5-testing test] 100496: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qcow2 9 debian-di-installfail REGR. vs. 100338
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 15 guest-localmigrate/x10 fail REGR. 
vs. 100338
 test-amd64-i386-xl-qemuu-winxpsp3 15 guest-localmigrate/x10 fail REGR. vs. 
100338

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 11 guest-start  fail REGR. vs. 100338
 test-amd64-amd64-xl-qemuu-winxpsp3 13 guest-localmigrate  fail like 100308
 test-amd64-amd64-xl-rtds  6 xen-boot fail  like 100338
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 100338
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 100338
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 100338
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 100338

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

version targeted for testing:
 xen  2ad058efbbe42035784d8b32b53e7708b05cf94c
baseline version:
 xen  08313b45bfc75fa4cbadb9d25a0561e5f5b2fee6

Last test of basis   100338  2016-08-08 08:24:15 Z7 days
Testing same since   100496  2016-08-15 12:13:37 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Anil Madhavapeddy 
  Anthony PERARD 
  Bob Liu 
  Boris Ostrovsky 
  Daniel De Graaf 
  George Dunlap 
  Ian Jackson 
  Jan Beulich 
  Jason Andryuk 
  Juergen Gross 
  Konrad Rzeszutek Wilk 
  Marek Marczykowski-Górecki 
  Matthew Daley 
  Olaf Hering 
  Roger Pau Monne 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-prev pass
 build-i386-prev  pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 build-amd64-rumpuserxen  pass
 build-i386-rumpuserxen   pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  pass
 

[Xen-devel] [PATCH] arm64: head: Fill image size

[Peng Fan]

When booting xen from U-Boot, U-Boot will use the image size
info. Because this information is lacked in XEN image,U-Boot
assume the image size is 16MB to memmove, which will cost lots
time on simulation platform.

Signed-off-by: Peng Fan 
Cc: Stefano Stabellini 
Cc: Julien Grall 
---
 xen/arch/arm/arm64/head.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 91e2817..9fade07 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -115,7 +115,7 @@ efi_head:
 add x13, x18, #0x16
 b   real_start   /* branch to kernel start */
 .quad   0/* Image load offset from start of RAM */
-.quad   0/* reserved */
+.quad   _end - start /* Effective size of Image, little-endian 
*/
 .quad   0/* reserved */
 .quad   0/* reserved */
 .quad   0/* reserved */
-- 
2.6.2


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

Re: [Xen-devel] [PATCH v2 23/23] libxc/xc_dom_core: Copy ACPI tables to guest space

[Shannon Zhao]

On 2016/8/15 20:49, Boris Ostrovsky wrote:
> On 08/15/2016 03:48 AM, Shannon Zhao wrote:
>> Hi Boris
>>
>> On 2016/8/5 5:06, Boris Ostrovsky wrote:
>>> Load ACPI modules into guest space
>>>
>>> Signed-off-by: Boris Ostrovsky 
>>> ---
>>> v2:
>>> * New patch, loosely based on Shannon's ARM patch
>>>
>>>  tools/libxc/xc_dom_core.c | 92 
>>> +++
>>>  1 file changed, 92 insertions(+)
>>>
>>> diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
>>> index ebada89..00d870f 100644
>>> --- a/tools/libxc/xc_dom_core.c
>>> +++ b/tools/libxc/xc_dom_core.c
>>> @@ -1040,6 +1040,94 @@ static int xc_dom_build_ramdisk(struct xc_dom_image 
>>> *dom)
>>>  return -1;
>>>  }
>>>  
>>> +static int populate_acpi_pages(struct xc_dom_image *dom,
>>> +   xen_pfn_t *extents,
>>> +   unsigned int num_pages)
>>> +{
>>> +int rc;
>>> +xc_interface *xch = dom->xch;
>>> +uint32_t domid = dom->guest_domid;
>>> +unsigned long idx, first_high_idx = (1ull << (32 - 12));
>>> +
>>> +for ( ; num_pages; num_pages--, extents++ )
>>> +{
>>> +
>>> +if ( xc_domain_populate_physmap(xch, domid, 1, 0, 0, extents) == 1 
>>> )
>>> +continue;
>>> +
>>> +if (dom->highmem_end)
>>> +{
>>> +idx = --dom->highmem_end;
>>> +if ( idx == first_high_idx )
>>> +dom->highmem_end = 0;
>>> +}
>>> +else
>>> +idx = --dom->lowmem_end;
>>> +
>>> +rc = xc_domain_add_to_physmap(xch, domid,
>>> +  XENMAPSPACE_gmfn,
>>> +  idx, *extents);
>>> +if (rc)
>>> +return rc;
>>> +}
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +static int xc_dom_load_acpi(struct xc_dom_image *dom)
>>> +{
>>> +int j, i = 0;
>>> +unsigned num_pages;
>>> +xen_pfn_t *extents, base;
>>> +void *ptr;
>>> +
>>> +while ( (i < MAX_ACPI_MODULES) && dom->acpi_modules[i].length )
>>> +{
>>> +DOMPRINTF("%s: %d bytes at address %lx\n", __FUNCTION__,
>>> +  dom->acpi_modules[i].length,
>>> +  dom->acpi_modules[i].guest_addr_out);
>>> +
>>> +num_pages = (dom->acpi_modules[i].length + (XC_PAGE_SIZE - 1)) >>
>>> +   XC_PAGE_SHIFT;
>>> +extents = malloc(num_pages * sizeof(*extents));
>>> +if ( !extents )
>>> +{
>>> +DOMPRINTF("%s: Out of memory", __FUNCTION__);
>>> +goto err;
>>> +}
>>> +
>>> +base = dom->acpi_modules[i].guest_addr_out >> XC_PAGE_SHIFT;
>>> +for (j=0; j>> +extents[j] = base + j;
>>> +if ( populate_acpi_pages(dom, extents, num_pages) )
>>> +{
>>> +DOMPRINTF("%s: Can populate ACPI pages", __FUNCTION__);
>>> +goto err;
>>> +}
>>> +
>>> +ptr = xc_map_foreign_range(dom->xch, dom->guest_domid,
>>> +  XC_PAGE_SIZE * num_pages,
>>> +  PROT_READ | PROT_WRITE, base);
>>> +if ( !ptr )
>>> +{
>>> +DOMPRINTF("%s: Can't map %d pages at 0x%lx",
>>> +  __FUNCTION__, num_pages, base);
>>> +goto err;
>>> +}
>>> +
>>> +memcpy(ptr, dom->acpi_modules[i].data, 
>>> dom->acpi_modules[i].length);
>>> +
>>> +free(extents);
>>> +i++;
>>> +}
>>> +
>>> +return 0;
>>> +
>>> +err:
>>> +free(extents);
>>> +return -1;
>>> +}
>>> +
>>>  int xc_dom_build_image(struct xc_dom_image *dom)
>>>  {
>>>  unsigned int page_size;
>>> @@ -1097,6 +1185,10 @@ int xc_dom_build_image(struct xc_dom_image *dom)
>>>  memcpy(devicetreemap, dom->devicetree_blob, dom->devicetree_size);
>>>  }
>>>  
>>> +/* load ACPI tables */
>>> +if ( xc_dom_load_acpi(dom) != 0 )
>>> +goto err;
>> I think it needs to move the definition of xc_dom_load_acpi() to
>> xc_dom_x86.c while I will move the corresponding one of ARM to xc_dom_arm.c.
> 
> 
> You don't think both x86 and ARM can use the same definition?  This
> looks very similar to what you had in your series.
> 
So I have to use dom->acpi_modules[i] in my series instead of
dom->acpitable_blob. I think it's fine to reuse what x86 uses.

Thanks,
-- 
Shannon


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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-xsm   6 xen-boot fail REGR. vs. 100352

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 100352
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 100352
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 100352
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 100352
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeatfail  like 100352

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

version targeted for testing:
 xen  e06d2bae53cb1a3542e7269fd35bf3885dd2e244
baseline version:
 xen  55292d3dee83c974e3d89d3a24cd35a8956ceaf5

Last test of basis   100352  2016-08-08 19:46:24 Z7 days
Testing same since   100495  2016-08-15 11:13:56 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Anil Madhavapeddy 
  Anthony PERARD 
  Bob Liu 
  Boris Ostrovsky 
  Daniel De Graaf 
  George Dunlap 
  Ian Jackson 
  Jan Beulich 
  Jason Andryuk 
  Juergen Gross 
  Konrad Rzeszutek Wilk 
  Marek Marczykowski-Górecki 
  Matthew Daley 
  Olaf Hering 
  Roger Pau Monne 
  Wei Liu 

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

Re: [Xen-devel] [RFC 05/22] xen/arm: traps: Move MMIO emulation code in a separate helper

[Stefano Stabellini]

On Thu, 28 Jul 2016, Julien Grall wrote:
> Currently, a stage-2 fault translation will likely access an emulated
> region. All the checks are pre-sanitity check for MMIO emulation.
> 
> A follow-up patch will handle a new case that could lead to a stage-2
> translation. To improve the clarity of the code and the changes, the
> current implementation is move in a separate helper.
> 
> Signed-off-by: Julien Grall 
> ---
>  xen/arch/arm/traps.c | 58 
> ++--
>  1 file changed, 33 insertions(+), 25 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 46e0663..b46284c 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2444,6 +2444,38 @@ static void do_trap_instr_abort_guest(struct 
> cpu_user_regs *regs,
>  inject_iabt_exception(regs, gva, hsr.len);
>  }
>  
> +static bool_t try_handle_mmio(struct cpu_user_regs *regs,
> +  mmio_info_t *info)
> +{
> +const struct hsr_dabt dabt = info->dabt;
> +int rc;
> +
> +/* stage-1 page table should never live in an emulated MMIO region */
> +if ( dabt.s1ptw )
> +return 0;
> +
> +/* All the instructions used on emulated MMIO region should be valid */
> +if ( !dabt.valid )
> +return 0;
> +
> +/*
> + * Erratum 766422: Thumb store translation fault to Hypervisor may
> + * not have correct HSR Rt value.
> + */
> +if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) &&
> + dabt.write )
> +{
> +rc = decode_instruction(regs, >dabt);
> +if ( rc )
> +{
> +gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
> +return 0;
> +}
> +}
> +
> +return !!handle_mmio(info);
> +}
> +
>  static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
>   const union hsr hsr)
>  {
> @@ -2487,40 +2519,16 @@ static void do_trap_data_abort_guest(struct 
> cpu_user_regs *regs,
>  break;
>  }
>  case FSC_FLT_TRANS:
> -if ( dabt.s1ptw )
> -goto bad_data_abort;
> -
> -/* XXX: Decode the instruction if ISS is not valid */
> -if ( !dabt.valid )
> -goto bad_data_abort;
> -
> -/*
> - * Erratum 766422: Thumb store translation fault to Hypervisor may
> - * not have correct HSR Rt value.
> - */
> -if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) &&
> - dabt.write )
> -{
> -rc = decode_instruction(regs, );
> -if ( rc )
> -{
> -gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
> -goto bad_data_abort;
> -}
> -}
> -
> -if ( handle_mmio() )
> +if ( try_handle_mmio(regs, ) )
>  {
>  advance_pc(regs, hsr);
>  return;
>  }
> -break;

I would keep this break, because we don't want to print the warning
below in all error cases, such as !dabt.valid.

Aside from the break removal:

Reviewed-by: Stefano Stabellini 


>  default:
>  gprintk(XENLOG_WARNING, "Unsupported DFSC: HSR=%#x DFSC=%#x\n",
>  hsr.bits, dabt.dfsc);
>  }
>  
> -bad_data_abort:
>  gdprintk(XENLOG_DEBUG, "HSR=0x%x pc=%#"PRIregister" gva=%#"PRIvaddr
>   " gpa=%#"PRIpaddr"\n", hsr.bits, regs->pc, info.gva, info.gpa);
>  inject_dabt_exception(regs, info.gva, hsr.len);
> -- 
> 1.9.1
> 

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

Re: [Xen-devel] [RFC 04/22] xen/arm: p2m: Use typesafe gfn in p2m_mem_access_radix_set

[Stefano Stabellini]

On Thu, 28 Jul 2016, Julien Grall wrote:
> p2m_mem_access_radix_set is expecting a gfn in a parameter. Rename the
> parameter 'pfn' to 'gfn' to match its content and use the typesafe gfn
> to avoid possible misusage.
> 
> Also rename the parameter to gfn to match its content.
> 
> Signed-off-by: Julien Grall 

Reviewed-by: Stefano Stabellini 


>  xen/arch/arm/p2m.c | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index ff82f12..aecdd1e 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -542,7 +542,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t 
> gfn,
>  return 0;
>  }
>  
> -static int p2m_mem_access_radix_set(struct p2m_domain *p2m, unsigned long 
> pfn,
> +static int p2m_mem_access_radix_set(struct p2m_domain *p2m, gfn_t gfn,
>  p2m_access_t a)
>  {
>  int rc;
> @@ -552,18 +552,18 @@ static int p2m_mem_access_radix_set(struct p2m_domain 
> *p2m, unsigned long pfn,
>  
>  if ( p2m_access_rwx == a )
>  {
> -radix_tree_delete(>mem_access_settings, pfn);
> +radix_tree_delete(>mem_access_settings, gfn_x(gfn));
>  return 0;
>  }
>  
> -rc = radix_tree_insert(>mem_access_settings, pfn,
> +rc = radix_tree_insert(>mem_access_settings, gfn_x(gfn),
> radix_tree_int_to_ptr(a));
>  if ( rc == -EEXIST )
>  {
>  /* If a setting already exists, change it to the new one */
>  radix_tree_replace_slot(
>  radix_tree_lookup_slot(
> ->mem_access_settings, pfn),
> +>mem_access_settings, gfn_x(gfn)),
>  radix_tree_int_to_ptr(a));
>  rc = 0;
>  }
> @@ -707,7 +707,7 @@ static int apply_one_level(struct domain *d,
>  */
>   (level == 3 || (!p2m_table(orig_pte) && 
> !p2m->mem_access_enabled)) )
>  {
> -rc = p2m_mem_access_radix_set(p2m, paddr_to_pfn(*addr), a);
> +rc = p2m_mem_access_radix_set(p2m, _gfn(paddr_to_pfn(*addr)), a);
>  if ( rc < 0 )
>  return rc;
>  
> @@ -825,7 +825,8 @@ static int apply_one_level(struct domain *d,
>  *flush = true;
>  
>  p2m_remove_pte(entry, p2m->clean_pte);
> -p2m_mem_access_radix_set(p2m, paddr_to_pfn(*addr), p2m_access_rwx);
> +p2m_mem_access_radix_set(p2m, _gfn(paddr_to_pfn(*addr)),
> + p2m_access_rwx);
>  
>  *addr += level_size;
>  *maddr += level_size;
> @@ -896,7 +897,8 @@ static int apply_one_level(struct domain *d,
>  
>  if ( p2m_valid(pte) )
>  {
> -rc = p2m_mem_access_radix_set(p2m, paddr_to_pfn(*addr), a);
> +rc = p2m_mem_access_radix_set(p2m, _gfn(paddr_to_pfn(*addr)),
> +  a);
>  if ( rc < 0 )
>  return rc;
>  
> -- 
> 1.9.1
> 

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

Re: [Xen-devel] [RFC 03/22] xen/arm: p2m: Rename parameter in p2m_{remove, write}_pte...

[Stefano Stabellini]

On Thu, 28 Jul 2016, Julien Grall wrote:
> to make clear of the usage. I.e it is used to inform whether Xen needs
> to clean the entry after writing in the page table.
> 
> Signed-off-by: Julien Grall 

Acked-by: Stefano Stabellini 


>  xen/arch/arm/p2m.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index d389f2b..ff82f12 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -390,19 +390,19 @@ static lpae_t mfn_to_p2m_entry(mfn_t mfn, p2m_type_t t, 
> p2m_access_t a)
>  return e;
>  }
>  
> -static inline void p2m_write_pte(lpae_t *p, lpae_t pte, bool_t flush_cache)
> +static inline void p2m_write_pte(lpae_t *p, lpae_t pte, bool clean_pte)
>  {
>  write_pte(p, pte);
> -if ( flush_cache )
> +if ( clean_pte )
>  clean_dcache(*p);
>  }
>  
> -static inline void p2m_remove_pte(lpae_t *p, bool_t flush_cache)
> +static inline void p2m_remove_pte(lpae_t *p, bool clean_pte)
>  {
>  lpae_t pte;
>  
>  memset(, 0x00, sizeof(pte));
> -p2m_write_pte(p, pte, flush_cache);
> +p2m_write_pte(p, pte, clean_pte);
>  }
>  
>  /*
> -- 
> 1.9.1
> 

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

Re: [Xen-devel] [RFC 02/22] xen/arm: p2m: Store in p2m_domain whether we need to clean the entry

[Stefano Stabellini]

On Thu, 28 Jul 2016, Julien Grall wrote:
> Each entry in the page table has to table clean when the IOMMU does not

What does it mean to "table clean" ?


> support coherent walk. Rather than querying every time the page table is
> updated, it is possible to do it only once when the p2m is initialized.
> 
> This is because this value can never change, Xen would be in big trouble
> otherwise.
> 
> With this change, the initialize of the IOMMU for a given domain has to

"the initialization"


> be done earlier in order to know whether the page table entries need to
> be clean. It is fine to move the call earlier because it has no

"be cleaned"


Aside from the small imperfections in the commit message:

Reviewed-by: Stefano Stabellini 


> dependency.
> 
> Signed-off-by: Julien Grall 
> ---
>  xen/arch/arm/domain.c |  8 +---
>  xen/arch/arm/p2m.c| 47 
> ++-
>  xen/include/asm-arm/p2m.h |  3 +++
>  3 files changed, 30 insertions(+), 28 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 20bb2ba..48f04c8 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -555,6 +555,11 @@ int arch_domain_create(struct domain *d, unsigned int 
> domcr_flags,
>  return 0;
>  
>  ASSERT(config != NULL);
> +
> +/* p2m_init relies on some value initialized by the IOMMU subsystem */
> +if ( (rc = iommu_domain_init(d)) != 0 )
> +goto fail;
> +
>  if ( (rc = p2m_init(d)) != 0 )
>  goto fail;
>  
> @@ -637,9 +642,6 @@ int arch_domain_create(struct domain *d, unsigned int 
> domcr_flags,
>  if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )
>  goto fail;
>  
> -if ( (rc = iommu_domain_init(d)) != 0 )
> -goto fail;
> -
>  return 0;
>  
>  fail:
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 40a0b80..d389f2b 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -416,7 +416,7 @@ static inline void p2m_remove_pte(lpae_t *p, bool_t 
> flush_cache)
>   * level_shift is the number of bits at the level we want to create.
>   */
>  static int p2m_create_table(struct p2m_domain *p2m, lpae_t *entry,
> -int level_shift, bool_t flush_cache)
> +int level_shift)
>  {
>  struct page_info *page;
>  lpae_t *p;
> @@ -462,7 +462,7 @@ static int p2m_create_table(struct p2m_domain *p2m, 
> lpae_t *entry,
>  else
>  clear_page(p);
>  
> -if ( flush_cache )
> +if ( p2m->clean_pte )
>  clean_dcache_va_range(p, PAGE_SIZE);
>  
>  unmap_domain_page(p);
> @@ -470,7 +470,7 @@ static int p2m_create_table(struct p2m_domain *p2m, 
> lpae_t *entry,
>  pte = mfn_to_p2m_entry(_mfn(page_to_mfn(page)), p2m_invalid,
> p2m->default_access);
>  
> -p2m_write_pte(entry, pte, flush_cache);
> +p2m_write_pte(entry, pte, p2m->clean_pte);
>  
>  return 0;
>  }
> @@ -653,12 +653,10 @@ static const paddr_t level_shifts[] =
>  
>  static int p2m_shatter_page(struct p2m_domain *p2m,
>  lpae_t *entry,
> -unsigned int level,
> -bool_t flush_cache)
> +unsigned int level)
>  {
>  const paddr_t level_shift = level_shifts[level];
> -int rc = p2m_create_table(p2m, entry,
> -  level_shift - PAGE_SHIFT, flush_cache);
> +int rc = p2m_create_table(p2m, entry, level_shift - PAGE_SHIFT);
>  
>  if ( !rc )
>  {
> @@ -680,7 +678,6 @@ static int p2m_shatter_page(struct p2m_domain *p2m,
>  static int apply_one_level(struct domain *d,
> lpae_t *entry,
> unsigned int level,
> -   bool_t flush_cache,
> enum p2m_operation op,
> paddr_t start_gpaddr,
> paddr_t end_gpaddr,
> @@ -719,7 +716,7 @@ static int apply_one_level(struct domain *d,
>  if ( level < 3 )
>  pte.p2m.table = 0; /* Superpage entry */
>  
> -p2m_write_pte(entry, pte, flush_cache);
> +p2m_write_pte(entry, pte, p2m->clean_pte);
>  
>  *flush |= p2m_valid(orig_pte);
>  
> @@ -754,7 +751,7 @@ static int apply_one_level(struct domain *d,
>  /* Not present -> create table entry and descend */
>  if ( !p2m_valid(orig_pte) )
>  {
> -rc = p2m_create_table(p2m, entry, 0, flush_cache);
> +rc = p2m_create_table(p2m, entry, 0);
>  if ( rc < 0 )
>  return rc;
>  return P2M_ONE_DESCEND;
> @@ -764,7 +761,7 @@ static int apply_one_level(struct domain *d,
>  if ( p2m_mapping(orig_pte) )
>  {
>  *flush = true;
> -

Re: [Xen-devel] [RFC 01/22] xen/arm: do_trap_instr_abort_guest: Move the IPA computation out of the switch

[Stefano Stabellini]

On Thu, 28 Jul 2016, Julien Grall wrote:
> A follow-up patch will add more case to the switch that will require the
> IPA. So move the computation out of the switch.
> 
> Signed-off-by: Julien Grall 
> ---
>  xen/arch/arm/traps.c | 36 ++--
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 683bcb2..46e0663 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2403,35 +2403,35 @@ static void do_trap_instr_abort_guest(struct 
> cpu_user_regs *regs,
>  int rc;
>  register_t gva = READ_SYSREG(FAR_EL2);
>  uint8_t fsc = hsr.iabt.ifsc & ~FSC_LL_MASK;
> +paddr_t gpa;
> +
> +if ( hpfar_is_valid(hsr.iabt.s1ptw, fsc) )
> +gpa = get_faulting_ipa(gva);
> +else
> +{
> +/*
> + * Flush the TLB to make sure the DTLB is clear before
> + * doing GVA->IPA translation. If we got here because of
> + * an entry only present in the ITLB, this translation may
> + * still be inaccurate.
> + */
> +flush_tlb_local();
> +
> +rc = gva_to_ipa(gva, , GV2M_READ);
> +if ( rc == -EFAULT )
> +return; /* Try again */

The issue with this is that now for any cases that don't require a gpa
if gva_to_ipa fails we wrongly return -EFAULT.

I suggest having two switches or falling through from the first case to
the second.


> +}
>  
>  switch ( fsc )
>  {
>  case FSC_FLT_PERM:
>  {
> -paddr_t gpa;
>  const struct npfec npfec = {
>  .insn_fetch = 1,
>  .gla_valid = 1,
>  .kind = hsr.iabt.s1ptw ? npfec_kind_in_gpt : npfec_kind_with_gla
>  };
>  
> -if ( hpfar_is_valid(hsr.iabt.s1ptw, fsc) )
> -gpa = get_faulting_ipa(gva);
> -else
> -{
> -/*
> - * Flush the TLB to make sure the DTLB is clear before
> - * doing GVA->IPA translation. If we got here because of
> - * an entry only present in the ITLB, this translation may
> - * still be inaccurate.
> - */
> -flush_tlb_local();
> -
> -rc = gva_to_ipa(gva, , GV2M_READ);
> -if ( rc == -EFAULT )
> -return; /* Try again */
> -}
> -
>  rc = p2m_mem_access_check(gpa, gva, npfec);
>  
>  /* Trap was triggered by mem_access, work here is done */
> -- 
> 1.9.1
> 

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

Re: [Xen-devel] [RFC v3 00/13] linux: generalize sections, ranges and linker tables

[James Bottomley]

On Tue, 2016-08-09 at 21:51 -0700, Andy Lutomirski wrote:
> On Aug 9, 2016 7:09 PM, "James Bottomley" <
> james.bottom...@hansenpartnership.com> wrote:
> > 
> > On Tue, 2016-08-09 at 15:24 +0100, One Thousand Gnomes wrote:
> > > > table development go under copyleft-next, Rusty recently asked 
> > > > for code to go in prior to the license tag being added denoting
> > > > this license as GPL-compatible [3] -- I had noted in the patch
> > > > submission which annotated copyleft-next's compatibility to 
> > > > GPLv2 that copyleft-next is the license of choice for ongoing 
> > > > kernel development on my end [4]. If this is objectionable I'm 
> > > > happy to change it to GPLv2 however I'd like a reason provided 
> > > > as I've gone through all possible channels to ensure this is 
> > > > kosher, including vetting by 3 attorneys now, 2 at SUSE.
> > > 
> > > You don't need a new tag, you can use "GPL" or "GPL and 
> > > additional rights". In fact you don't want any other tag because 
> > > when combined  with the kernel it is GPLv2 anyway because the 
> > > only way the two are fully compatible is for the kernel community 
> > > to license the derived work under the GPL.
> > 
> > This is the module tag ... it says what licence the module is 
> > under, not the licence for the module combined with the kernel, 
> > which is always GPLv2 because the stricter licence rules.
> 
> Then why isn't "BSD" in the license_is_gpl_compatible list?

[Sorry about this, the list seems to have stopped sending me copies of
stuff I'm on the to: line for; not sure why.  Anyway, having fished
this copy out of my trash:]

It is, here specifically:

https://www.gnu.org/licenses/license-list.en.html#ModifiedBSD

James


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

Re: [Xen-devel] [RFC v3 00/13] linux: generalize sections, ranges and linker tables

[James Bottomley]

On Mon, 2016-08-15 at 21:15 +0100, Alan Cox wrote:
> > This is the module tag ... it says what licence the module is 
> > under, not the licence for the module combined with the kernel, 
> > which is always GPLv2 because the stricter licence rules.
> 
> Because if I build a BSD licensed module against the kernel, give you
> the binaries and refuse to give you the source I am conforming to the
> BSD licence in letter.

No, you can't.  Forget dual licensing for a minute: I can produce a
Linux kernel module under a pure BSD licence because BSD is compatible
with GPL.  However, if we assume for the sake of argument that a binary
module is a derived work of Linux, producing and distributing the
binary for the module brings the combination under GPLv2 via the
derivative works clause and I'm required to offer you corresponding
source code in spite of the fact that my module *only* has a BSD
licence.  The only known get out from this is if I make *you* produce
the binary (the open source shim defence).

>  So to use it with the kernel it needs to be GPL with additional
> rights (eg BSD including the source...)

I'm not quite sure what you're disagreeing over?  Is it semantics of a
Dual BSD/GPL licence vs a GPL with additional rights one?  Dual
licensed code is a well settled area: the ruling licence is the one
which permits the action.  So for a dual licencesed kernel module, if I
compile the module as a binary and distribute it, I'm required to
follow all the provisions of GPLv2 i.e. make you and offer of
corresponding source.   Conversely, if I cut and paste a section of the
driver source code into a BSD driver, I'm permitted to do this under
the BSD licence, so the pasted code doesn't carry GPLv2 contamination
into BSD.

You can certainly call the Dual BSD/GPL licence a GPL licence with
additional permission to include the source code in a BSD licensed
system, but it's not a precise equivalence because a true Dual Licensed
BSD/GPL driver may be cut and pasted into any code which is compatible
either with the BSD or GPL licences, meaning it's actually less strict
than your additional permission to include into BSD.

James


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

[Xen-devel] [xen-unstable test] 100492: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-winxpsp3  9 windows-install fail REGR. vs. 100488

Regressions which are regarded as allowable (not blocking):
 build-amd64-rumpuserxen   6 xen-buildfail  like 100488
 build-i386-rumpuserxen6 xen-buildfail  like 100488
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 100488
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 100488
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeatfail  like 100488
 test-amd64-amd64-xl-rtds  9 debian-install   fail  like 100488
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 100488
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 100488

Tests which did not succeed, but are not blocking:
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-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-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-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   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-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass

version targeted for testing:
 xen  01fe4da6243be692b972de4ec7b22ec92527c0f5
baseline version:
 xen  a55ad65d3a30d5b3a026a7481ce05f28065920f0

Last test of basis   100488  2016-08-15 01:58:52 Z0 days
Testing same since   100492  2016-08-15 10:43:55 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 

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

Re: [Xen-devel] [RFC v3 00/13] linux: generalize sections, ranges and linker tables

[Steven Rostedt]

On Mon, 15 Aug 2016 21:15:06 +0100
Alan Cox  wrote:

> > This is the module tag ... it says what licence the module is under,
> > not the licence for the module combined with the kernel, which is
> > always GPLv2 because the stricter licence rules.  
> 
> Because if I build a BSD licensed module against the kernel, give you
> the binaries and refuse to give you the source I am conforming to the
> BSD licence in letter. So to use it with the kernel it needs to be GPL
> with additional rights (eg BSD including the source...)
> 

But that only pertains to the code that was modified to be used with
the Linux kernel, right? That is, if there's a BSD licensed module for
device FOO, and I port it to the Linux kernel, that will need to have a
GPL added to it to be included in Linux. But the original BSD code is
not affected. If a fix is made to the GPL Linux version, I'm assuming
(because I've been asked when doing something like this), that the
author of that fix will have to give it a dual license to be used back
in the original BSD only code. Correct?

I'm just trying to understand this. From what would make sense to me
(but may or may not to a court of law, where it counts), is that the
code added to Linux must be under GPL. But using that code depends on
where you get it from. If you use BSD source, it stays under BSD. But
any fixes to the GPL version will require permission to be put back to
the BSD version. A change to the GPL version doesn't automatically get
allowed back to the BSD only version?

-- Steve

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

Re: [Xen-devel] [PULL 0/2] xen-20160812-tag-2

[Peter Maydell]

On 13 August 2016 at 00:42, Stefano Stabellini  wrote:
> The following changes since commit 28b874429ba16e71e0caa46453f3a3e31efb3c51:
>
>   Merge remote-tracking branch 
> 'remotes/amit-migration/tags/migration-for-2.7-7' into staging (2016-08-11 
> 17:53:35 +0100)
>
> are available in the git repository at:
>
>
>   git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/xen-20160812-tag-2
>
> for you to fetch changes up to b7665c6027c972c23668ee74b878b5c617218514:
>
>   xen: handle inbound migration of VMs without ioreq server pages (2016-08-12 
> 16:38:30 -0700)
>
> 
> Xen 2016/08/12, fixed commit message
>
> 
> Cao jin (1):
>   Xen: fix converity warning of xen_pt_config_init()
>
> Paul Durrant (1):
>   xen: handle inbound migration of VMs without ioreq server pages

Applied, thanks.

-- PMM

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

[Xen-devel] [xen-4.7-testing test] 100491: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-credit2  19 guest-start/debian.repeat fail REGR. vs. 99972

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds  9 debian-installfail REGR. vs. 99972
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 99972
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 99972

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 build-i386-rumpuserxen6 xen-buildfail   never pass
 build-amd64-rumpuserxen   6 xen-buildfail   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-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-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-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-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
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail 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-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  822464961ae1bac44dcabb049255d61d5511e368
baseline version:
 xen  f2160ba6e60e990060de96f2fc9be645f51f5995

Last test of basis99972  2016-08-05 23:11:40 Z9 days
Testing same since   100491  2016-08-15 10:42:45 Z0 days1 attempts


People who touched revisions under test:
  Bob Liu 
  Boris Ostrovsky 
  Ian Jackson 
  Juergen Gross 
  Wei Liu 

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

Re: [Xen-devel] [V4 PATCH 2/2] mips/panic: Replace smp_send_stop() with kdump friendly version in panic path

[Corey Minyard]

On 08/15/2016 12:06 PM, Corey Minyard wrote:

On 08/15/2016 06:35 AM, 河合英宏 / KAWAI，HIDEHIRO wrote:

Hi Corey,


From: Corey Minyard [mailto:cminy...@mvista.com]
Sent: Friday, August 12, 2016 10:56 PM
I'll try to test this, but I have one comment inline...

Thank you very much!


On 08/11/2016 10:17 PM, Dave Young wrote:

On 08/10/16 at 05:09pm, Hidehiro Kawai wrote:

[snip]

diff --git a/arch/mips/kernel/crash.c b/arch/mips/kernel/crash.c
index 610f0f3..1723b17 100644
--- a/arch/mips/kernel/crash.c
+++ b/arch/mips/kernel/crash.c
@@ -47,9 +47,14 @@ static void crash_shutdown_secondary(void 
*passed_regs)


   static void crash_kexec_prepare_cpus(void)
   {
+static int cpus_stopped;
   unsigned int msecs;
+unsigned int ncpus;

-unsigned int ncpus = num_online_cpus() - 1;/* Excluding the 
panic cpu */

+if (cpus_stopped)
+return;

Wouldn't you want an atomic operation and some special handling here to
ensure that only one CPU does this?  So if a CPU comes in here and
another CPU is already in the process stopping the CPUs it won't 
result in a

deadlock.

Because this function can be called only one panicking CPU,
there is no problem.

There are two paths which crash_kexec_prepare_cpus is called.

Path 1 (panic path):
panic()
   crash_smp_send_stop()
 crash_kexec_prepare_cpus()

Path 2 (oops path):
crash_kexec()
   __crash_kexec()
 machine_crash_shutdown()
   default_machine_crash_shutdown() // for MIPS
 crash_kexec_prepare_cpus()

Here, panic() and crash_kexec() run exclusively via
panic_cpu atomic variable.  So we can use cpus_stopped as
normal variable.


Ok, if the code can only be entered once, what's the purpose of 
cpus_stopped?
I guess that's what confused me.  You are right, the panic_cpu atomic 
should

keep this on a single CPU.


Never mind, I see the path through panic() where that is required. My 
question

below still remains, though.

-corey



Also, panic() will call panic_smp_self_stop() if it finds another CPU 
has already
called panic, which will just spin with interrupts off by default. I 
didn't see a
definition for it in MIPS, wouldn't it need to be overridden to avoid 
a deadlock?


-corey



Best regards,

Hidehiro Kawai






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

Re: [Xen-devel] [PATCH 6/7] arm64: xen: Enable user access before a privcmd hvc call

[Stefano Stabellini]

On Mon, 15 Aug 2016, Julien Grall wrote:
> Hi Catalin,
> 
> I have CCed Stefano who is maintaining the Xen ARM code in Linux.
> 
> On 12/08/2016 17:27, Catalin Marinas wrote:
> > Privcmd calls are issued by the userspace. The kernel needs to enable
> > access to TTBR0_EL1 as the hypervisor would issue stage 1 translations
> > to user memory via AT instructions. Since AT instructions are not
> > affected by the PAN bit (ARMv8.1), we only need the explicit
> > uaccess_enable/disable if the TTBR0 PAN option is enabled.
> > 
> > Cc: Julien Grall 
> > Cc: Will Deacon 
> > Cc: James Morse 
> > Cc: Kees Cook 
> > Signed-off-by: Catalin Marinas 
> 
> Reviewed-by: Julien Grall 
> 
> Regards,
> 
> > ---
> >  arch/arm64/xen/hypercall.S | 18 ++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
> > index 329c8027b0a9..4c509f4f4dcc 100644
> > --- a/arch/arm64/xen/hypercall.S
> > +++ b/arch/arm64/xen/hypercall.S
> > @@ -91,6 +91,24 @@ ENTRY(privcmd_call)
> > mov x2, x3
> > mov x3, x4
> > mov x4, x5
> > +#ifdef CONFIG_ARM64_TTBR0_PAN
> > +   /*
> > +* Privcmd calls are issued by the userspace. The kernel needs to
> > +* enable access to TTBR0_EL1 as the hypervisor would issue stage 1
> > +* translations to user memory via AT instructions. Since AT
> > +* instructions are not affected by the PAN bit (ARMv8.1), we only
> > +* need the explicit uaccess_enable/disable if the TTBR0 PAN option is
> > +* enabled.

That's a pity but it is how it is.

Acked-by: Stefano Stabellini 

Given that the patch is part of your PAN series, I expect that it is
going to go via your tree.


> > +   uaccess_enable x6, x7, x8
> > +#endif
> > hvc XEN_IMM
> > +
> > +#ifdef CONFIG_ARM64_TTBR0_PAN
> > +   /*
> > +* Disable userspace access from kernel once the hyp call completed.
> > +*/
> > +   uaccess_disable x6
> > +#endif
> > ret
> >  ENDPROC(privcmd_call);
> > 
> > ___
> > linux-arm-kernel mailing list
> > linux-arm-ker...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> 
> -- 
> Julien Grall
> 

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

Re: [Xen-devel] [Qemu-devel] [RFC for 2.8 0/3] Drop support for 64 bit guests on 32 bit hosts

[Stefano Stabellini]

On Wed, 10 Aug 2016, Gerd Hoffmann wrote:
> On Di, 2016-08-09 at 16:55 +0100, Alex Bennée wrote:
> > Hi,
> > 
> > I'm proposing for the 2.8 cycle we officially drop supporting 64 bit
> > guests on 32 bit hosts. For most of the KVM targets it doesn't make
> > any sense anyway and for TCG it makes things harder (e.g. supporting
> > 64 bit atomics on a 32 bit platform). I'm not actually convinced
> > things actually work if built or that anyone relies on these
> > combinations. Consider these patches a way of flushing any such users
> > out ;-)
> 
> Adding xen-devel to Cc.
> 
> 64bit xen hypervisor with 32bit dom0 running 32bit qemu, providing
> device emulation for 64bit dumUs at least used to be a common setup
> years ago.  I have my doubts this is still the case in recent xen
> versions, but I guess we better ask the Xen guys ...
> 
> Can anyone clarify?

Hi Gerd, thanks for CC'ing xen-devel on this.

Although 32bit Dom0 deployments are not as common as they used to be,
they are still certainly a valid scenario. However for them to work,
QEMU doesn't really need to support TCG 32bit on 64bit. QEMU when used
with Xen as accelerator doesn't actually do any CPU emulation; there was
even a proposal to remove the CPU emulation completely from Xen machines
a couple of years back
(http://marc.info/?l=qemu-devel=139051862915170=2). I hope this
helps.___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [V4 PATCH 2/2] mips/panic: Replace smp_send_stop() with kdump friendly version in panic path

[Corey Minyard]

On 08/15/2016 06:35 AM, 河合英宏 / KAWAI，HIDEHIRO wrote:

Hi Corey,


From: Corey Minyard [mailto:cminy...@mvista.com]
Sent: Friday, August 12, 2016 10:56 PM
I'll try to test this, but I have one comment inline...

Thank you very much!


On 08/11/2016 10:17 PM, Dave Young wrote:

On 08/10/16 at 05:09pm, Hidehiro Kawai wrote:

[snip]

diff --git a/arch/mips/kernel/crash.c b/arch/mips/kernel/crash.c
index 610f0f3..1723b17 100644
--- a/arch/mips/kernel/crash.c
+++ b/arch/mips/kernel/crash.c
@@ -47,9 +47,14 @@ static void crash_shutdown_secondary(void *passed_regs)

   static void crash_kexec_prepare_cpus(void)
   {
+   static int cpus_stopped;
unsigned int msecs;
+   unsigned int ncpus;

-   unsigned int ncpus = num_online_cpus() - 1;/* Excluding the panic cpu */
+   if (cpus_stopped)
+   return;

Wouldn't you want an atomic operation and some special handling here to
ensure that only one CPU does this?  So if a CPU comes in here and
another CPU is already in the process stopping the CPUs it won't result in a
deadlock.

Because this function can be called only one panicking CPU,
there is no problem.

There are two paths which crash_kexec_prepare_cpus is called.

Path 1 (panic path):
panic()
   crash_smp_send_stop()
 crash_kexec_prepare_cpus()

Path 2 (oops path):
crash_kexec()
   __crash_kexec()
 machine_crash_shutdown()
   default_machine_crash_shutdown() // for MIPS
 crash_kexec_prepare_cpus()

Here, panic() and crash_kexec() run exclusively via
panic_cpu atomic variable.  So we can use cpus_stopped as
normal variable.


Ok, if the code can only be entered once, what's the purpose of 
cpus_stopped?

I guess that's what confused me.  You are right, the panic_cpu atomic should
keep this on a single CPU.

Also, panic() will call panic_smp_self_stop() if it finds another CPU 
has already
called panic, which will just spin with interrupts off by default. I 
didn't see a
definition for it in MIPS, wouldn't it need to be overridden to avoid a 
deadlock?


-corey



Best regards,

Hidehiro Kawai




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

Re: [Xen-devel] [win-pv-devel] rtc timeoffset not being set on TZ changes?

[Nathan March]

Hi Paul,

 

Nope, I’m using qemu provided by xen-runtime in the centos packages 
(https://cbs.centos.org/koji/packageinfo?packageID=88).

 

root  1462  0.2  0.3 406856 21016 ?SLsl Aug13   9:37 
/usr/lib64/xen/bin/qemu-system-i386 -xen-domid 31 -chardev 
socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-31,server,nowait -no-shutdown 
-mon chardev=libxl-cmd,mode=control -chardev 
socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-31,server,nowait -mon 
chardev=libxenstat-cmd,mode=control -nodefaults -name nathanwin -vnc 
127.0.0.1:0,to=99 -display none -device cirrus-vga,vgamem_mb=8 -boot order=d 
-smp 2,maxcpus=2 -device rtl8139,id=nic0,netdev=net0,mac=00:16:3d:01:04:eb 
-netdev type=tap,id=net0,ifname=vif31.0-emu,script=no,downscript=no -device 
rtl8139,id=nic1,netdev=net1,mac=00:16:3d:01:04:ee -netdev 
type=tap,id=net1,ifname=vif31.1-emu,script=no,downscript=no -machine xenfv -m 
8184 -drive 
file=/mnt/gtc_disk_f1/nathanwin/drive_c,if=ide,index=0,media=disk,format=raw,cache=writeback
 -drive 
file=/mnt/xen/iso/xen_pv-8.1.iso,if=ide,index=2,readonly=on,media=cdrom,format=raw,cache=writeback,id=ide-51745

 

~ # rpm -qf /usr/lib64/xen/bin/qemu-system-i386

xen-runtime-4.6.3-1.el6.x86_64

 

- Nathan

 

 

From: Paul Durrant [mailto:paul.durr...@citrix.com] 
Sent: Monday, August 15, 2016 1:54 AM
To: Nathan March ; win-pv-de...@lists.xenproject.org
Cc: xen-devel@lists.xen.org
Subject: RE: [win-pv-devel] rtc timeoffset not being set on TZ changes?

 

Hi Nathan,

 

  Are you using upstream QEMU? If you are then you’re problem is expected. The 
code in xen-hvm.c:handle_ioreq() completely ignores RTC updates from Xen, as 
can be seen at 
http://xenbits.xen.org/gitweb/?p=qemu-upstream-unstable.git;a=blob;f=xen-hvm.c;hb=HEAD#l927
 whereas QEMU trad handles them, as can be seen at 
http://xenbits.xen.org/gitweb/?p=qemu-xen-unstable.git;a=blob;f=i386-dm/helper2.c;hb=HEAD#l475.
 The PV drivers do not directly interact with this key so there’s nothing 
essentially ‘wrong’ in your VM.

 

  Paul

 

From: win-pv-devel [mailto:win-pv-devel-boun...@lists.xenproject.org] On Behalf 
Of Nathan March
Sent: 12 August 2016 18:51
To: win-pv-de...@lists.xenproject.org 
 
Subject: [win-pv-devel] rtc timeoffset not being set on TZ changes?

 

Hi All,

 

On Win 2012 R2 with the latest 8.1 signed drivers, I'm having issues with 
clocks being reset back to host time on migration. Xen 4.6.3  with host kernel 
3.18.34, using gwd's centos packages.

 

Based on https://wiki.xenproject.org/wiki/HVM_timeoffsets I would expect to see 
rtc/timeoffset being set when I change a timezone/clock in windows, but that's 
not the case. No matter  what, it always seems to be null:

 

7c253d95-b15f-45b5-bf1c-395c1cc7b034 = ""

name = "nathanwin"

uuid = "7c253d95-b15f-45b5-bf1c-395c1cc7b034"

rtc = ""

  timeoffset = ""

image = ""

  ostype = "hvm"

start_time = "1471022816.81"

 

If I issue a shutdown from XL windows does a graceful shutdown, so the xenbus 
drivers do seem to be running fine.

 

Can anyone comment on what might be up here, or if this is just a bug? 

 

Cheers,

Nathan

 

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

Re: [Xen-devel] [PATCH v2 23/25] arm/altp2m: Extend libxl to activate altp2m on ARM.

[Sergej Proskurin]

Hi Wei,

On 08/11/2016 06:00 PM, Wei Liu wrote:
> Sorry for the late reply.
> 

No worries, it's all good :) Thanks for your reply.

[...]

 ("smbios_firmware",  string),
 ("acpi_firmware",string),
 ("hdtype",   libxl_hdtype),
 @@ -561,6 +560,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
  
  ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
> 
> But you do need to wrap the line properly.

I am not sure whether this comment was intended, as the upper line
containing the fields arch_arm and gic_version were not changed by the
patch.

])),
 +# Alternate p2m is not bound to any architecture or guest type, as it 
 is
 +# supported by x86 HVM and ARM PV guests.
>>> Just "ARM guests" would do. ARM doesn't have notion of PV vs HVM.
>>
>> I will change that, thank you. I mentioned ARM PV as currently it is the
>> type that is registered for guests on ARM.
>>
 +("altp2m", libxl_defbool),
  
  ], dir=DIR_IN
  )

[...]

  
  xlu_cfg_replace_string(config, "smbios_firmware",
 _info->u.hvm.smbios_firmware, 0);
 @@ -1727,6 +1732,25 @@ static void parse_config_data(const char 
 *config_source,
  abort();
  }
  
 +bool altp2m_support = false;
 +#if defined(__i386__) || defined(__x86_64__)
 +/* Alternate p2m support on x86 is available only for HVM guests. */
 +if (b_info->type == LIBXL_DOMAIN_TYPE_HVM)
 +altp2m_support = true;
 +#elif defined(__arm__) || defined(__aarch64__)
 +/* Alternate p2m support on ARM is available for all guests. */
 +altp2m_support = true;
 +#endif
 +
>>> I don't think you need to care too much about dead option here.
>>> Xl should be able to set altp2m field all the time. 
>>
>> I am actually not sure what you mean by the dead option. Could you be
>> more specific, please?
>>  
>> Also, in the last patch, we came to the agreement to guard the altp2m
>> functionality solely through the HVM param (instead of the additional
>> Xen cmd-line option altp2m), which is set through libxl. Because of
>> this, the entire domu initialization routines depend on this option to
>> be set at the moment of domain creation -- not after it. That is, being
>> able to set the altp2m option at all time would actually break the system.
>>
> 
> Why is this? It's possible we're talking past each other.
> 

I believe I was not entirely clear in my upper comment. By "we", I meant
all parties involved into the discussion concerning altp2m on ARM in
general. Sorry, for the confusion.

Also, I was wrong: The HVM param can indeed be set at any time without
any troubles.

>>> And there should be
>>> code in libxl to handle situation when altp2m is not available.
>>
>> I am not so sure about that either:
>>
>> Currently, altp2m is supported on x86 HVM and on ARM.
>>  * on x86, altp2m depends on HW to support altp2m. Therefore, the
>> cmd-line option "altp2m" is used do activate altp2m. All libxl
>> interaction with the altp2m subsystem will be discarded at this point.
>>  * on ARM, altp2m is implemented purely in SW. That is, we do not have
>> ARM architectures, that would not support altp2m -- so the idea.
>>  * All other architectures should not be able to activate altp2m, which
>> is why I chose the #defines (__arm__, __x86_64__, ...) to guard the
>> upper option.
>>
 +if (altp2m_support) {
 +/* The config parameter "altp2m" replaces the parameter 
 "altp2mhvm".
 + * For legacy reasons, both parameters are accepted on x86 HVM 
 guests
 + * (only "altp2m" is accepted on ARM guests). If both parameters 
 are
 + * given, it must be considered that the config parameter 
 "altp2m" will
 + * always have priority over "altp2mhvm". */
 +xlu_cfg_get_defbool(config, "altp2m", _info->altp2m, 0);
 +}
 +
> 
> 
> What I meant was:
> 
> We don't care xl (the application) sets altp2m or not. It's entirely possible
> that the xl on its own sets that field. The validation should be done
> inside libxl. If a particular configuration is not supported by libxl,
> it should reject that. Libxl shouldn't rely on xl (the application) to
> pass in fully sanitised data, it should sanitise the input itself.
> 
> Basically that means, in xl, you only need:
> 
> + xlu_cfg_get_defbool(config, "altp2m", _info->altp2m, 0);
> 
> And then in libxl:initiate_domain_create you check if the configuration
> is valid. You can see a bunch of other checks are already there.
> 
> Does this make sense to you?
> 
> 

I think so. As you said, in my next patch, I will simply set
b_info->u.hvm.altp2m and b_info->altp2m during parsing and check for

Re: [Xen-devel] [PATCH 2/2] xen/events: Convert to hotplug state machine

[Boris Ostrovsky]

On 08/15/2016 11:06 AM, David Vrabel wrote:
> On 15/08/16 15:46, Boris Ostrovsky wrote:
>> From: Sebastian Andrzej Siewior 
>>
>> Install the callbacks via the state machine.
> [...]
>> +static int xen_evtchn_cpu_dead(unsigned int cpu)
>> +{
>> +__evtchn_fifo_handle_events(cpu, true);
>> +return 0;
>> +}
> I'm not familiar with the new state machine.  When this is called, what
> state is the CPU in?
>
> In particular, local interrupts must be disabled and all non-percpu irqs
> must have been migrated to other CPUs.

This (xen_evtchn_cpu_dead()) is called immediately after notify_dead()
on the way down.

The state machine is walking cpuhp_state list and for each member of the
list it calls the callback that has been registered for that member.

So when we bring a CPU up first we call xen_evtchn_cpu_prepare() and
then in the next iteration of the state machine loop notify_prepare()
(because CPUHP_XEN_EVTCHN_PREPARE is immediately before
CPUHP_NOTIFY_PREPARE). On the way down it's done in reverse: first
notify_dead() and then xen_evtchn_cpu_dead().

In other words, the old notification scheme is part of new state machine:

CPUHP_RCUTREE_PREP,
CPUHP_XEN_PREPARE,
+   CPUHP_XEN_EVTCHN_PREPARE,
CPUHP_NOTIFY_PREPARE,  <=== CPU notifiers callback
CPUHP_TIMERS_DEAD,
CPUHP_BRINGUP_CPU,


-boris


>
>
>>  int __init xen_evtchn_fifo_init(void)
>>  {
>> @@ -456,7 +444,9 @@ int __init xen_evtchn_fifo_init(void)
>>  
>>  evtchn_ops = _ops_fifo;
>>  
>> -register_cpu_notifier(_fifo_cpu_notifier);
>> +cpuhp_setup_state_nocalls(CPUHP_XEN_EVTCHN_PREPARE,
>> +  "CPUHP_XEN_EVTCHN_PREPARE",
>> +  xen_evtchn_cpu_prepare, xen_evtchn_cpu_dead);
>>  out:
>>  put_cpu();
>>  return ret;
>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
>> index d6beeb9..c60a17c 100644
>> --- a/include/linux/cpuhotplug.h
>> +++ b/include/linux/cpuhotplug.h
>> @@ -22,6 +22,7 @@ enum cpuhp_state {
>>  CPUHP_SMPCFD_PREPARE,
>>  CPUHP_RCUTREE_PREP,
>>  CPUHP_XEN_PREPARE,
>> +CPUHP_XEN_EVTCHN_PREPARE,
>>  CPUHP_NOTIFY_PREPARE,
>>  CPUHP_TIMERS_DEAD,
>>  CPUHP_BRINGUP_CPU,
>>



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

Re: [Xen-devel] [PATCH v1] Livepatch ARM 64 implementation

[Konrad Rzeszutek Wilk]

On Mon, Aug 15, 2016 at 04:52:58PM +0200, Julien Grall wrote:
> 
> 
> On 15/08/2016 01:07, Konrad Rzeszutek Wilk wrote:
> > Hey!
> 
> Hi Konrad,
> 
> > This is the first (non RFC) posting of the enablement of Livepatch under 
> > ARM64.
> > 
> > The patches are based on: [PATCH v3] Livepatch fixes and features for v4.8.
> > (https://lists.xen.org/archives/html/xen-devel/2016-08/msg01825.html)
> > 
> > And the git tree is:
> >  git://xenbits.xen.org/people/konradwilk/xen.git livepatch.v4.8.v3
> > 
> > I've only tested this under Foundation Platform with only one CPU working
> > (the other CPUs wouldn't boot up for some reason) and without a proper 
> > working
> > disk image (can't recall why, but it did have an initramfs) - I ended
> > up building the hypervisor with the livepatch built in and loading it during
> > dom0 execution (via timers), see
> > (http://xenbits.xen.org/gitweb/?p=people/konradwilk/xen.git;a=commit;h=39517b2b807025d0d63d4f042ada5eb3de32ff45)
> > [That patch is not part of this patchset of course]
> 
> I am able to use both SMP and the rootfs on the foundation model. What is
> hte command line you are using? How about the device tree?

~/ARM/Foundation_Platformpkg/models/Linux64_GCC-4.7/Foundation_Platform --image 
~/ARM/boot-wrapper-aarch64.git/xen-system.axf 

And the boot-wrapper-aarch64.git does:
aarch64  git log
--oneline | head -1
b564cbf Fix build when USE_INITRD not set
aarch64  make xen-system.axf
aarch64-linux-gnu-gcc  -DCNTFRQ=0x0180   -DUART_BASE=0x1c09 
-DLED=0x0008 -DSYSREGS_BASE=0x1c01 -DGIC_DIST_BASE=0x2c001000 
-DGIC_CPU_BASE=0x2c002000 -c -o boot.xen.o boot.S -DXEN
aarch64-linux-gnu-gcc  -DPHYS_OFFSET=0x8000 -DMBOX_OFFSET=0xfff8 
-DBOOT=boot.xen.o -DXEN_OFFSET=0xA0 -DKERNEL_OFFSET=0x8 
-DFDT_OFFSET=0x0800 -DFS_OFFSET=0x1000 -DXEN=Xen -DKERNEL=Image 
-DFILESYSTEM=filesystem.cpio.gz -E -P -C -o model.xen.lds model.lds.S

And it looks I tried at some point to play with the LEDs (not very well)

diff --git a/Makefile b/Makefile
index 241091f..dd50f09 100644
--- a/Makefile
+++ b/Makefile
@@ -13,7 +13,7 @@ SYSREGS_BASE  := 0x1c01
 GIC_DIST_BASE  := 0x2c001000
 GIC_CPU_BASE   := 0x2c002000
 CNTFRQ := 0x0180   # 24Mhz
-
+LED:= 0x0008
 #INITRD_FLAGS  := -DUSE_INITRD
 CPPFLAGS   += $(INITRD_FLAGS)
 
@@ -78,13 +78,13 @@ $(XIMAGE): boot.xen.o model.xen.lds fdt.dtb $(XEN) 
$(KERNEL) $(FILESYSTEM)
$(LD) -o $@ --script=model.xen.lds
 
 boot.o: $(BOOTLOADER) Makefile
-   $(CC) $(CPPFLAGS) -DCNTFRQ=$(CNTFRQ) -DUART_BASE=$(UART_BASE) 
-DSYSREGS_BASE=$(SYSREGS_BASE) -DGIC_DIST_BASE=$(GIC_DIST_BASE) 
-DGIC_CPU_BASE=$(GIC_CPU_BASE) -c -o $@ $(BOOTLOADER)
+   $(CC) $(CPPFLAGS) -DCNTFRQ=$(CNTFRQ) -DUART_BASE=$(UART_BASE) 
-DLED=$(LED) -DSYSREGS_BASE=$(SYSREGS_BASE) -DGIC_DIST_BASE=$(GIC_DIST_BASE) 
-DGIC_CPU_BASE=$(GIC_CPU_BASE) -c -o $@ $(BOOTLOADER)
 
 model.lds: $(LD_SCRIPT) Makefile
$(CC) $(CPPFLAGS) -DPHYS_OFFSET=$(PHYS_OFFSET) 
-DMBOX_OFFSET=$(MBOX_OFFSET) -DBOOT=boot.o -DKERNEL_OFFSET=$(KERNEL_OFFSET) 
-DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) -DKERNEL=$(KERNEL) 
-DFILESYSTEM=$(FILESYSTEM) -E -P -C -o $@ $<
 
 boot.xen.o: $(BOOTLOADER) Makefile
-   $(CC) $(CPPFLAGS) -DCNTFRQ=$(CNTFRQ) -DUART_BASE=$(UART_BASE) 
-DSYSREGS_BASE=$(SYSREGS_BASE) -DGIC_DIST_BASE=$(GIC_DIST_BASE) 
-DGIC_CPU_BASE=$(GIC_CPU_BASE) -c -o $@ $(BOOTLOADER) -DXEN
+   $(CC) $(CPPFLAGS) -DCNTFRQ=$(CNTFRQ) -DUART_BASE=$(UART_BASE) 
-DLED=$(LED) -DSYSREGS_BASE=$(SYSREGS_BASE) -DGIC_DIST_BASE=$(GIC_DIST_BASE) 
-DGIC_CPU_BASE=$(GIC_CPU_BASE) -c -o $@ $(BOOTLOADER) -DXEN
 
 model.xen.lds: $(LD_SCRIPT) Makefile
$(CC) $(CPPFLAGS) -DPHYS_OFFSET=$(PHYS_OFFSET) 
-DMBOX_OFFSET=$(MBOX_OFFSET) -DBOOT=boot.xen.o -DXEN_OFFSET=$(XEN_OFFSET) 
-DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) 
-DFS_OFFSET=$(FS_OFFSET) -DXEN=$(XEN) -DKERNEL=$(KERNEL) 
-DFILESYSTEM=$(FILESYSTEM) -E -P -C -o $@ $<
diff --git a/boot.S b/boot.S
index 3e2cecd..60b642c 100644
--- a/boot.S
+++ b/boot.S
@@ -107,6 +107,13 @@ start_ns:
str wzr, [x4, #0xa0]// V2M_SYS_CFGDATA
str w5, [x4, #0xa4] // V2M_SYS_CFGCTRL
 
+   ldr x4, =LED
+   mov w5, #0xFF
+   str w5, [x4]
+
+   mov x4, #0x00A4
+   mov w5, #0xC080 
+   str w5, [x4]
/*
 * Primary CPU
 */

Anyhow the more important is the DTS:

total 248784
drwxrwxr-x.  3 konrad konrad 4096 Aug 15 11:46 .
drwxrwxr-x. 11 konrad konrad 4096 Aug 15 11:40 ..
-rw-rw-r--.  1 konrad konrad 90957497 Apr 15 15:50 64
-rw-rw-r--.  1 konrad konrad 90571528 Apr 15 15:49 a
-rw-rw-r--.  1 konrad konrad 2388 Apr 15 22:54 boot.S
-rw-rw-r--.  1 konrad konrad 1544 Aug 15 11:46 boot.xen.o
lrwxrwxrwx.  1 konrad konrad   41 Apr 15 14:17 fdt.dtb -> 

[Xen-devel] [PATCH] tools: delete gtraceview and gtracestat

[Wei Liu]

There has not been any substantial update to them since 2011. My quick
check shows that they don't work.

Just delete them. It would be easy to resurrect them from git log should
people still need them.

Signed-off-by: Wei Liu 
---
Cc: Ian Jackson 
---
 .gitignore  |2 -
 .hgignore   |2 -
 tools/misc/Makefile |8 -
 tools/misc/gtracestat.c | 1110 --
 tools/misc/gtraceview.c |  ---
 5 files changed, 2233 deletions(-)
 delete mode 100644 tools/misc/gtracestat.c
 delete mode 100644 tools/misc/gtraceview.c

diff --git a/.gitignore b/.gitignore
index d193820..065a8a9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -192,8 +192,6 @@ tools/misc/xen-livepatch
 tools/misc/xenperf
 tools/misc/xenpm
 tools/misc/xen-hvmctx
-tools/misc/gtraceview
-tools/misc/gtracestat
 tools/misc/xenlockprof
 tools/misc/lowmemd
 tools/misc/xencov
diff --git a/.hgignore b/.hgignore
index 0bd29a1..8342f36 100644
--- a/.hgignore
+++ b/.hgignore
@@ -205,8 +205,6 @@
 ^tools/misc/xenpm$
 ^tools/misc/xen-hvmctx$
 ^tools/misc/xen-lowmemd$
-^tools/misc/gtraceview$
-^tools/misc/gtracestat$
 ^tools/misc/xenlockprof$
 ^tools/misc/xencov$
 ^tools/pygrub/build/.*$
diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index cee2b99..8152f7b 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -17,8 +17,6 @@ INSTALL_BIN+= xencov_split
 INSTALL_BIN += $(INSTALL_BIN-y)
 
 # Everything to be installed in regular sbin/
-INSTALL_SBIN   += gtracestat
-INSTALL_SBIN   += gtraceview
 INSTALL_SBIN   += xen-bugtool
 INSTALL_SBIN-$(CONFIG_MIGRATE) += xen-hptool
 INSTALL_SBIN-$(CONFIG_X86) += xen-hvmcrash
@@ -85,9 +83,6 @@ xenperf: xenperf.o
 xenpm: xenpm.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
 
-gtracestat: gtracestat.o
-   $(CC) $(LDFLAGS) -o $@ $< $(APPEND_LDFLAGS)
-
 xenlockprof: xenlockprof.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
 
@@ -110,9 +105,6 @@ xen-livepatch: xen-livepatch.o
 xen-lowmemd: xen-lowmemd.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenevtchn) $(LDLIBS_libxenctrl) 
$(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
 
-gtraceview: gtraceview.o
-   $(CC) $(LDFLAGS) -o $@ $< $(CURSES_LIBS) $(TINFO_LIBS) $(APPEND_LDFLAGS)
-
 xencov: xencov.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
 
diff --git a/tools/misc/gtracestat.c b/tools/misc/gtracestat.c
deleted file mode 100644
index 67cb003..000
--- a/tools/misc/gtracestat.c
+++ /dev/null
@@ -1,1110 +0,0 @@
-/*
- * gtracestat.c: list the statistics information for a dumped xentrace file.
- * Copyright (c) 2009, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see .
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-#include 
-
-#define CHECK_DUP_CX 0
-
-/** MACROS **/
-#define MAX_CPU_NR  32
-#define MAX_CX_NR   8
-#define MAX_MODE_NR 16
-#define MAX_PX_NR  100
-
-/* simplified xentrace record */
-struct rec {
-uint64_t tsc;
-int cpu;
-unsigned char cx;
-unsigned char irqs[4];
-unsigned int predicted;
-unsigned int expected;
-int px;
-};
-
-/** FORWARD DECLARATION **/
-static void show_help(void);
-static void show_version(void);
-static int load_file(char *fname);
-static void do_digest(uint64_t start, uint64_t end, uint64_t scale);
-static void do_breakevents(void);
-static void do_count(void);
-static void do_px_count(void);
-static void do_maxmin(void);
-static void do_average(void);
-static void do_exp_ratio(void);
-static void do_exp_pred(void);
-
-/** GLOBAL VARIABLES **/
-/* store simplified xentrace data */
-static struct rec *data;
-static int64_t data_nr, data_cur;
-/* store max cx state number and cpu number */
-static int max_cx_num = -1, max_cpu_num = -1;
-static int px_freq_table[MAX_PX_NR];
-static int max_px_num = 0;
-
-static int is_menu_gov_enabled = 0;
-
-/* user specified translation unit */
-static uint64_t tsc2ms = 2793000UL;
-static uint64_t tsc2us = 2793UL;
-static uint64_t tsc2phase = 5580UL;
-
-/* each cpu column width */
-static int width = 0;
-
-/* digest mode variables */
-static struct rec 

Re: [Xen-devel] [PATCH v1 6/9] livepatch: Initial ARM64 support.

[Andrew Cooper]

On 15/08/16 16:25, Julien Grall wrote:
>
>
> On 15/08/2016 17:17, Konrad Rzeszutek Wilk wrote:
>> On Mon, Aug 15, 2016 at 04:57:26PM +0200, Julien Grall wrote:
>>> Hi Jan and Konrad,
>>>
>>> On 15/08/2016 16:23, Jan Beulich wrote:
>>> On 15.08.16 at 16:09,  wrote:
> On Mon, Aug 15, 2016 at 02:21:48AM -0600, Jan Beulich wrote:
> On 15.08.16 at 01:07,  wrote:
>>> @@ -711,9 +711,15 @@ static int prepare_payload(struct payload
>>> *payload,
>>>  return -EINVAL;
>>>  }
>>>  }
>>> +#ifndef CONFIG_ARM
>>>  apply_alternatives_nocheck(start, end);
>>> +#else
>>> +apply_alternatives(start, sec->sec->sh_size);
>>> +#endif
>>
>> Conditionals like this are ugly - can't this be properly abstracted?
>
> Yes, I can introduce an apply_alternatives_nocheck on ARM that will
> hava the same set of arguments on x86.
>
> Or I can make a new function name?

 Either way is fine with me, with a slight preference to the former
 one.
>>>
>>> I am fine with the prototype of the function
>>> apply_alternatives_nocheck but
>>> I don't think the name is relevant for ARM.
>>>
>>> Is there any reason we don't want to call directly
>>> apply_alternatives in
>>> x86?
>>
>> It assumes (and has an ASSERT) that it is called with interrupts
>> disabled.
>> And we don't need to do that (as during livepatch loading we can
>> modify the
>> livepatch payload without worrying about interrupts).
>
> Oh, it makes more sense now.
>
>>
>> P.S.
>> loading != applying.
>>
>> I could do a patch where we rename 'apply_alternatives' ->
>> 'apply_alternatives_boot'
>> and 'apply_alternatives_nocheck' to 'apply_alternatives'.

The only reason apply_alternatives() is named thusly is to match Linux. 
I am not fussed if it changes.

~Andrew

>>
>> Also the x86 version instead of having:
>>
>> apply_alternatives(struct alt_instr *start, struct alt_instr *end)
>>
>> would end up with:
>>
>> apply_alternatives(void *start, size_t length)
>
> I would be fine with the prototype
> apply_alternatives(struct alt_instr *start, struct alt_instr *end)
>
> for ARM. It is up to you.
>
> Cheers,
>


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

Re: [Xen-devel] [PATCH v1 6/9] livepatch: Initial ARM64 support.

[Julien Grall]

On 15/08/2016 17:17, Konrad Rzeszutek Wilk wrote:

On Mon, Aug 15, 2016 at 04:57:26PM +0200, Julien Grall wrote:

Hi Jan and Konrad,

On 15/08/2016 16:23, Jan Beulich wrote:

On 15.08.16 at 16:09,  wrote:

On Mon, Aug 15, 2016 at 02:21:48AM -0600, Jan Beulich wrote:

On 15.08.16 at 01:07,  wrote:

@@ -711,9 +711,15 @@ static int prepare_payload(struct payload *payload,
 return -EINVAL;
 }
 }
+#ifndef CONFIG_ARM
 apply_alternatives_nocheck(start, end);
+#else
+apply_alternatives(start, sec->sec->sh_size);
+#endif


Conditionals like this are ugly - can't this be properly abstracted?


Yes, I can introduce an apply_alternatives_nocheck on ARM that will
hava the same set of arguments on x86.

Or I can make a new function name?


Either way is fine with me, with a slight preference to the former
one.


I am fine with the prototype of the function apply_alternatives_nocheck but
I don't think the name is relevant for ARM.

Is there any reason we don't want to call directly apply_alternatives in
x86?


It assumes (and has an ASSERT) that it is called with interrupts disabled.
And we don't need to do that (as during livepatch loading we can modify the
livepatch payload without worrying about interrupts).


Oh, it makes more sense now.



P.S.
loading != applying.

I could do a patch where we rename 'apply_alternatives' -> 
'apply_alternatives_boot'
and 'apply_alternatives_nocheck' to 'apply_alternatives'.

Also the x86 version instead of having:

apply_alternatives(struct alt_instr *start, struct alt_instr *end)

would end up with:

apply_alternatives(void *start, size_t length)


I would be fine with the prototype
apply_alternatives(struct alt_instr *start, struct alt_instr *end)

for ARM. It is up to you.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 15.08.16 at 16:50,  wrote:
> On 09/08/16 11:29, Jan Beulich wrote:
> On 08.08.16 at 15:46,  wrote:
>>> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
>>> depriv)"):
 On 05.08.16 at 18:28,  wrote:
> That is, a bug of class 2 would allow the unprivileged qemu process in
> dom0 to cause damage to other parts of dom0.
>>> ...
 Ah, okay, I think I finally understand. [...]

 I'm having, however, a hard time imagining a class 2 bug for any
 of the hvmop-s that are being converted by the hvmctl series:
 These act on the target domain, so would not touch the calling
 ones state other than for copying argument structures to/from
 guest memory (and I don't view mixing up domain pointers as
 a likely source of problems).
>>>
>>> Right.  AIUI all the hypercall arguments are passed using
>>> calling-guest-virtual addresses, which the dom0 privcmd arrangements
>>> are capable of ensuring are sane.
>> 
>> Actually, having thought about this some more, and taking this
>> together with the expectations to the privcmd driver previously
>> outlined, I think this part is problematic: If all the driver is to know
>> is the position (within the interface structure) of the target domain
>> ID, then any guest handles embedded in the interface structure
>> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
>> validated, and hence user mode code would have a way to access
>> or modify kernel memory.
> 
> It seems simpler to me to have in the privcmd driver:
> 
> if (op == HVMCTL_track_dirty_vram)
> ret = access_ok(...);
> 
> It looks simpler to me to fix the ABI and add the checking in the
> privcmd driver than add one of the proposed mechanisms to allow the
> hypervisor to do the checking.

Simpler, yes, but ...

> To avoid the issues with having to update the kernel in lock-step with
> the hypervisor (if new checks are needed in privcmd), we can (in the
> common case) do version the checking in the driver.
> 
> i.e., if the privcmd drivers knows about hypervisor ABI V but qemu needs
> V+1 then it can choose to disable the depriv and thus continue to work
> (with reduced defense in depth).

... less flexible, and prone to end up in a mess once we have more
than a handful of versions for the driver to deal with.

Jan


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

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 15.08.16 at 16:57,  wrote:
> On 15/08/16 12:20, Jan Beulich wrote:
> On 15.08.16 at 12:47,  wrote:
>>> On 15/08/16 11:19, Jan Beulich wrote:
 Well, none of the options considered so far are really nice or
 readily available. I think the easiest to use for both the caller and
 the implementation of the hypercall would be the auxiliary
 hypercall for a kernel to indicate user/kernel boundaries plus a
 flag on the DMOP one for the kernel mode driver to indicate its
 user mode origin. The main (purely theoretical afaict) downside
 of this is the difficulty to use it in OSes with variable user/kernel
 boundaries.
>>>
>>> What about including in the "fixed" part of the hypercall a virtual
>>> address range that all pointers must be in?  That wouldn't even require
>>> a user/kernel flag actually; and could conceivably be used by the caller
>>> (either userspace or kernel space) to thwart certain kinds of potential
>>> attacks.
>> 
>> That's definitely an option, if we're sufficiently certain that no OSes
>> will ever require two or more ranges.
> 
> I'm sorry, I think this is getting a bit silly.  There are currently no
> known operating systems which have discontinuous user-space virtual
> address ranges.  Even if there were, the hypercall structure I'm
> proposing would still function; the only restriction would be that any
> single hypercall would have to have all arguments within one of those
> ranges.

Well, you then discount the original vDSO range 64-bit Linux had
high up in the fixmap area. Not that this area would be usable for
any bad here, but it shows that such an idea isn't pure theory.

> Another idea I had was to do a multi-call-style hypercall "wrapper"
> hypercall, similar to David's XSM idea, but instead of running with a
> specific XSM label, would restrict the target domain and VA range of all
> included hypercalls.  Then the individual hypercall structure wouldn't
> need to be known at all; and if it ever became important to have (say)
> two VA ranges, we could simply add a new "wrapper" hypercall.

That should work too, yes.

Jan


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

Re: [Xen-devel] [PATCH v3 6/9] livepatch: Add parsing for the symbol+0x/

[Konrad Rzeszutek Wilk]

On Mon, Aug 15, 2016 at 09:12:21AM -0600, Jan Beulich wrote:
> >>> On 15.08.16 at 16:35,  wrote:
> > On Mon, Aug 15, 2016 at 04:53:48AM -0600, Jan Beulich wrote:
> >> >>> On 14.08.16 at 23:52,  wrote:
> >> > in case we want to patch at specific offsets inside
> >> > a function. (for example if we want to do NOP patching).
> >> > 
> >> > We also assume that the 'len' is only the size of
> >> > an isns that would be for a call opcode (so 5 bytes
> >> > on x86, and 4 on ARM 32/64).
> >> 
> >> Which makes the notation using a slash quite confusing: Normally
> >> if we see +/ the length part is assumed
> >> to be the overall size of the object/function the name refers to.
> > 
> > Oh, I somehow had the length of instruction on my mind.
> > 
> >> Could I talk you into using a different separator, like colon or
> >> semicolon?
> > 
> > Of course. Whatever is easier to folks.
> > 
> > I can also ditch the  part altogether?
> 
> The question is why you added it in the first place (see also the
> comments on the other patch, where I'm puzzled by the restriction
> to 5 bytes length).

Oh, I was looking at the dump stacks and got in my mind that /
MUST be part of it. But it most certainly does not and I can remove it.

> 
> Jan
> 

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

Re: [Xen-devel] [PATCH v1 6/9] livepatch: Initial ARM64 support.

[Julien Grall]

Hi Konrad,

On 15/08/2016 01:07, Konrad Rzeszutek Wilk wrote:

[...]


diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index b20db64..5966de0 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -4,8 +4,8 @@ obj-$(EARLY_PRINTK) += debug.o
 obj-y += domctl.o
 obj-y += domain.o
 obj-y += entry.o
+obj-$(CONFIG_LIVEPATCH) += livepatch.o
 obj-y += proc-v7.o proc-caxx.o
 obj-y += smpboot.o
 obj-y += traps.o
 obj-y += vfp.o
-


Spurious change?


diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
new file mode 100644
index 000..89ee2f5
--- /dev/null
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -0,0 +1,55 @@
+/*
+ *  Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+void arch_livepatch_apply_jmp(struct livepatch_func *func)
+{
+}
+
+void arch_livepatch_revert_jmp(const struct livepatch_func *func)
+{
+}
+
+int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
+{
+const Elf_Ehdr *hdr = elf->hdr;
+
+if ( hdr->e_machine != EM_ARM ||
+ hdr->e_ident[EI_CLASS] != ELFCLASS32 )
+{
+dprintk(XENLOG_ERR, LIVEPATCH "%s: Unsupported ELF Machine type!\n",
+elf->name);
+return -EOPNOTSUPP;
+}
+
+if ( (hdr->e_flags & EF_ARM_EABI_MASK) != EF_ARM_EABI_VER5 )
+{
+dprintk(XENLOG_ERR, LIVEPATCH "%s: Unsupported ELF EABI(%x)!\n",
+elf->name, hdr->e_flags);
+return -EOPNOTSUPP;
+}
+
+return 0;


I would prefer if you introduce ARM32 implementation in one go. I.e can 
you only return -EOPNOTSUPP here for now?



[...]


+int arch_livepatch_perform_rela(struct livepatch_elf *elf,
+const struct livepatch_elf_sec *base,
+const struct livepatch_elf_sec *rela)
+{
+const Elf_RelA *r;
+unsigned int symndx, i;
+uint64_t val;
+void *dest;
+
+for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ )
+{
+int err = 0;
+
+r = rela->data + i * rela->sec->sh_entsize;
+
+symndx = ELF64_R_SYM(r->r_info);
+
+if ( symndx > elf->nsym )
+{
+dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative relocation wants symbol@%u 
which is past end!\n",
+elf->name, symndx);
+return -EINVAL;
+}
+
+dest = base->load_addr + r->r_offset; /* P */
+val = elf->sym[symndx].sym->st_value +  r->r_addend; /* S+A */
+
+/* ARM64 operations at minimum are always 32-bit. */
+if ( r->r_offset >= base->sec->sh_size ||
+(r->r_offset + sizeof(uint32_t)) > base->sec->sh_size )
+goto bad_offset;
+
+switch ( ELF64_R_TYPE(r->r_info) ) {
+/* Data */
+case R_AARCH64_ABS64:
+if ( r->r_offset + sizeof(uint64_t) > base->sec->sh_size )
+goto bad_offset;


As you borrow the code from Linux, could we keep the abstraction with 
reloc_data and defer the overflow check? It would avoid to have the same 
if in multiple place in this code.



+*(int64_t *)dest = val;
+break;
+
+case R_AARCH64_ABS32:
+*(int32_t *)dest = val;
+if ( (int64_t)val !=  *(int32_t *)dest )
+err = -EOVERFLOW;
+break;
+
+case R_AARCH64_PREL64:
+if ( r->r_offset + sizeof(uint64_t) > base->sec->sh_size )
+goto bad_offset;
+
+val -= (uint64_t)dest;
+*(int64_t *)dest = val;
+break;
+
+case R_AARCH64_PREL32:
+val -= (uint64_t)dest;
+*(int32_t *)dest = val;
+if ( (int64_t)val !=  *(int32_t *)dest )
+err = -EOVERFLOW;
+break;
+
+/* Instructions. */
+case R_AARCH64_ADR_PREL_LO21:
+val -= (uint64_t)dest;
+err = reloc_insn_imm(dest, val, 0, 21, AARCH64_INSN_IMM_ADR);
+break;
+


It seems the implementation of R_AARCH64_ADR_PREL_PG_HI21_NC is missing.


+case R_AARCH64_ADR_PREL_PG_HI21:
+val = (val & ~0xfff) - ((u64)dest & ~0xfff);
+err = reloc_insn_imm(dest, val, 12, 21, AARCH64_INSN_IMM_ADR);
+break;


Hmmm, I think we want to avoid the payload having 
R_AARCH64_ADR_PREL_PG_HI21* relocations due to the erratum #843419 on 
some Cortex-A53.


I haven't yet looked at how you build the payload, but this may also 
affects the process to do it. I will give a look on it.



+
+case R_AARCH64_LDST8_ABS_LO12_NC:
+case R_AARCH64_ADD_ABS_LO12_NC:
+err = reloc_insn_imm(dest, val, 0, 12, AARCH64_INSN_IMM_12);
+if ( err == -EOVERFLOW )
+err = 0;
+break;
+
+case R_AARCH64_LDST16_ABS_LO12_NC:
+err = reloc_insn_imm(dest, val, 1, 11, AARCH64_INSN_IMM_12);
+if ( err == -EOVERFLOW )
+err 

Re: [Xen-devel] [PATCH v3 9/9] livepach: Add .livepatch.hooks functions and test-case

[Jan Beulich]

>>> On 15.08.16 at 16:46,  wrote:
> On Mon, Aug 15, 2016 at 05:15:28AM -0600, Jan Beulich wrote:
>> >>> On 14.08.16 at 23:52,  wrote:
>> > @@ -72,7 +73,11 @@ struct payload {
>> >  struct livepatch_build_id dep;   /* 
>> > ELFNOTE_DESC(.livepatch.depends). */
>> >  void *bss;   /* .bss of the payload. */
>> >  size_t bss_size; /* and its size. */
>> > -char name[XEN_LIVEPATCH_NAME_SIZE]; /* Name of it. */
>> > +livepatch_loadcall_t **load_funcs;   /* The array of funcs to call 
>> > after */
>> > +livepatch_unloadcall_t **unload_funcs;/* load and unload of the 
>> > payload. */
>> 
>> Considering above you said "Learned a lot of about 'const'", where
>> are they? (Interestingly, LIVEPATCH_{,UN}LOAD_HOOK() below look
>> correct now, so effectively you lose constness here.)
> 
> Can't do const here at all. Any placement of them will make the compile
> omit the call to them.
> 
> That is either one of:
> 
>  const livepatch_loadcall_t **load_funcs;
>  livepatch_loadcall_t const **load_funcs;

These two (functionally identical) variants aren't exhaustive for
"any placement"; the right one ought to be

 livepatch_loadcall_t *const *load_funcs;

(which is what I tried to clarify by saying "in the middle" in the original
reply).

Jan


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

Re: [Xen-devel] [PATCH v1 6/9] livepatch: Initial ARM64 support.

[Konrad Rzeszutek Wilk]

On Mon, Aug 15, 2016 at 04:57:26PM +0200, Julien Grall wrote:
> Hi Jan and Konrad,
> 
> On 15/08/2016 16:23, Jan Beulich wrote:
> > > > > On 15.08.16 at 16:09,  wrote:
> > > On Mon, Aug 15, 2016 at 02:21:48AM -0600, Jan Beulich wrote:
> > > > > > > On 15.08.16 at 01:07,  wrote:
> > > > > @@ -711,9 +711,15 @@ static int prepare_payload(struct payload 
> > > > > *payload,
> > > > >  return -EINVAL;
> > > > >  }
> > > > >  }
> > > > > +#ifndef CONFIG_ARM
> > > > >  apply_alternatives_nocheck(start, end);
> > > > > +#else
> > > > > +apply_alternatives(start, sec->sec->sh_size);
> > > > > +#endif
> > > > 
> > > > Conditionals like this are ugly - can't this be properly abstracted?
> > > 
> > > Yes, I can introduce an apply_alternatives_nocheck on ARM that will
> > > hava the same set of arguments on x86.
> > > 
> > > Or I can make a new function name?
> > 
> > Either way is fine with me, with a slight preference to the former
> > one.
> 
> I am fine with the prototype of the function apply_alternatives_nocheck but
> I don't think the name is relevant for ARM.
> 
> Is there any reason we don't want to call directly apply_alternatives in
> x86?

It assumes (and has an ASSERT) that it is called with interrupts disabled.
And we don't need to do that (as during livepatch loading we can modify the
livepatch payload without worrying about interrupts).

P.S.
loading != applying.

I could do a patch where we rename 'apply_alternatives' -> 
'apply_alternatives_boot'
and 'apply_alternatives_nocheck' to 'apply_alternatives'.

Also the x86 version instead of having:

apply_alternatives(struct alt_instr *start, struct alt_instr *end)

would end up with:

apply_alternatives(void *start, size_t length)

to comply with ARM version.
> 
> Regards,
> 
> -- 
> Julien Grall

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

Re: [Xen-devel] [PATCH v3 6/9] livepatch: Add parsing for the symbol+0x/

[Jan Beulich]

>>> On 15.08.16 at 16:35,  wrote:
> On Mon, Aug 15, 2016 at 04:53:48AM -0600, Jan Beulich wrote:
>> >>> On 14.08.16 at 23:52,  wrote:
>> > in case we want to patch at specific offsets inside
>> > a function. (for example if we want to do NOP patching).
>> > 
>> > We also assume that the 'len' is only the size of
>> > an isns that would be for a call opcode (so 5 bytes
>> > on x86, and 4 on ARM 32/64).
>> 
>> Which makes the notation using a slash quite confusing: Normally
>> if we see +/ the length part is assumed
>> to be the overall size of the object/function the name refers to.
> 
> Oh, I somehow had the length of instruction on my mind.
> 
>> Could I talk you into using a different separator, like colon or
>> semicolon?
> 
> Of course. Whatever is easier to folks.
> 
> I can also ditch the  part altogether?

The question is why you added it in the first place (see also the
comments on the other patch, where I'm puzzled by the restriction
to 5 bytes length).

Jan


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

Re: [Xen-devel] [RFC 00/22] xen/arm: Rework the P2M code to follow break-before-make sequence

[Edgar E. Iglesias]

On Thu, Jul 28, 2016 at 03:51:23PM +0100, Julien Grall wrote:
> Hello all,
> 
> The ARM architecture mandates the use of a break-before-make sequence when
> changing translation entries if the page table is shared between multiple
> CPUs whenever a valid entry is replaced by another valid entry (see D4.7.1
> in ARM DDI 0487A.j for more details).
> 
> The current P2M code does not respect this sequence and may result to
> break coherency on some processors.
> 
> Adapting the current implementation to use break-before-make sequence would
> imply some code duplication and more TLBs invalidations than necessary.
> For instance, if we are replacing a 4KB page and the current mapping in
> the P2M is using a 1GB superpage, the following steps will happen:
> 1) Shatter the 1GB superpage into a series of 2MB superpages
> 2) Shatter the 2MB superpage into a series of 4KB superpages
> 3) Replace the 4KB page
> 
> As the current implementation is shattering while descending and install
> the mapping before continuing to the next level, Xen would need to issue 3
> TLB invalidation instructions which is clearly inefficient.
> 
> Furthermore, all the operations which modify the page table are using the
> same skeleton. It is more complicated to maintain different code paths than
> having a generic function that set an entry and take care of the break-before-
> make sequence.
> 
> The new implementation is based on the x86 EPT one which, I think, fits
> quite well for the break-before-make sequence whilst keeping the code
> simple.
> 
> I sent this patch series as an RFC because there are still some TODOs
> in the code (mostly sanity check and possible optimization) and I have
> done limited testing. However, I think it is a good shape to start reviewing,
> get more feedback and have wider testing on different board.
> 
> Also, I need to figure out the impact on ARM32 because the domheap is not
> always mapped.
> 
> This series has dependencies on some rework sent separately ([1] and [2]).
> I have provided a branch with all the dependencies and this series applied:
> 
> git://xenbits.xen.org/people/julieng/xen-unstable.git branch p2m-rfc


Hi Julien,

FWIW, I gave this a spin on the ZynqMP and it seems to be working fine.
I tried dom0 and starting a few additional guests. All looks good.

Tested-by: Edgar E. Iglesias 

Cheers,
Edgar


> 
> Comments are welcome.
> 
> Yours sincerely,
> 
> Cc: Razvan Cojocaru 
> Cc: Tamas K Lengyel 
> Cc: Shanker Donthineni 
> Cc: Dirk Behme 
> Cc: Edgar E. Iglesias 
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg02936.html
> [2] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg02830.html
> 
> Julien Grall (22):
>   xen/arm: do_trap_instr_abort_guest: Move the IPA computation out of
> the switch
>   xen/arm: p2m: Store in p2m_domain whether we need to clean the entry
>   xen/arm: p2m: Rename parameter in p2m_{remove,write}_pte...
>   xen/arm: p2m: Use typesafe gfn in p2m_mem_access_radix_set
>   xen/arm: traps: Move MMIO emulation code in a separate helper
>   xen/arm: traps: Check the P2M before injecting a data/instruction
> abort
>   xen/arm: p2m: Rework p2m_put_l3_page
>   xen/arm: p2m: Invalidate the TLBs when write unlocking the p2m
>   xen/arm: p2m: Change the type of level_shifts from paddr_t to unsigned
> int
>   xen/arm: p2m: Move the lookup helpers at the top of the file
>   xen/arm: p2m: Introduce p2m_get_root_pointer and use it in
> __p2m_lookup
>   xen/arm: p2m: Introduce p2m_get_entry and use it to implement
> __p2m_lookup
>   xen/arm: p2m: Replace all usage of __p2m_lookup with p2m_get_entry
>   xen/arm: p2m: Re-implement p2m_cache_flush using p2m_get_entry
>   xen/arm: p2m: Re-implement relinquish_p2m_mapping using p2m_get_entry
>   xen/arm: p2m: Make p2m_{valid,table,mapping} helpers inline
>   xen/arm: p2m: Introduce a helper to check if an entry is a superpage
>   xen/arm: p2m: Introduce p2m_set_entry and __p2m_set_entry
>   xen/arm: p2m: Re-implement p2m_remove_using using p2m_set_entry
>   xen/arm: p2m: Re-implement p2m_insert_mapping using p2m_set_entry
>   xen/arm: p2m: Re-implement p2m_set_mem_access using
> p2m_{set,get}_entry
>   xen/arm: p2m: Do not handle shattering in p2m_create_table
> 
>  xen/arch/arm/domain.c  |8 +-
>  xen/arch/arm/p2m.c | 1274 
> ++--
>  xen/arch/arm/traps.c   |  126 +++--
>  xen/include/asm-arm/p2m.h  |   14 +
>  xen/include/asm-arm/page.h |4 +
>  5 files changed, 742 insertions(+), 684 deletions(-)
> 
> -- 
> 1.9.1
> 

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

Re: [Xen-devel] [PATCH v3 1/9] livepatch: Clear .bss when payload is reverted

[Jan Beulich]

>>> On 15.08.16 at 16:29,  wrote:
> On Mon, Aug 15, 2016 at 04:27:38AM -0600, Jan Beulich wrote:
>> >>> On 14.08.16 at 23:52,  wrote:
>> > @@ -1034,6 +1047,9 @@ static int revert_payload(struct payload *data)
>> >  list_del_rcu(>applied_list);
>> >  unregister_virtual_region(>region);
>> >  
>> > +/* And clear the BSS for subsequent operation. */
>> > +memset(data->bss, 0, data->bss_size);
>> 
>> Instead of doing it in two places, how about moving this into
>> apply_payload()?
> 
> I was thinking of it too, but then it occurred to me that between the
> 'check' -> 'apply' the BSS is left unitialized. And if we ever crash
> (right as somebody scheduled the livepatch) one could form the opinion
> that it is due to the .bss having garbage. Or more importantly - the
> hooks may not have run, but all its values looked like they have been
> initialized.
> 
> And spend considerable amount of time debugging something which in reality
> is not a problem?
> 
> Perhaps put it in multiple places :-)

Well, you and Ross know. Afaic I'd always do things in just one
place if that's possible.

Jan


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

Re: [Xen-devel] [PATCH 2/4] x86emul: drop RIP-relative special case for TEST

[Jan Beulich]

>>> On 15.08.16 at 16:25,  wrote:
> On 15/08/16 09:34, Jan Beulich wrote:
>> @@ -1851,11 +1911,6 @@ x86_emulate(
>>  ((op_bytes == 8) ? 4 : op_bytes);
>>  else if ( (d & SrcMask) == SrcImmByte )
>>  ea.mem.off += 1;
>> -else if ( !ext && ((b & 0xfe) == 0xf6) &&
>> -  ((modrm_reg & 7) <= 1) )
> 
> Do we actually handle these cases correctly?  0xf6 /0 (imm8) and 0xf7 /0
> (imm) look to work as expected

I think we do; what makes you think we might not?

> However, 0xf6 /1, 0xf7 /1 are harder to pin down.  We have an
> implementation of it, but the only other reference I can find to them
> are in the AMD grp3 opcode map, where they appear equal to their /0
> variants.  The /1 variants do not appear in the AMD description of the
> TEST instruction, and do not appear anywhere in the Intel manuals.
> 
> Suravee: Can you confirm whether the /1 variants are expected to be
> implemented and copies of the /0 variants?

I've check on Intel systems that they are aliases (Intel doesn't
document them), and AMD halfway documenting them as aliases
makes me assume they indeed are. Other than opcode 82 they're
also aliases regardless of whether in 64-bit mode (on Intel again,
that is, I can't get to my AMD boxes right now).

Jan


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

Re: [Xen-devel] [PATCH 2/2] xen/events: Convert to hotplug state machine

[David Vrabel]

On 15/08/16 15:46, Boris Ostrovsky wrote:
> From: Sebastian Andrzej Siewior 
> 
> Install the callbacks via the state machine.
[...]
> +static int xen_evtchn_cpu_dead(unsigned int cpu)
> +{
> + __evtchn_fifo_handle_events(cpu, true);
> + return 0;
> +}

I'm not familiar with the new state machine.  When this is called, what
state is the CPU in?

In particular, local interrupts must be disabled and all non-percpu irqs
must have been migrated to other CPUs.


>  int __init xen_evtchn_fifo_init(void)
>  {
> @@ -456,7 +444,9 @@ int __init xen_evtchn_fifo_init(void)
>  
>   evtchn_ops = _ops_fifo;
>  
> - register_cpu_notifier(_fifo_cpu_notifier);
> + cpuhp_setup_state_nocalls(CPUHP_XEN_EVTCHN_PREPARE,
> +   "CPUHP_XEN_EVTCHN_PREPARE",
> +   xen_evtchn_cpu_prepare, xen_evtchn_cpu_dead);
>  out:
>   put_cpu();
>   return ret;
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index d6beeb9..c60a17c 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -22,6 +22,7 @@ enum cpuhp_state {
>   CPUHP_SMPCFD_PREPARE,
>   CPUHP_RCUTREE_PREP,
>   CPUHP_XEN_PREPARE,
> + CPUHP_XEN_EVTCHN_PREPARE,
>   CPUHP_NOTIFY_PREPARE,
>   CPUHP_TIMERS_DEAD,
>   CPUHP_BRINGUP_CPU,
> 


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

Re: [Xen-devel] [Very RFC PATCH 3/3] livepatch: Initial ARM32/64 support.

[Konrad Rzeszutek Wilk]

On Mon, Aug 15, 2016 at 03:27:08PM +0200, Julien Grall wrote:
> Hi Konrad,
> 
> On 12/08/2016 22:50, Konrad Rzeszutek Wilk wrote:
> > On Fri, Aug 12, 2016 at 05:00:47PM +0200, Julien Grall wrote:
> > > > diff --git a/xen/include/asm-arm/current.h 
> > > > b/xen/include/asm-arm/current.h
> > > > index 65c0cdf..f4fcfd6 100644
> > > > --- a/xen/include/asm-arm/current.h
> > > > +++ b/xen/include/asm-arm/current.h
> > > > @@ -33,8 +33,15 @@ static inline struct cpu_info *get_cpu_info(void)
> > > > 
> > > >  #define guest_cpu_user_regs() (_cpu_info()->guest_cpu_user_regs)
> > > > 
> > > > +#ifdef CONFIG_LIVEPATCH
> > > > +#define switch_stack_and_jump(stack, fn)   
> > > >  \
> > > > +asm volatile ("mov sp,%0;" 
> > > >  \
> > > > +  "bl check_for_livepatch_work;"   
> > > >  \
> > > 
> > > May I ask why check_for_livepatch_work is called in switch_stack_and_jump?
> > 
> > From 29f4ab0b0a4ff62589af35b7cbc2334e1d9acdcd:
> > To perform and action on a payload, the hypercall sets up a data
> > structure to schedule the work.  A hook is added in the 
> > reset_stack_and_jump
> > to check for work and execute it if needed (specifically we check an
> > per-cpu flag to make this as quick as possible).
> > 
> > In this way, patches can be applied with all CPUs idle and without
> > stacks.  The first CPU to run check_for_xsplice_work() becomes the
> > master and triggers a reschedule softirq to trigger all the other CPUs
> > to enter check_for_xsplice_work() with no stack.  Once all CPUs
> > have rendezvoused, all CPUs disable their IRQs and NMIs are ignored.
> > The system is then quiscient and the master performs the action.
> > After this, all CPUs enable IRQs and NMIs are re-enabled.
> 
> 
> I am a bit confused, switch_stack_and_jump will only be called on ARM when a
> vCPU is created. So why do you want to check livepatch at that time?
> 
> After looking at the x86 version, I noticed that reset_stack_and_jump is
> called every time Xen returns to the guest, correct?

Yes. I assumed ARM was the same.
> 
> If so, you may want to call check_for_livepatch_work in
> leave_hypervisor_tail instead. The latter will be call every time Xen
> returns to the guest.

Oooh. Thanks!
> 
> Regards,
> 
> -- 
> Julien Grall

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

[Xen-devel] [PATCH] xenbus: don't look up transaction IDs for ordinary writes

[Jan Beulich]

This should really only be done for XS_TRANSACTION_END messages, or
else at least some of the xenstore-* tools don't work anymore.

Fixes: 0beef634b8 ("xenbus: don't BUG() on user mode induced condition")
Reported-by: Richard Schütz 
Cc: 
Signed-off-by: Jan Beulich 
Tested-by: Richard Schütz 
---
 drivers/xen/xenbus/xenbus_dev_frontend.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- 4.8-rc2/drivers/xen/xenbus/xenbus_dev_frontend.c
+++ 
4.8-rc2-xen-xenbus-dev-write-oridinary/drivers/xen/xenbus/xenbus_dev_frontend.c
@@ -316,7 +316,7 @@ static int xenbus_write_transaction(unsi
rc = -ENOMEM;
goto out;
}
-   } else {
+   } else if (msg_type == XS_TRANSACTION_END) {
list_for_each_entry(trans, >transactions, list)
if (trans->handle.id == u->u.msg.tx_id)
break;



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

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[George Dunlap]

On 15/08/16 12:20, Jan Beulich wrote:
 On 15.08.16 at 12:47,  wrote:
>> On 15/08/16 11:19, Jan Beulich wrote:
>>> Well, none of the options considered so far are really nice or
>>> readily available. I think the easiest to use for both the caller and
>>> the implementation of the hypercall would be the auxiliary
>>> hypercall for a kernel to indicate user/kernel boundaries plus a
>>> flag on the DMOP one for the kernel mode driver to indicate its
>>> user mode origin. The main (purely theoretical afaict) downside
>>> of this is the difficulty to use it in OSes with variable user/kernel
>>> boundaries.
>>
>> What about including in the "fixed" part of the hypercall a virtual
>> address range that all pointers must be in?  That wouldn't even require
>> a user/kernel flag actually; and could conceivably be used by the caller
>> (either userspace or kernel space) to thwart certain kinds of potential
>> attacks.
> 
> That's definitely an option, if we're sufficiently certain that no OSes
> will ever require two or more ranges.

I'm sorry, I think this is getting a bit silly.  There are currently no
known operating systems which have discontinuous user-space virtual
address ranges.  Even if there were, the hypercall structure I'm
proposing would still function; the only restriction would be that any
single hypercall would have to have all arguments within one of those
ranges.

If we find such a monster in the future, we can try to figure out how to
accommodate it at that point.

Another idea I had was to do a multi-call-style hypercall "wrapper"
hypercall, similar to David's XSM idea, but instead of running with a
specific XSM label, would restrict the target domain and VA range of all
included hypercalls.  Then the individual hypercall structure wouldn't
need to be known at all; and if it ever became important to have (say)
two VA ranges, we could simply add a new "wrapper" hypercall.

 -George

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

Re: [Xen-devel] [PATCH v1 6/9] livepatch: Initial ARM64 support.

[Julien Grall]

Hi Jan and Konrad,

On 15/08/2016 16:23, Jan Beulich wrote:

On 15.08.16 at 16:09,  wrote:

On Mon, Aug 15, 2016 at 02:21:48AM -0600, Jan Beulich wrote:

On 15.08.16 at 01:07,  wrote:

@@ -711,9 +711,15 @@ static int prepare_payload(struct payload *payload,
 return -EINVAL;
 }
 }
+#ifndef CONFIG_ARM
 apply_alternatives_nocheck(start, end);
+#else
+apply_alternatives(start, sec->sec->sh_size);
+#endif


Conditionals like this are ugly - can't this be properly abstracted?


Yes, I can introduce an apply_alternatives_nocheck on ARM that will
hava the same set of arguments on x86.

Or I can make a new function name?


Either way is fine with me, with a slight preference to the former
one.


I am fine with the prototype of the function apply_alternatives_nocheck 
but I don't think the name is relevant for ARM.


Is there any reason we don't want to call directly apply_alternatives in 
x86?


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 3/3] Significant changes to decision making; some new roles and minor changes

[Lars Kurth]

Hi Tim,

>At 00:13 +0100 on 12 Aug (1470960826), Lars Kurth wrote:
>> +### Conflict Resolution {#conflict}
>> +
>> +Sub-projects and teams hosted on Xenproject.org are not democracies
>>but 
>> +meritocracies. In situations where there is disagreement on issues
>>related to 
>> +the day-to-day running of the project, the [project leadership
>> +team](#leadership) is expected to act as referee and make a decision
>>on behalf 
>> +of the community. Projects leadership teams can choose to delegate
>>entire 
>> +classes of conflict resolution issues to community members and/or the
>>project 
>> +lead (e.g. the project can choose to delegate refereeing on committer
>> +disagreements to the project lead; or it could choose a specific
>>committer to 
>> +always act as referee amongst a group of committers). Any such
>>delegation needs 
>> +to be approved as normal and has to be documented.
>> +
>> +Should a project leadership team become dysfunctional or paralysed,
>>the project 
>> +leadership team or project lead should work with the community manager
>>or 
>> +advisory board to find a way forward.
>> +
>> +In situations where there is significant disagreement between
>>sub-projects, the
>> +issue is deferred to the [Xen Project Advisory Board](/join.html).
>
>This looks like a pretty significant shift of responsibilty to the AB.
>As I read it, the current governance requires a _vote_ if subprojects
>disagree, with the AB only called to break a tie.
>
>It also seems to conflict with the wording that the AB "leaves all
>technical decisions to the open source meritocracy".
>
>IMO if this is to be changed it should be to something more concrete
>than "significant disagreement".

That was not intentional. It crept in, because I wanted to avoid repeating
the phrase I used in the previous paragraph, purely for style reasons.

A bit of background on my thinking
A) The AB never felt comfortable with the tie-breaker scenario
B) The new voting model doesn't require the AB to be a tie maker any more
C) It does spell out the areas where AB sign-off is needed regardless of
   Community votes more clearly (in practice mostly it will primarily in
   areas where funds are needed to implement something)

   See your comment below

D) Also, from a scope perspective, global votes would only ever be about
   non-technical issues, such as policy

But I see your point. The text should really have said something like...
-
In situations where the entire Xen Project community becomes paralysed,
the project leaderships team or project lead should work with the
community 
manager or advisory board to find a way forward.
-


It would be nice to list an examples of "becoming paralysed", but
I can't think of anything.



>> +-   Some sections of this document such as [Xen Project wide
>> +roles](#roles-global) and [making contributions](#contributions)
>>**cannot be 
>> +changed by the community** without obtaining additional approval from
>>the 
>> +Advisory Board and/or the Linux Foundation, if these conflict
>>requirements that
>> +stem from being part of a Linux Foundation Collaborative Project (e.g
>>requiring 
>> +a contributor license agreement). Areas with such requirements cover
>> +trademarks, legal oversight, financial oversight and project funding.
>
>Again, this is a change from the current governance, which just
>delegates those things to the AB and leaves it at that (with the
>implication that the project as a whole controls its own governance).

I can see how this comes across. I will lay out my thoughts after answering
your other concerns.

>IMO it would be better to leave the AB wording as it is, and refer to
>a _specific_ LF policy document in the section on the LF.

I am lost now: there is not much wording related to the Advisory Board
in the original governance at all (except where the AB is defined). I
could 
take this entire paragraph out, as in fact we did not have it and we
Managed well. In practice, people would just come to me when there were
grey 
areas. 

>
>Or if people want a section like this then it should be a clear list
>of exactly which things require approval from which bodies, with no
>"such as" or similar, so there is no confusion later.

That's a problem, because there are no public documents listing these.
For example, there is no published document which says, collaborative
Projects must not have a CLA. But we were told that me must never
introduce one, when we became an LF project.

I put this section in, because in practice community members do tend
to come to me (as member of the AB) when it comes to funding stuff: e.g.
build and CI infra for the Win PV driver project, ... but these were
project local things. And we had past instances when an AB member
raised concrete issues (e.g. in 2012 a number of contributors were really
Unhappy that we didn't have a release and roadmap process). But in
hindsight, 
This paragraph doesn't add much and isn't really needed.

I think we have two options:

[Xen-devel] [XTF PATCH v3] xtf-runner: support two modes for getting output

[Wei Liu]

We need two modes for getting output:

1. Use console directly with newer (>=4.8) Xen
2. Use log files for older Xen

This patch implements both. The default behaviour is to use the console
mode.

Signed-off-by: Wei Liu 
---
v3:
1. use os module for opening log file
2. poll up to 1 second for final output in logfile mode

v2: delete auto selection mode, squash all patches into one.
---
 xtf-runner | 119 +
 1 file changed, 97 insertions(+), 22 deletions(-)

diff --git a/xtf-runner b/xtf-runner
index c063699..96a5ba0 100755
--- a/xtf-runner
+++ b/xtf-runner
@@ -7,7 +7,7 @@
 Currently assumes the presence and availability of the `xl` toolstack.
 """
 
-import sys, os, os.path as path
+import sys, os, os.path as path, signal
 
 from optparse import OptionParser
 from subprocess import Popen, PIPE, call as subproc_call, check_output
@@ -425,30 +425,19 @@ def list_tests(opts):
 for sel in opts.selection:
 print sel
 
+def run_test_console(opts, test):
+""" Run a specific test via xenconsole"""
 
-def run_test(test):
-""" Run a specific test """
-
-cmd = ['xl', 'create', '-p', test.cfg_path()]
-print "Executing '%s'" % (" ".join(cmd), )
-rc = subproc_call(cmd)
-if rc:
-raise RunnerError("Failed to create VM")
-
-cmd = ['xl', 'console', test.vm_name()]
+cmd = ['xl', 'create', '-Fc', test.cfg_path()]
 print "Executing '%s'" % (" ".join(cmd), )
-console = Popen(cmd, stdout = PIPE)
+guest = Popen(cmd, stdout = PIPE, stderr = PIPE)
 
-cmd = ['xl', 'unpause', test.vm_name()]
-print "Executing '%s'" % (" ".join(cmd), )
-rc = subproc_call(cmd)
-if rc:
-raise RunnerError("Failed to unpause VM")
+# stdout is console output, stderr is xl output
+stdout, stderr = guest.communicate()
 
-stdout, _ = console.communicate()
-
-if console.returncode:
-raise RunnerError("Failed to obtain VM console")
+if guest.returncode:
+print stderr
+raise RunnerError("Failed to communicate with guest")
 
 lines = stdout.splitlines()
 
@@ -470,6 +459,59 @@ def run_test(test):
 return "ERROR"
 
 
+should_exit = False
+def sigalrm_handler(signum, frame):
+"""Signal handler for SIGALRM"""
+
+global should_exit
+should_exit = True
+
+def run_test_logfile(opts, test):
+""" Run a specific test via grepping log file"""
+
+global should_exit
+fn = opts.logfile_dir + (opts.logfile_pattern % test)
+
+print "Using %s" % fn
+
+fd = os.open(fn, os.O_CREAT|os.O_RDONLY, 0644)
+logfile = os.fdopen(fd)
+logfile.seek(0, os.SEEK_END)
+
+cmd = ['xl', 'create', '-F', test.cfg_path()]
+print "Executing '%s'" % (" ".join(cmd), )
+rc = subproc_call(cmd)
+if rc:
+raise RunnerError("Failed to run test")
+
+signal.signal(signal.SIGALRM, sigalrm_handler)
+
+lines = []
+should_exit = False
+signal.alarm(1)
+while not should_exit:
+line = logfile.readline()
+lines.append(line)
+if "Test result:" in line:
+should_exit = True
+signal.alarm(0)
+
+logfile.close()
+
+print "".join(lines)
+
+test_result = lines[-1]
+if not "Test result:" in test_result:
+return "ERROR"
+
+for res in all_results:
+
+if res in test_result:
+return res
+
+return "ERROR"
+
+
 def run_tests(opts):
 """ Run tests """
 
@@ -477,12 +519,21 @@ def run_tests(opts):
 if not len(tests):
 raise RunnerError("No tests to run")
 
+if opts.mode == "console":
+run_test = run_test_console
+elif opts.mode == "logfile":
+print "Using logfile mode, please make sure the log \
+files are not rotated!"
+run_test = run_test_logfile
+else:
+raise RunnerError("Unrecognised mode")
+
 rc = all_results.index('SUCCESS')
 results = []
 
 for test in tests:
 
-res = run_test(test)
+res = run_test(opts, test)
 res_idx = all_results.index(res)
 if res_idx > rc:
 rc = res_idx
@@ -519,6 +570,17 @@ def main():
   "  all tests in the selection, printing a summary of their\n"
   "  results at the end.\n"
   "\n"
+  "  To determine how runner should get output from Xen, use\n"
+  "  --mode option. The default value is \"console\", which\n"
+  "  means using xenconsole program to extract output.\n"
+  "  The other supported value is \"logfile\", which\n"
+  "  means to get output from log file.\n"
+  "\n"
+  "  The \"logfile\" mode requires users to configure\n"
+  "  xenconsoled to log guest console output. This mode\n"
+  "  is useful for Xen version < 4.8. Also see --logfile-dir\n"
+  "  and --logfile-pattern options.\n"
+   

Re: [Xen-devel] [PATCH v1] Livepatch ARM 64 implementation

[Julien Grall]

On 15/08/2016 01:07, Konrad Rzeszutek Wilk wrote:

Hey!


Hi Konrad,


This is the first (non RFC) posting of the enablement of Livepatch under ARM64.

The patches are based on: [PATCH v3] Livepatch fixes and features for v4.8.
(https://lists.xen.org/archives/html/xen-devel/2016-08/msg01825.html)

And the git tree is:
 git://xenbits.xen.org/people/konradwilk/xen.git livepatch.v4.8.v3

I've only tested this under Foundation Platform with only one CPU working
(the other CPUs wouldn't boot up for some reason) and without a proper working
disk image (can't recall why, but it did have an initramfs) - I ended
up building the hypervisor with the livepatch built in and loading it during
dom0 execution (via timers), see
(http://xenbits.xen.org/gitweb/?p=people/konradwilk/xen.git;a=commit;h=39517b2b807025d0d63d4f042ada5eb3de32ff45)
[That patch is not part of this patchset of course]


I am able to use both SMP and the rootfs on the foundation model. What 
is hte command line you are using? How about the device tree?


Cheers,

--
Julien Grall

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

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[David Vrabel]

On 09/08/16 11:29, Jan Beulich wrote:
 On 08.08.16 at 15:46,  wrote:
>> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
>> depriv)"):
>>> On 05.08.16 at 18:28,  wrote:
 That is, a bug of class 2 would allow the unprivileged qemu process in
 dom0 to cause damage to other parts of dom0.
>> ...
>>> Ah, okay, I think I finally understand. [...]
>>>
>>> I'm having, however, a hard time imagining a class 2 bug for any
>>> of the hvmop-s that are being converted by the hvmctl series:
>>> These act on the target domain, so would not touch the calling
>>> ones state other than for copying argument structures to/from
>>> guest memory (and I don't view mixing up domain pointers as
>>> a likely source of problems).
>>
>> Right.  AIUI all the hypercall arguments are passed using
>> calling-guest-virtual addresses, which the dom0 privcmd arrangements
>> are capable of ensuring are sane.
> 
> Actually, having thought about this some more, and taking this
> together with the expectations to the privcmd driver previously
> outlined, I think this part is problematic: If all the driver is to know
> is the position (within the interface structure) of the target domain
> ID, then any guest handles embedded in the interface structure
> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
> validated, and hence user mode code would have a way to access
> or modify kernel memory.

It seems simpler to me to have in the privcmd driver:

if (op == HVMCTL_track_dirty_vram)
ret = access_ok(...);

It looks simpler to me to fix the ABI and add the checking in the
privcmd driver than add one of the proposed mechanisms to allow the
hypervisor to do the checking.

To avoid the issues with having to update the kernel in lock-step with
the hypervisor (if new checks are needed in privcmd), we can (in the
common case) do version the checking in the driver.

i.e., if the privcmd drivers knows about hypervisor ABI V but qemu needs
V+1 then it can choose to disable the depriv and thus continue to work
(with reduced defense in depth).

David

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

Re: [Xen-devel] [PATCH v3 9/9] livepach: Add .livepatch.hooks functions and test-case

[Konrad Rzeszutek Wilk]

On Mon, Aug 15, 2016 at 05:15:28AM -0600, Jan Beulich wrote:
> >>> On 14.08.16 at 23:52,  wrote:
> > v4..v11: Defered for v4.8
> > v12: s/xsplice/livepatch/
> > v13: Clarify the comments about spin_debug_enable
> 
> (Side note: v13 here vs v3 in the subject.)
> 
> >  Rename one of the hooks to lower-case (Z->z) to guarantee it being
> >  called last.
> 
> Does lower case z really guarantee that? Wouldn't it be better to
> use a sort order independent mechanism, like using another object
> file (iirc object file order defines placement-within-sections unless
> options get handed to the linker which specifically allow it to
> shuffle things around)?
> 
> > @@ -72,7 +73,11 @@ struct payload {
> >  struct livepatch_build_id dep;   /* 
> > ELFNOTE_DESC(.livepatch.depends). */
> >  void *bss;   /* .bss of the payload. */
> >  size_t bss_size; /* and its size. */
> > -char name[XEN_LIVEPATCH_NAME_SIZE]; /* Name of it. */
> > +livepatch_loadcall_t **load_funcs;   /* The array of funcs to call 
> > after */
> > +livepatch_unloadcall_t **unload_funcs;/* load and unload of the 
> > payload. */
> 
> Considering above you said "Learned a lot of about 'const'", where
> are they? (Interestingly, LIVEPATCH_{,UN}LOAD_HOOK() below look
> correct now, so effectively you lose constness here.)

Can't do const here at all. Any placement of them will make the compile
omit the call to them.

That is either one of:

 const livepatch_loadcall_t **load_funcs;
 livepatch_loadcall_t const **load_funcs;

results in (on gcc-5.2.1):
   .loc 1 1152 0
callspin_debug_disable
.LVL69: 
.loc 1 1153 0
movl324(%r12), %edx
testl   %edx, %edx
je  .L32
movl$0, %eax
.LVL70:
.L33:   
.loc 1 1153 0 is_stmt 0 discriminator 3
addl$1, %eax
.LVL71: 
cmpl%edx, %eax
jne .L33
.LVL72:
.L32:   
.loc 1 1155 0 is_stmt 1
callspin_debug_enable

(on older compiler 4.4.4) I get:
.L122:
.loc 1 1108 0
callspin_debug_disable
.LVL175:
.loc 1  0
.p2align 4,,8
callspin_debug_enable


> 
> > @@ -1065,6 +1089,18 @@ static int apply_payload(struct payload *data)
> >  
> >  arch_livepatch_quiesce();
> >  
> > +/*
> > + * Since we are running with IRQs disabled and the hooks may call 
> > common
> > + * code - which expects the spinlocks to run with IRQs enabled - we 
> > temporarly
> > + * disable the spin locks IRQ state checks.
> > + */
> 
> Much better, but a little further to go: I'd suggest
> s/the spinlocks/certain spinlocks/ or some such. Also - "temporarily".
> 
> Jan
> 

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

[Xen-devel] [PATCH 1/2] xen/x86: Convert to hotplug state machine

[Boris Ostrovsky]

Switch to new CPU hotplug infrastructure.

Signed-off-by: Boris Ostrovsky 
Suggested-by: Sebastian Andrzej Siewior 
---
 arch/x86/xen/enlighten.c   |  115 +---
 include/linux/cpuhotplug.h |2 +
 2 files changed, 67 insertions(+), 50 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c7f6b1f..2283976 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -140,7 +140,9 @@ RESERVE_BRK(shared_info_page_brk, PAGE_SIZE);
 __read_mostly int xen_have_vector_callback;
 EXPORT_SYMBOL_GPL(xen_have_vector_callback);
 
-static struct notifier_block xen_cpu_notifier;
+static int xen_cpu_up_prepare(unsigned int cpu);
+static int xen_cpu_up_online(unsigned int cpu);
+static int xen_cpu_up_cancel(unsigned int cpu);
 
 /*
  * Point at some empty memory to start with. We map the real shared_info
@@ -1541,6 +1543,24 @@ static void __init xen_dom0_set_legacy_features(void)
x86_platform.legacy.rtc = 1;
 }
 
+static int xen_cpuhp_setup(void)
+{
+   int rc;
+
+   rc = cpuhp_setup_state_nocalls(CPUHP_XEN_PREPARE,
+  "XEN_HVM_GUEST_PREPARE",
+  xen_cpu_up_prepare, xen_cpu_up_cancel);
+   if (!rc) {
+   rc = cpuhp_setup_state_nocalls(CPUHP_AP_XEN_ONLINE,
+  "XEN_HVM_GUEST_PREPARE",
+  xen_cpu_up_online, NULL);
+   if (rc)
+   cpuhp_remove_state_nocalls(CPUHP_XEN_PREPARE);
+   }
+
+   return rc;
+}
+
 /* First C function to be called on Xen boot */
 asmlinkage __visible void __init xen_start_kernel(void)
 {
@@ -1629,7 +1649,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
xen_initial_gdt = _cpu(gdt_page, 0);
 
xen_smp_init();
-   register_cpu_notifier(_cpu_notifier);
+   WARN_ON(xen_cpuhp_setup());
 
 #ifdef CONFIG_ACPI_NUMA
/*
@@ -1823,63 +1843,58 @@ static void __init init_hvm_pv_info(void)
xen_domain_type = XEN_HVM_DOMAIN;
 }
 
-static int xen_cpu_notify(struct notifier_block *self, unsigned long action,
-void *hcpu)
+static int xen_cpu_up_prepare(unsigned int cpu)
 {
-   int cpu = (long)hcpu;
int rc;
 
-   switch (action) {
-   case CPU_UP_PREPARE:
-   if (xen_hvm_domain()) {
-   /*
-* This can happen if CPU was offlined earlier and
-* offlining timed out in common_cpu_die().
-*/
-   if (cpu_report_state(cpu) == CPU_DEAD_FROZEN) {
-   xen_smp_intr_free(cpu);
-   xen_uninit_lock_cpu(cpu);
-   }
-
-   if (cpu_acpi_id(cpu) != U32_MAX)
-   per_cpu(xen_vcpu_id, cpu) = cpu_acpi_id(cpu);
-   else
-   per_cpu(xen_vcpu_id, cpu) = cpu;
-   xen_vcpu_setup(cpu);
+   if (xen_hvm_domain()) {
+   /*
+* This can happen if CPU was offlined earlier and
+* offlining timed out in common_cpu_die().
+*/
+   if (cpu_report_state(cpu) == CPU_DEAD_FROZEN) {
+   xen_smp_intr_free(cpu);
+   xen_uninit_lock_cpu(cpu);
}
 
-   if (xen_pv_domain() ||
-   (xen_have_vector_callback &&
-xen_feature(XENFEAT_hvm_safe_pvclock)))
-   xen_setup_timer(cpu);
+   if (cpu_acpi_id(cpu) != U32_MAX)
+   per_cpu(xen_vcpu_id, cpu) = cpu_acpi_id(cpu);
+   else
+   per_cpu(xen_vcpu_id, cpu) = cpu;
+   xen_vcpu_setup(cpu);
+   }
 
-   rc = xen_smp_intr_init(cpu);
-   if (rc) {
-   WARN(1, "xen_smp_intr_init() for CPU %d failed: %d\n",
-cpu, rc);
-   return NOTIFY_BAD;
-   }
+   if (xen_pv_domain() ||
+   (xen_have_vector_callback &&
+xen_feature(XENFEAT_hvm_safe_pvclock)))
+   xen_setup_timer(cpu);
 
-   break;
-   case CPU_ONLINE:
-   xen_init_lock_cpu(cpu);
-   break;
-   case CPU_UP_CANCELED:
-   xen_smp_intr_free(cpu);
-   if (xen_pv_domain() ||
-   (xen_have_vector_callback &&
-xen_feature(XENFEAT_hvm_safe_pvclock)))
-   xen_teardown_timer(cpu);
-   break;
-   default:
-   break;
+   rc = xen_smp_intr_init(cpu);
+   if (rc) {
+   WARN(1, "xen_smp_intr_init() for CPU %d failed: %d\n",
+cpu, rc);
+ 

Re: [Xen-devel] [PATCH v3 7/9] livepatch: NOP if func->new_[addr, size] is zero.

[Konrad Rzeszutek Wilk]

On Mon, Aug 15, 2016 at 04:59:52AM -0600, Jan Beulich wrote:
> >>> On 14.08.16 at 23:52,  wrote:
> > The NOP functionality will NOP any of the code at
> > the 'old_addr' or at 'name' if the 'new_addr' and 'new_size'
> > are both zero. The purpose of this is to NOP out calls, such as:
> > 
> >  e9 <4-bytes-offset>
> 
> Except that E9 is JMP; CALL is E8.
> 
> > (5 byte insn), or on ARM a 4 byte insn for branching.
> > 
> > We need the EIP of where we need to the NOP, and that can
> > be provided via the `old_addr` or `name`.
> > 
> > If the `old_addr` is provided we will NOP
> > 5 instructions (on x86) at that location.
> > 
> > If `name` is provided with the symbol+0x
> > we make sure that  is 5 (on x86) and upon retrieving the
> > EIP based on `name` will NOP that location.
> 
> So why does this need to be restricted to 5-byte (on x86) code
> blocks? I.e. what's wrong with NOP-ing out other code.

It can most certainly nop variable sizes. I will have to update
the design to make it clear that if 'new_addr' is zero then we will
NOP (and the .new_size will determine the amount of NOPs to sprinkle).

Let me do that along with your comments. Thanks!
> 
> > @@ -46,18 +42,27 @@ int arch_livepatch_verify_func(const struct 
> > livepatch_func *func)
> >  
> >  void arch_livepatch_apply_jmp(struct livepatch_func *func)
> >  {
> > -int32_t val;
> >  uint8_t *old_ptr;
> > +uint8_t insn[PATCH_INSN_SIZE];
> >  
> >  BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->opaque));
> > -BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof(val)));
> >  
> >  old_ptr = func->old_addr;
> >  memcpy(func->opaque, old_ptr, PATCH_INSN_SIZE);
> >  
> > -*old_ptr++ = 0xe9; /* Relative jump */
> > -val = func->new_addr - func->old_addr - PATCH_INSN_SIZE;
> > -memcpy(old_ptr, , sizeof(val));
> > +if ( func->new_size )
> > +{
> > +int32_t val;
> > +
> > +BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof(val)));
> > +
> > +insn[0] = 0xe9;
> > +val = func->new_addr - func->old_addr - PATCH_INSN_SIZE;
> > +memcpy([1], , sizeof(val));
> > +} else
> 
> Style.
> 
> > --- a/xen/common/livepatch.c
> > +++ b/xen/common/livepatch.c
> > @@ -561,11 +561,15 @@ static int prepare_payload(struct payload *payload,
> >  return -EOPNOTSUPP;
> >  }
> >  
> > -if ( !f->new_addr || !f->new_size )
> > +/* If both are zero then we are NOPing. */
> > +if ( (!f->new_addr || !f->new_size) )
> 
> Comment and condition contradict one another. And you're adding
> unnecessary parentheses.
> 
> Jan
> 

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

[Xen-devel] [PATCH 2/2] xen/events: Convert to hotplug state machine

[Boris Ostrovsky]

From: Sebastian Andrzej Siewior 

Install the callbacks via the state machine.

Signed-off-by: Sebastian Andrzej Siewior 
Signed-off-by: Boris Ostrovsky 
---
Changes from Sebastian's original version:
* Renamed hotplug state from CPUHP_XEN_EV_PREPEARE to CPUHP_XEN_EVTCHN_PREPARE
* Dropped suggestion in the commit messages about moving 
evtchn_fifo_alloc_control_block
  call since I think it is in the right place right now.


 drivers/xen/events/events_fifo.c |   34 --
 include/linux/cpuhotplug.h   |1 +
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/drivers/xen/events/events_fifo.c b/drivers/xen/events/events_fifo.c
index 266c2c7..7ef27c6 100644
--- a/drivers/xen/events/events_fifo.c
+++ b/drivers/xen/events/events_fifo.c
@@ -418,30 +418,18 @@ static int evtchn_fifo_alloc_control_block(unsigned cpu)
return ret;
 }
 
-static int evtchn_fifo_cpu_notification(struct notifier_block *self,
- unsigned long action,
- void *hcpu)
+static int xen_evtchn_cpu_prepare(unsigned int cpu)
 {
-   int cpu = (long)hcpu;
-   int ret = 0;
-
-   switch (action) {
-   case CPU_UP_PREPARE:
-   if (!per_cpu(cpu_control_block, cpu))
-   ret = evtchn_fifo_alloc_control_block(cpu);
-   break;
-   case CPU_DEAD:
-   __evtchn_fifo_handle_events(cpu, true);
-   break;
-   default:
-   break;
-   }
-   return ret < 0 ? NOTIFY_BAD : NOTIFY_OK;
+   if (!per_cpu(cpu_control_block, cpu))
+   return evtchn_fifo_alloc_control_block(cpu);
+   return 0;
 }
 
-static struct notifier_block evtchn_fifo_cpu_notifier = {
-   .notifier_call  = evtchn_fifo_cpu_notification,
-};
+static int xen_evtchn_cpu_dead(unsigned int cpu)
+{
+   __evtchn_fifo_handle_events(cpu, true);
+   return 0;
+}
 
 int __init xen_evtchn_fifo_init(void)
 {
@@ -456,7 +444,9 @@ int __init xen_evtchn_fifo_init(void)
 
evtchn_ops = _ops_fifo;
 
-   register_cpu_notifier(_fifo_cpu_notifier);
+   cpuhp_setup_state_nocalls(CPUHP_XEN_EVTCHN_PREPARE,
+ "CPUHP_XEN_EVTCHN_PREPARE",
+ xen_evtchn_cpu_prepare, xen_evtchn_cpu_dead);
 out:
put_cpu();
return ret;
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index d6beeb9..c60a17c 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -22,6 +22,7 @@ enum cpuhp_state {
CPUHP_SMPCFD_PREPARE,
CPUHP_RCUTREE_PREP,
CPUHP_XEN_PREPARE,
+   CPUHP_XEN_EVTCHN_PREPARE,
CPUHP_NOTIFY_PREPARE,
CPUHP_TIMERS_DEAD,
CPUHP_BRINGUP_CPU,
-- 
1.7.1


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

[Xen-devel] [PATCH 0/2] Convert to new CPU hotplug framework

[Boris Ostrovsky]

New CPU hotplug framework was introduced recently. These patches convert Xen
CPU hotplug code to this infrastructure.

The patches (patch 1 specifically) will apply on top of
  https://lists.xenproject.org/archives/html/xen-devel/2016-08/msg00562.html

(I will be out of the office starting tomorrow so may not be able to respond
to comments right away)


Boris Ostrovsky (2):
  xen/x86: Convert to hotplug state machine
  xen/events: Convert to hotplug state machine

 arch/x86/xen/enlighten.c |  115 +
 drivers/xen/events/events_fifo.c |   34 ---
 include/linux/cpuhotplug.h   |3 +
 3 files changed, 80 insertions(+), 72 deletions(-)


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

Re: [Xen-devel] [PATCH v3 6/9] livepatch: Add parsing for the symbol+0x/

[Konrad Rzeszutek Wilk]

On Mon, Aug 15, 2016 at 04:53:48AM -0600, Jan Beulich wrote:
> >>> On 14.08.16 at 23:52,  wrote:
> > in case we want to patch at specific offsets inside
> > a function. (for example if we want to do NOP patching).
> > 
> > We also assume that the 'len' is only the size of
> > an isns that would be for a call opcode (so 5 bytes
> > on x86, and 4 on ARM 32/64).
> 
> Which makes the notation using a slash quite confusing: Normally
> if we see +/ the length part is assumed
> to be the overall size of the object/function the name refers to.

Oh, I somehow had the length of instruction on my mind.

> Could I talk you into using a different separator, like colon or
> semicolon?

Of course. Whatever is easier to folks.

I can also ditch the  part altogether?

> 
> > --- a/xen/common/livepatch.c
> > +++ b/xen/common/livepatch.c
> > @@ -234,14 +234,46 @@ static const char *livepatch_symbols_lookup(unsigned 
> > long addr,
> >  
> >  static int lookup_symbol(struct livepatch_func *f, struct livepatch_elf 
> > *elf)
> >  {
> > +const char *s;
> > +char *plus = NULL, *slash = NULL;
> 
> Pointless initializers, afaict. Also the latter can be const.
> 
> > +unsigned long offset = 0;
> > +
> >  if ( f->old_addr )
> >  return 0;
> >  
> > +s = f->name;
> > +/* +/ */
> > +plus = strchr(f->name, '+');
> > +if ( plus )
> > +{
> > +slash = strchr(plus, '/');
> > +
> > +if ( slash )
> > +{
> > +const char *endp = NULL;
> > +unsigned int len;
> > +
> > +offset = simple_strtoul(plus+1, , 16);
> 
> Missing blanks around +.
> 
> > +if ( endp != slash )
> > +return -EINVAL;
> > +
> > +len = simple_strtoul(slash+1, NULL, 16);
> 
> Would you perhaps want to be even more precise and verify that
> nothing follows this number?

/me nods.
> 
> Jan
> 

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

Re: [Xen-devel] [PATCH v3 4/9] livepatch: Sync cache of build-id before using it first time.

[Konrad Rzeszutek Wilk]

On Mon, Aug 15, 2016 at 11:46:13AM +0100, Andrew Cooper wrote:
> On 15/08/16 11:38, Jan Beulich wrote:
>  On 14.08.16 at 23:52,  wrote:
> >> We don't print at bootup time the build-id. The reason is
> >> that xen_build_init and livepatch_init are both __initcall
> >> type routines. This meant that when livepatch_init called
> >> xen_build_id, it would return -ENODATA as build_id_len was
> >> not setup yet (b/c xen_build_init would be called later).
> >>
> >> We fix this by calling xen_build_init in livepatch_init which
> >> allows us to print the build-id of the hypervisor.
> >>
> >> We also keep xen_build_init as __initcall because build-id
> >> can be built without livepatching being enabled (so
> >> no livepatch_init being called).
> > Now if the build ID is potentially useful outside of live patching,
> > shouldn't its logging also be done outside of livepatch code?

And by logging you mean the printk of it which is right now
only done in common/livepatch.c ?
> 
> IMO the Build ID should be available and printed irrespective of live
> patching.
> 
> It is useful information simply to identify the hypervisor.

Right, and it is.

As in you can build the hypervisor without livepatching and the build-id will 
be built in.

I can certainly make xen_build_id call:

 printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p);

which would be a much simpler fix :-)

Let me do that.

> 
> ~Andrew

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

Re: [Xen-devel] [PATCH v3 1/9] livepatch: Clear .bss when payload is reverted

[Konrad Rzeszutek Wilk]

On Mon, Aug 15, 2016 at 04:27:38AM -0600, Jan Beulich wrote:
> >>> On 14.08.16 at 23:52,  wrote:
> > @@ -374,7 +376,18 @@ static int move_payload(struct payload *payload, 
> > struct livepatch_elf *elf)
> >  elf->name, elf->sec[i].name, 
> > elf->sec[i].load_addr);
> >  }
> >  else
> > -memset(elf->sec[i].load_addr, 0, elf->sec[i].sec->sh_size);
> > +{
> > +/* We expect only one BSS. */
> > +if ( payload->bss )
> > +{
> > +rc = -EINVAL;
> 
> -EOPNOTSUPP according to e.g. the only- one-symbol-table code.

Thanks!
> 
> > +goto out;
> > +}
> > +payload->bss = elf->sec[i].load_addr;
> > +payload->bss_size = elf->sec[i].sec->sh_size;
> > +
> > +memset(payload->bss, 0, payload->bss_size);
> > +}
> >  }
> >  }
> >  
> > @@ -1034,6 +1047,9 @@ static int revert_payload(struct payload *data)
> >  list_del_rcu(>applied_list);
> >  unregister_virtual_region(>region);
> >  
> > +/* And clear the BSS for subsequent operation. */
> > +memset(data->bss, 0, data->bss_size);
> 
> Instead of doing it in two places, how about moving this into
> apply_payload()?

I was thinking of it too, but then it occurred to me that between the
'check' -> 'apply' the BSS is left unitialized. And if we ever crash
(right as somebody scheduled the livepatch) one could form the opinion
that it is due to the .bss having garbage. Or more importantly - the
hooks may not have run, but all its values looked like they have been
initialized.

And spend considerable amount of time debugging something which in reality
is not a problem?

Perhaps put it in multiple places :-)

> 
> Jan
> 

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

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 15.08.16 at 14:07,  wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> On 15.08.16 at 12:47,  wrote:
>> > What about including in the "fixed" part of the hypercall a virtual
>> > address range that all pointers must be in?  That wouldn't even require
>> > a user/kernel flag actually; and could conceivably be used by the caller
>> > (either userspace or kernel space) to thwart certain kinds of potential
>> > attacks.
>> 
>> That's definitely an option, if we're sufficiently certain that no OSes
>> will ever require two or more ranges.
> 
> How hard would it be to allow the caller to specify several allowable
> ranges ?

Not very hard - just like with so many things an additional level
of indirection would do.

> Note that the hypercall argument construction code in libxc already
> has to handle all hypercall argument memory specially, so libxc could
> automatically build a list of the arguments' memory addresses.
> 
> What would be needed is some kind of restriction on (or variant of)
> copy_* which double-checked against the list provided in the
> non-op-specific part of the hypercall.

Yeah, as George already mentioned. I'd favor "variant of", to avoid
penalizing all other callers.

Jan


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

Re: [Xen-devel] [PATCH 2/4] x86emul: drop RIP-relative special case for TEST

[Andrew Cooper]

On 15/08/16 09:34, Jan Beulich wrote:
> @@ -1851,11 +1911,6 @@ x86_emulate(
>  ((op_bytes == 8) ? 4 : op_bytes);
>  else if ( (d & SrcMask) == SrcImmByte )
>  ea.mem.off += 1;
> -else if ( !ext && ((b & 0xfe) == 0xf6) &&
> -  ((modrm_reg & 7) <= 1) )

Do we actually handle these cases correctly?  0xf6 /0 (imm8) and 0xf7 /0
(imm) look to work as expected

However, 0xf6 /1, 0xf7 /1 are harder to pin down.  We have an
implementation of it, but the only other reference I can find to them
are in the AMD grp3 opcode map, where they appear equal to their /0
variants.  The /1 variants do not appear in the AMD description of the
TEST instruction, and do not appear anywhere in the Intel manuals.

Suravee: Can you confirm whether the /1 variants are expected to be
implemented and copies of the /0 variants?

~Andrew

> -/* Special case in Grp3: test has immediate operand. */
> -ea.mem.off += (d & ByteOp) ? 1
> -: ((op_bytes == 8) ? 4 : op_bytes);
>  break;
>  case 1:
>  ea.mem.off += insn_fetch_type(int8_t);


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

Re: [Xen-devel] [PATCH v1 6/9] livepatch: Initial ARM64 support.

[Jan Beulich]

>>> On 15.08.16 at 16:09,  wrote:
> On Mon, Aug 15, 2016 at 02:21:48AM -0600, Jan Beulich wrote:
>> >>> On 15.08.16 at 01:07,  wrote:
>> > @@ -711,9 +711,15 @@ static int prepare_payload(struct payload *payload,
>> >  return -EINVAL;
>> >  }
>> >  }
>> > +#ifndef CONFIG_ARM
>> >  apply_alternatives_nocheck(start, end);
>> > +#else
>> > +apply_alternatives(start, sec->sec->sh_size);
>> > +#endif
>> 
>> Conditionals like this are ugly - can't this be properly abstracted?
> 
> Yes, I can introduce an apply_alternatives_nocheck on ARM that will
> hava the same set of arguments on x86.
> 
> Or I can make a new function name?

Either way is fine with me, with a slight preference to the former
one.

Jan


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

Re: [Xen-devel] [PATCH 2/2] x86/EFI: use less crude way of generating the build ID

[Konrad Rzeszutek Wilk]

On Mon, Aug 15, 2016 at 01:58:47AM -0600, Jan Beulich wrote:
> >>> On 15.08.16 at 01:42,  wrote:
> >> --- a/xen/arch/x86/efi/Makefile
> >> +++ b/xen/arch/x86/efi/Makefile
> >> @@ -9,6 +9,9 @@ efi := $(if $(efi),$(shell $(CC) $(filte
> >>  efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi 
> >> check.o 2>disabled && echo y))
> >>  efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call 
> >> create,boot.init.o); $(call create,runtime.o)))
> >>  
> >> -extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o
> >> +extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o
> >> +
> >> +%.o: %.ihex
> >> +  $(OBJCOPY) -I ihex -O binary $< $@
> > 
> > 
> > Under  Ubuntu 14.04.4 this fails compilation:
> > 
> > 
> > make[4]: *** No rule to make target `buildid.o', needed by `stub.o'.
> 
> That's extremely odd, namely considering that the rule is right
> there in the quoted text above. Could you double check
> xen/arch/x86/efi/buildid.ihex is actually there?


No it is not. I had issues with your email (git am -s) so I did it by
hand (patch -p2) and of course forgot to include the new file. And later
on built it on another OS after pulling from a git tree which missed this file.

Sorry about the false report!

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

Re: [Xen-devel] [PATCH v1 6/9] livepatch: Initial ARM64 support.

[Konrad Rzeszutek Wilk]

On Mon, Aug 15, 2016 at 02:21:48AM -0600, Jan Beulich wrote:
> >>> On 15.08.16 at 01:07,  wrote:
> > --- a/xen/common/Kconfig
> > +++ b/xen/common/Kconfig
> > @@ -222,7 +222,7 @@ endmenu
> >  config LIVEPATCH
> > bool "Live patching support (TECH PREVIEW)"
> > default n
> > -   depends on X86 && HAS_BUILD_ID = "y"
> > +   depends on (X86 || ARM_64) && HAS_BUILD_ID = "y"
> 
> Would this better become a black list?

OK, that would make it easier.
> 
> > @@ -711,9 +711,15 @@ static int prepare_payload(struct payload *payload,
> >  return -EINVAL;
> >  }
> >  }
> > +#ifndef CONFIG_ARM
> >  apply_alternatives_nocheck(start, end);
> > +#else
> > +apply_alternatives(start, sec->sec->sh_size);
> > +#endif
> 
> Conditionals like this are ugly - can't this be properly abstracted?

Yes, I can introduce an apply_alternatives_nocheck on ARM that will
hava the same set of arguments on x86.

Or I can make a new function name?
> 
> > --- a/xen/include/xen/elfstructs.h
> > +++ b/xen/include/xen/elfstructs.h
> > @@ -103,6 +103,15 @@ typedef uint64_t   Elf64_Xword;
> >(ehdr).e_ident[EI_MAG2] == ELFMAG2 && \
> >(ehdr).e_ident[EI_MAG3] == ELFMAG3)
> >  
> > +/* e_flags */
> > +#define EF_ARM_EABI_MASK   0xff00
> > +#define EF_ARM_EABI_UNKNOWN0x
> > +#define EF_ARM_EABI_VER1   0x0100
> > +#define EF_ARM_EABI_VER2   0x0200
> > +#define EF_ARM_EABI_VER3   0x0300
> > +#define EF_ARM_EABI_VER4   0x0400
> > +#define EF_ARM_EABI_VER5   0x0500
> 
> Aren't these ARM32 definitions, which should be unneeded for
> ARM64 support?

Correct. Let me move that and also the 
arch/arm/arm32/livepatch:arch_livepatch_verify_elf
code out of this patch.
> 
> > @@ -171,6 +180,7 @@ typedef struct {
> >  #define EM_PPC 20  /* PowerPC */
> >  #define EM_PPC64   21  /* PowerPC 64-bit */
> >  #define EM_ARM 40  /* Advanced RISC Machines ARM */
> > +#define EM_AARCH64 183 /* ARM 64-bit */
> >  #define EM_ALPHA   41  /* DEC ALPHA */
> >  #define EM_SPARCV9 43  /* SPARC version 9 */
> >  #define EM_ALPHA_EXP   0x9026  /* DEC ALPHA */
> 
> I think this tries to be sorted by number.
> 
> > +/*
> > + * S - address of symbol.
> > + * A - addend for relocation (r_addend)
> > + * P - address of the dest being relocated (derieved from r_offset)
> > + * NC -  No check for overflow.
> > + *
> > + * The defines also use _PREL for PC-relative address, and _NC is No Check.
> > + */
> > +#define R_AARCH64_ABS64257 /* Direct 64 bit. S+A, NC*/
> > +#define R_AARCH64_ABS32258 /* Direct 32 bit. S+A */
> > +#define R_AARCH64_PREL64   260 /* S+A-P, NC */
> > +#define R_AARCH64_PREL32   261 /* S+A-P */
> > +
> > +#define R_AARCH64_ADR_PREL_LO21274 /* ADR imm, [20:0]. S+A-P */
> > +#define R_AARCH64_ADR_PREL_PG_HI21 275 /* ADRP imm, [32:12]. Page(S+A) - 
> > Page(P).*/
> > +#define R_AARCH64_ADD_ABS_LO12_NC  277 /* ADD imm. [11:0]. S+A, NC */
> > +
> > +#define R_AARCH64_CONDBR19 280 /* Bits 20:2, S+A-P */
> > +#define R_AARCH64_JUMP26   282 /* Bits 27:2, S+A-P */
> > +#define R_AARCH64_CALL26   283 /* Bits 27:2, S+A-P */
> 
> No R_AARCH64_TSTBR14?
> 
> > +#define R_AARCH64_LDST16_ABS_LO12_NC   284 /* LD/ST to bits 11:1, S+A, 
> > NC */
> > +#define R_AARCH64_LDST32_ABS_LO12_NC   285 /* LD/ST to bits 11:2, S+A, 
> > NC */
> > +#define R_AARCH64_LDST64_ABS_LO12_NC   286 /* LD/ST to bits 11:3, S+A, 
> > NC */
> > +#define R_AARCH64_LDST8_ABS_LO12_NC278 /* LD/ST to bits 11:0, S+A, 
> > NC */
> 
> What about R_AARCH64_LDST128_ABS_LO12_NC?

I didnt' see it in the ELF of xen-syms or the livepatch tests cases so I 
omitted them.
But then I added some that weren't in the ELF files either: LDST16 and LDST8.

I will add the two missing ones and also sort this.

Thanks!
> 
> Jan
> 

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

Re: [Xen-devel] [PATCH 1/4] x86emul: remove dead code

[Andrew Cooper]

On 15/08/16 09:34, Jan Beulich wrote:
> As of commit 989cdfa9b4 ("x86emul: don't special case fetching unsigned
> 8-bit immediates") the conditional being removed has been always false.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xentop: Adds options for tabs-separators, and including the domain ID in the output.

[Ian Jackson]

Wei Liu writes ("Re: [PATCH] xentop: Adds options for tabs-separators, and 
including the domain ID in the output."):
> On Sat, Aug 13, 2016 at 05:51:21PM +, Stefan Wieser wrote:
> > This change adds two options to xentop:
> > 
> > -T  adds a tabulator (\t) character after each field,
> > to allow easier and more robust parsing. This
> > affects batch mode only.
...
> All in all I have no opinion for a few new options. I will wait a few
> gays so that other people can express their opinions.

In the absence of a better way of collecting this data in a
machine-readable fashion, I think something like this patch is a good
idea.

Stefan, I think you need to do something about the possibility that a
domain name could contain \t.  I suggest \-escaping the domname
(\ => \\; tab => \t; space left unchanged; bytes 0-31 and 127 encoded
with \xXX; assumption is that output may be UTF-8, which will be
passed unchanged if it contains no control chars).

I think this behaviour should be enabled by -T.

Also, I think you could profitably add something to the docs
explaining how to parse the output.  In particular, that a parser must
look at the column headings and not assume the columns will always be
the same columsn in the same order.

Ian.

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

Re: [Xen-devel] [Very RFC PATCH 3/3] livepatch: Initial ARM32/64 support.

[Julien Grall]

Hi Konrad,

On 12/08/2016 22:50, Konrad Rzeszutek Wilk wrote:

On Fri, Aug 12, 2016 at 05:00:47PM +0200, Julien Grall wrote:

diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
index 65c0cdf..f4fcfd6 100644
--- a/xen/include/asm-arm/current.h
+++ b/xen/include/asm-arm/current.h
@@ -33,8 +33,15 @@ static inline struct cpu_info *get_cpu_info(void)

 #define guest_cpu_user_regs() (_cpu_info()->guest_cpu_user_regs)

+#ifdef CONFIG_LIVEPATCH
+#define switch_stack_and_jump(stack, fn)\
+asm volatile ("mov sp,%0;"  \
+  "bl check_for_livepatch_work;"\


May I ask why check_for_livepatch_work is called in switch_stack_and_jump?


From 29f4ab0b0a4ff62589af35b7cbc2334e1d9acdcd:
To perform and action on a payload, the hypercall sets up a data
structure to schedule the work.  A hook is added in the reset_stack_and_jump
to check for work and execute it if needed (specifically we check an
per-cpu flag to make this as quick as possible).

In this way, patches can be applied with all CPUs idle and without
stacks.  The first CPU to run check_for_xsplice_work() becomes the
master and triggers a reschedule softirq to trigger all the other CPUs
to enter check_for_xsplice_work() with no stack.  Once all CPUs
have rendezvoused, all CPUs disable their IRQs and NMIs are ignored.
The system is then quiscient and the master performs the action.
After this, all CPUs enable IRQs and NMIs are re-enabled.



I am a bit confused, switch_stack_and_jump will only be called on ARM 
when a vCPU is created. So why do you want to check livepatch at that time?


After looking at the x86 version, I noticed that reset_stack_and_jump is 
called every time Xen returns to the guest, correct?


If so, you may want to call check_for_livepatch_work in 
leave_hypervisor_tail instead. The latter will be call every time Xen 
returns to the guest.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] acpi: Re-license ACPI builder files from GPLv2 to LGPLv2.1

[Boris Ostrovsky]

On 08/01/2016 09:56 AM, Boris Ostrovsky wrote:
> Simon, Keir,


Simon, ping?


>
>
> In case you didn't realize --- this needs your ACK.
>
> Jan, now that I checked the logs more carefully, you are the only one
> from Suse/Novell who touched these files so your ACK is needed as well.
>
> Thanks.
> -boris
>
>
> On 07/18/2016 10:01 AM, Boris Ostrovsky wrote:
>> ACPI builder is currently distributed under GPLv2 license.
>>
>> We plan to make the builder available to components other
>> than the hvmloader (which is also GPLv2). Some of these
>> components (such as libxl) may be distributed under LGPL-2.1
>> so that they can be used by non-GPLv2 callers.  But this
>> will not be possible if we incorporate the ACPI builder in
>> those other components.
>>
>> To avoid this problem we are relicensing sources in ACPI
>> bulder directory to the Lesser GNU Public License (LGPL)
>> version 2.1
>>
>> Signed-off-by: Boris Ostrovsky 
>> CC: Kouya Shimura 
>> CC: Daniel Kiper 
>> CC: Stefan Berger 
>> CC: Simon Horman 
>> CC: Keir Fraser 
>> CC: Ian Jackson 
>> CC: Lars Kurth 
>> ---
>>
>> More details can be found in
>>   https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg01367.html
>>
>>
>>  tools/firmware/hvmloader/acpi/Makefile|   18 --
>>  tools/firmware/hvmloader/acpi/acpi2_0.h   |   19 ---
>>  tools/firmware/hvmloader/acpi/build.c |   18 --
>>  tools/firmware/hvmloader/acpi/dsdt.asl|   18 --
>>  tools/firmware/hvmloader/acpi/mk_dsdt.c   |   12 
>>  tools/firmware/hvmloader/acpi/ssdt_pm.asl |   11 ---
>>  tools/firmware/hvmloader/acpi/ssdt_s3.asl |   11 ---
>>  tools/firmware/hvmloader/acpi/ssdt_s4.asl |   11 ---
>>  tools/firmware/hvmloader/acpi/ssdt_tpm.asl|   18 --
>>  tools/firmware/hvmloader/acpi/static_tables.c |   18 --
>>  10 files changed, 72 insertions(+), 82 deletions(-)
>>
>> diff --git a/tools/firmware/hvmloader/acpi/Makefile 
>> b/tools/firmware/hvmloader/acpi/Makefile
>> index d3e882a..703d67b 100644
>> --- a/tools/firmware/hvmloader/acpi/Makefile
>> +++ b/tools/firmware/hvmloader/acpi/Makefile
>> @@ -1,17 +1,15 @@
>>  #
>>  # Copyright (c) 2004, Intel Corporation.
>>  #
>> -# This program is free software; you can redistribute it and/or modify it
>> -# under the terms and conditions of the GNU General Public License,
>> -# version 2, as published by the Free Software Foundation.
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU Lesser General Public License as published
>> +# by the Free Software Foundation; version 2.1 only. with the special
>> +# exception on linking described in file LICENSE.
>>  #
>> -# This program is distributed in the hope it will be useful, but WITHOUT
>> -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> -# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> -# more details.
>> -#
>> -# You should have received a copy of the GNU General Public License along 
>> with
>> -# this program; If not, see .
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# NU Lesser General Public License for more details.
>>  #
>>  
>>  XEN_ROOT = $(CURDIR)/../../../..
>> diff --git a/tools/firmware/hvmloader/acpi/acpi2_0.h 
>> b/tools/firmware/hvmloader/acpi/acpi2_0.h
>> index 78eb43d..6fa3452 100644
>> --- a/tools/firmware/hvmloader/acpi/acpi2_0.h
>> +++ b/tools/firmware/hvmloader/acpi/acpi2_0.h
>> @@ -1,18 +1,15 @@
>>  /*
>>   * Copyright (c) 2004, Intel Corporation.
>>   *
>> - * This program is free software; you can redistribute it and/or modify it
>> - * under the terms and conditions of the GNU General Public License,
>> - * version 2, as published by the Free Software Foundation.
>> - *
>> - * This program is distributed in the hope it will be useful, but WITHOUT
>> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> - * more details.
>> - *
>> - * You should have received a copy of the GNU General Public License along 
>> with
>> - * this program; If not, see .
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU Lesser General Public License as published
>> + * by the Free Software Foundation; version 2.1 only. with the special
>> + * exception on linking described in file LICENSE.
>>   *
>> + * This program is distributed in the hope 

Re: [Xen-devel] [PATCH v2 23/23] libxc/xc_dom_core: Copy ACPI tables to guest space

[Boris Ostrovsky]

On 08/15/2016 03:48 AM, Shannon Zhao wrote:
> Hi Boris
>
> On 2016/8/5 5:06, Boris Ostrovsky wrote:
>> Load ACPI modules into guest space
>>
>> Signed-off-by: Boris Ostrovsky 
>> ---
>> v2:
>> * New patch, loosely based on Shannon's ARM patch
>>
>>  tools/libxc/xc_dom_core.c | 92 
>> +++
>>  1 file changed, 92 insertions(+)
>>
>> diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
>> index ebada89..00d870f 100644
>> --- a/tools/libxc/xc_dom_core.c
>> +++ b/tools/libxc/xc_dom_core.c
>> @@ -1040,6 +1040,94 @@ static int xc_dom_build_ramdisk(struct xc_dom_image 
>> *dom)
>>  return -1;
>>  }
>>  
>> +static int populate_acpi_pages(struct xc_dom_image *dom,
>> +   xen_pfn_t *extents,
>> +   unsigned int num_pages)
>> +{
>> +int rc;
>> +xc_interface *xch = dom->xch;
>> +uint32_t domid = dom->guest_domid;
>> +unsigned long idx, first_high_idx = (1ull << (32 - 12));
>> +
>> +for ( ; num_pages; num_pages--, extents++ )
>> +{
>> +
>> +if ( xc_domain_populate_physmap(xch, domid, 1, 0, 0, extents) == 1 )
>> +continue;
>> +
>> +if (dom->highmem_end)
>> +{
>> +idx = --dom->highmem_end;
>> +if ( idx == first_high_idx )
>> +dom->highmem_end = 0;
>> +}
>> +else
>> +idx = --dom->lowmem_end;
>> +
>> +rc = xc_domain_add_to_physmap(xch, domid,
>> +  XENMAPSPACE_gmfn,
>> +  idx, *extents);
>> +if (rc)
>> +return rc;
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +static int xc_dom_load_acpi(struct xc_dom_image *dom)
>> +{
>> +int j, i = 0;
>> +unsigned num_pages;
>> +xen_pfn_t *extents, base;
>> +void *ptr;
>> +
>> +while ( (i < MAX_ACPI_MODULES) && dom->acpi_modules[i].length )
>> +{
>> +DOMPRINTF("%s: %d bytes at address %lx\n", __FUNCTION__,
>> +  dom->acpi_modules[i].length,
>> +  dom->acpi_modules[i].guest_addr_out);
>> +
>> +num_pages = (dom->acpi_modules[i].length + (XC_PAGE_SIZE - 1)) >>
>> +   XC_PAGE_SHIFT;
>> +extents = malloc(num_pages * sizeof(*extents));
>> +if ( !extents )
>> +{
>> +DOMPRINTF("%s: Out of memory", __FUNCTION__);
>> +goto err;
>> +}
>> +
>> +base = dom->acpi_modules[i].guest_addr_out >> XC_PAGE_SHIFT;
>> +for (j=0; j> +extents[j] = base + j;
>> +if ( populate_acpi_pages(dom, extents, num_pages) )
>> +{
>> +DOMPRINTF("%s: Can populate ACPI pages", __FUNCTION__);
>> +goto err;
>> +}
>> +
>> +ptr = xc_map_foreign_range(dom->xch, dom->guest_domid,
>> +  XC_PAGE_SIZE * num_pages,
>> +  PROT_READ | PROT_WRITE, base);
>> +if ( !ptr )
>> +{
>> +DOMPRINTF("%s: Can't map %d pages at 0x%lx",
>> +  __FUNCTION__, num_pages, base);
>> +goto err;
>> +}
>> +
>> +memcpy(ptr, dom->acpi_modules[i].data, dom->acpi_modules[i].length);
>> +
>> +free(extents);
>> +i++;
>> +}
>> +
>> +return 0;
>> +
>> +err:
>> +free(extents);
>> +return -1;
>> +}
>> +
>>  int xc_dom_build_image(struct xc_dom_image *dom)
>>  {
>>  unsigned int page_size;
>> @@ -1097,6 +1185,10 @@ int xc_dom_build_image(struct xc_dom_image *dom)
>>  memcpy(devicetreemap, dom->devicetree_blob, dom->devicetree_size);
>>  }
>>  
>> +/* load ACPI tables */
>> +if ( xc_dom_load_acpi(dom) != 0 )
>> +goto err;
> I think it needs to move the definition of xc_dom_load_acpi() to
> xc_dom_x86.c while I will move the corresponding one of ARM to xc_dom_arm.c.


You don't think both x86 and ARM can use the same definition?  This
looks very similar to what you had in your series.

-boris


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

Re: [Xen-devel] [PATCH v2 00/23] Make ACPI builder available to components other than hvmloader

[Boris Ostrovsky]

On 08/15/2016 02:37 AM, Shannon Zhao wrote:
>
> On 2016/8/5 5:06, Boris Ostrovsky wrote:
>> The goal here is to build ACPI tables for PVHv2/HVMlite guests while reusing 
>> existing
>> hvmloader's ACPI builder code. The builder is provided as a library in 
>> tools/libacpi.
>>
>> This version is built on top of Anthony's 
>> git://xenbits.xen.org/people/aperard/xen-unstable.git:hvmloader-with-separated-bios-v7
>>
>> It also requires (still not fully ACKed, sigh) ACPI relicensing patch
>> https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg01975.html
>>
> Hi Boris,
> Is there a git repo from which I cloud fetch this series? I didn't find
> them in https://oss.oracle.com/git/?p=bostrovs/xen.git;a=summary


I am having trouble pulling Anthony's branch for some reason (I need it
before pushing my patches). I'll try to see what's going on but not sure
I'll be able to resolve it before I leave for vacation tomorrow. I can
resend patches to you directly if that helps.

-boris

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

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

[osstest service owner]

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

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  1f848de6f229e2b3a5aa84399d2639a958a6e945
baseline version:
 xen  01fe4da6243be692b972de4ec7b22ec92527c0f5

Last test of basis   100490  2016-08-15 09:01:58 Z0 days
Testing same since   100493  2016-08-15 11:01:42 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 

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=1f848de6f229e2b3a5aa84399d2639a958a6e945
+ . ./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 
1f848de6f229e2b3a5aa84399d2639a958a6e945
+ branch=xen-unstable-smoke
+ revision=1f848de6f229e2b3a5aa84399d2639a958a6e945
+ . ./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-4.7-testing
+ '[' x1f848de6f229e2b3a5aa84399d2639a958a6e945 = 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 

Re: [Xen-devel] [PATCH] xentop: Adds options for tabs-separators, and including the domain ID in the output.

[Wei Liu]

On Mon, Aug 15, 2016 at 01:09:32PM +0100, Wei Liu wrote:
> On Sat, Aug 13, 2016 at 05:51:21PM +, Stefan Wieser wrote:
> > This change adds two options to xentop:
> > 
> > -T  adds a tabulator (\t) character after each field, to allow
> > easier and more robust parsing. This affects batch mode 
> > only.
> > -I  includes a column with the domain ID in the output (both the
> > graphical output, and the batch output)
> > 
> > This makes the output easier to parse for automated tools.
> > If none of the options are passed, the output is unchanged, so none of them 
> > would break existing tools.
> 
> Line too long.
> 
> And thanks for having compatibility in mind.
> 
> > @@ -1235,9 +1273,19 @@ int main(int argc, char **argv)
> > case 't':
> > show_tmem = 1;
> > break;
> > +   case 'T':
> > +   use_tabs = 1;
> > +   break;
> > +   case 'I':
> > +   show_domid = 1;
> > +   break;
> > }
> > }
> >  
> > +   if (use_tabs && !batch) {
> > +   fail("Cannot use tabs in interactive mode.\n");
> > +   }
> > +
> 
> Minor nit: no need to use {} here.
> 
> No need to resend, though. Should be easy to fix if I am to commit this
> patch.
> 
> All in all I have no opinion for a few new options. I will wait a few
> gays so that other people can express their opinions.
> 

^ days, sorry...

> Wei.
> 
> > /* Get xenstat handle */
> > xhandle = xenstat_init();
> > if (xhandle == NULL)
> > -- 
> > 1.9.1
> > 

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

Re: [Xen-devel] [PATCH 0/4] Clarify License information : m4, stubdom, tools/blktap2, xen/crypto

[Wei Liu]

Cc Ian's correct email address

On Fri, Aug 12, 2016 at 06:32:32PM +0100, Lars Kurth wrote:
> This series contains some of the easier license clean-up cases, which can be 
> fixed
> by adding COPYING files or README.source files. The series specially contains:
> 
> xen/crypto and xen/include/crypto
> Added a README.source file
> Does not need a COPYING file: all files are imported 
> 
> m4
> Updated README.source to include ax_compare_version.m4
> Does not need a COPYING file: all files are GPLv2  
> Exceptions are exclusively from imported files: GPLv2+ 
> and Laurikari 
> (https://enterprise.dejacode.com/license_library/Demo/laurikari/)
> 
> stubdom
> Added a COPYING file as a boilerplate to explain license oddities in this 
> directory
> Added a vtpm/COPYING file which contains MIT licensed files only
> Added a vtpmmgr/README.source file which contains many BSD-3-Clause files 
> only,
> that originally came from tools/vtpm_manager
> 
> tools/blktap2
> Added a COPYING file 
> There is some complexity here, but I believe it is not necessary to fix this 
> at this
> stage, as for some time we were discussing to remove blktap2.
> 
> -- 
> 2.5.4 (Apple Git-61)
> 

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

Re: [Xen-devel] [PATCH] xentop: Adds options for tabs-separators, and including the domain ID in the output.

[Wei Liu]

On Sat, Aug 13, 2016 at 05:51:21PM +, Stefan Wieser wrote:
> This change adds two options to xentop:
> 
> -T  adds a tabulator (\t) character after each field, to allow
> easier and more robust parsing. This affects batch mode only.
> -I  includes a column with the domain ID in the output (both the
> graphical output, and the batch output)
> 
> This makes the output easier to parse for automated tools.
> If none of the options are passed, the output is unchanged, so none of them 
> would break existing tools.

Line too long.

And thanks for having compatibility in mind.

> @@ -1235,9 +1273,19 @@ int main(int argc, char **argv)
>   case 't':
>   show_tmem = 1;
>   break;
> + case 'T':
> + use_tabs = 1;
> + break;
> + case 'I':
> + show_domid = 1;
> + break;
>   }
>   }
>  
> + if (use_tabs && !batch) {
> + fail("Cannot use tabs in interactive mode.\n");
> + }
> +

Minor nit: no need to use {} here.

No need to resend, though. Should be easy to fix if I am to commit this
patch.

All in all I have no opinion for a few new options. I will wait a few
gays so that other people can express their opinions.

Wei.

>   /* Get xenstat handle */
>   xhandle = xenstat_init();
>   if (xhandle == NULL)
> -- 
> 1.9.1
> 

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

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
depriv)"):
> On 15.08.16 at 12:47,  wrote:
> > What about including in the "fixed" part of the hypercall a virtual
> > address range that all pointers must be in?  That wouldn't even require
> > a user/kernel flag actually; and could conceivably be used by the caller
> > (either userspace or kernel space) to thwart certain kinds of potential
> > attacks.
> 
> That's definitely an option, if we're sufficiently certain that no OSes
> will ever require two or more ranges.

How hard would it be to allow the caller to specify several allowable
ranges ?

Note that the hypercall argument construction code in libxc already
has to handle all hypercall argument memory specially, so libxc could
automatically build a list of the arguments' memory addresses.

What would be needed is some kind of restriction on (or variant of)
copy_* which double-checked against the list provided in the
non-op-specific part of the hypercall.

Ian.

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

Re: [Xen-devel] [OSSTEST PATCH] ts-coverity-upload: Double coverity upload timeout

[Andrew Cooper]

On 15/08/16 11:46, Ian Jackson wrote:
> From 3600s to 7200s.  It failed in flight 100483 in the Mass colo,
> with a timeout, having uploaded 95% (151Mby) in 3600s.
>
> Ideally we would like to release the build host, but there is no
> infrastructure for doing that right now.
>
> Signed-off-by: Ian Jackson 

LGTM

Reviewed-by: Andrew Cooper 

> ---
>  ts-coverity-upload | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ts-coverity-upload b/ts-coverity-upload
> index 6d43cf8..219c8be 100755
> --- a/ts-coverity-upload
> +++ b/ts-coverity-upload
> @@ -43,7 +43,7 @@ sub upload() {
>  
>  my @args = map { ("--form", $_) } @form_args;
>  
> -push @args, qw(--max-time 3600);
> +push @args, qw(--max-time 7200);
>  push @args, qw(--fail); # turn 404 etc into a failure.
>  push @args, $r{coverity_submit_url};
>  


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

Re: [Xen-devel] [V4 PATCH 2/2] mips/panic: Replace smp_send_stop() with kdump friendly version in panic path

[河合英宏 / KAWAI，HIDEHIRO]

Hi Corey,

> From: Corey Minyard [mailto:cminy...@mvista.com]
> Sent: Friday, August 12, 2016 10:56 PM
> I'll try to test this, but I have one comment inline...

Thank you very much!

> On 08/11/2016 10:17 PM, Dave Young wrote:
> > On 08/10/16 at 05:09pm, Hidehiro Kawai wrote:
[snip]
> >> diff --git a/arch/mips/kernel/crash.c b/arch/mips/kernel/crash.c
> >> index 610f0f3..1723b17 100644
> >> --- a/arch/mips/kernel/crash.c
> >> +++ b/arch/mips/kernel/crash.c
> >> @@ -47,9 +47,14 @@ static void crash_shutdown_secondary(void *passed_regs)
> >>
> >>   static void crash_kexec_prepare_cpus(void)
> >>   {
> >> +  static int cpus_stopped;
> >>unsigned int msecs;
> >> +  unsigned int ncpus;
> >>
> >> -  unsigned int ncpus = num_online_cpus() - 1;/* Excluding the panic cpu */
> >> +  if (cpus_stopped)
> >> +  return;
> 
> Wouldn't you want an atomic operation and some special handling here to
> ensure that only one CPU does this?  So if a CPU comes in here and
> another CPU is already in the process stopping the CPUs it won't result in a
> deadlock.

Because this function can be called only one panicking CPU,
there is no problem.

There are two paths which crash_kexec_prepare_cpus is called.

Path 1 (panic path):
panic()
  crash_smp_send_stop()
crash_kexec_prepare_cpus()

Path 2 (oops path):
crash_kexec()
  __crash_kexec()
machine_crash_shutdown()
  default_machine_crash_shutdown() // for MIPS
crash_kexec_prepare_cpus()

Here, panic() and crash_kexec() run exclusively via
panic_cpu atomic variable.  So we can use cpus_stopped as
normal variable.

Best regards,

Hidehiro Kawai

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

Re: [Xen-devel] [V4 PATCH 1/2] x86/panic: Replace smp_send_stop() with kdump friendly version in panic path

[河合英宏 / KAWAI，HIDEHIRO]

Hi Dave,

Thank you for the review.

> From: Dave Young [mailto:dyo...@redhat.com]
> Sent: Friday, August 12, 2016 12:17 PM
> 
> Thanks for the update.
> On 08/10/16 at 05:09pm, Hidehiro Kawai wrote:
> > Daniel Walker reported problems which happens when
> > crash_kexec_post_notifiers kernel option is enabled
> > (https://lkml.org/lkml/2015/6/24/44).
> >
> > In that case, smp_send_stop() is called before entering kdump routines
> > which assume other CPUs are still online.  As the result, for x86,
> > kdump routines fail to save other CPUs' registers  and disable
> > virtualization extensions.
> 
> Seems you simplified the changelog, but I think a little more details
> will be helpful to understand the patch. You know sometimes lkml.org
> does not work well.

So, I'll try another archives when I post patch set next time.

> > To fix this problem, call a new kdump friendly function,
> > crash_smp_send_stop(), instead of the smp_send_stop() when
> > crash_kexec_post_notifiers is enabled.  crash_smp_send_stop() is a
> > weak function, and it just call smp_send_stop().  Architecture
> > codes should override it so that kdump can work appropriately.
> > This patch only provides x86-specific version.
> >
> > For Xen's PV kernel, just keep the current behavior.
> 
> Could you explain a bit about above Xen PV kernel behavior?
> 
> BTW, this version looks better,  I think I'm fine with this version
> besides of the questions about changelog.

As for Dom0 kernel, it doesn't use crash_kexec routines, and
it relies on panic notifier chain.  At the end of the chain,
xen_panic_event is called, and it issues a hypercall which
requests Hypervisor to execute kdump.  This means whether
crash_kexec_panic_notifiers is set or not, panic notifiers
are called after smp_send_stop.  Even if we save registers
in Dom0 kernel, they seem to be ignored (Hypervisor is responsible
for that).  This is why I kept the current behavior for Xen.

For PV DomU kernel, kdump is not supported.  For PV HVM
DomU, I'm not sure what will happen on panic because I
couldn't boot PV HVM DomU and test it.  But I think it will
work similarly to baremetal kernels with extra cleanups
for Hypervisor.

Best regards,

Hidehiro Kawai

> > Changes in V4:
> > - Keep to use smp_send_stop if crash_kexec_post_notifiers is not set
> > - Rename panic_smp_send_stop to crash_smp_send_stop
> > - Don't change the behavior for Xen's PV kernel
> >
> > Changes in V3:
> > - Revise comments, description, and symbol names
> >
> > Changes in V2:
> > - Replace smp_send_stop() call with crash_kexec version which
> >   saves cpu states and cleans up VMX/SVM
> > - Drop a fix for Problem 1 at this moment
> >
> > Reported-by: Daniel Walker 
> > Fixes: f06e5153f4ae (kernel/panic.c: add "crash_kexec_post_notifiers" 
> > option)
> > Signed-off-by: Hidehiro Kawai 
> > Cc: Dave Young 
> > Cc: Baoquan He 
> > Cc: Vivek Goyal 
> > Cc: Eric Biederman 
> > Cc: Masami Hiramatsu 
> > Cc: Daniel Walker 
> > Cc: Xunlei Pang 
> > Cc: Thomas Gleixner 
> > Cc: Ingo Molnar 
> > Cc: "H. Peter Anvin" 
> > Cc: Borislav Petkov 
> > Cc: David Vrabel 
> > Cc: Toshi Kani 
> > Cc: Andrew Morton 
> > ---
> >  arch/x86/include/asm/kexec.h |1 +
> >  arch/x86/include/asm/smp.h   |1 +
> >  arch/x86/kernel/crash.c  |   22 +---
> >  arch/x86/kernel/smp.c|5 
> >  kernel/panic.c   |   47 
> > --
> >  5 files changed, 66 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
> > index d2434c1..282630e 100644
> > --- a/arch/x86/include/asm/kexec.h
> > +++ b/arch/x86/include/asm/kexec.h
> > @@ -210,6 +210,7 @@ struct kexec_entry64_regs {
> >
> >  typedef void crash_vmclear_fn(void);
> >  extern crash_vmclear_fn __rcu *crash_vmclear_loaded_vmcss;
> > +extern void kdump_nmi_shootdown_cpus(void);
> >
> >  #endif /* __ASSEMBLY__ */
> >
> > diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> > index ebd0c16..f70989c 100644
> > --- a/arch/x86/include/asm/smp.h
> > +++ b/arch/x86/include/asm/smp.h
> > @@ -50,6 +50,7 @@ struct smp_ops {
> > void (*smp_cpus_done)(unsigned max_cpus);
> >
> > void (*stop_other_cpus)(int wait);
> > +   void (*crash_stop_other_cpus)(void);
> > void (*smp_send_reschedule)(int cpu);
> >
> > int (*cpu_up)(unsigned cpu, struct task_struct *tidle);
> > diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> > index 9616cf7..650830e 100644
> > --- a/arch/x86/kernel/crash.c
> > +++ b/arch/x86/kernel/crash.c
> > @@ -133,15 +133,31 @@ static void kdump_nmi_callback(int cpu, struct 
> > pt_regs 

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 15.08.16 at 12:47,  wrote:
> On 15/08/16 11:19, Jan Beulich wrote:
>> Well, none of the options considered so far are really nice or
>> readily available. I think the easiest to use for both the caller and
>> the implementation of the hypercall would be the auxiliary
>> hypercall for a kernel to indicate user/kernel boundaries plus a
>> flag on the DMOP one for the kernel mode driver to indicate its
>> user mode origin. The main (purely theoretical afaict) downside
>> of this is the difficulty to use it in OSes with variable user/kernel
>> boundaries.
> 
> What about including in the "fixed" part of the hypercall a virtual
> address range that all pointers must be in?  That wouldn't even require
> a user/kernel flag actually; and could conceivably be used by the caller
> (either userspace or kernel space) to thwart certain kinds of potential
> attacks.

That's definitely an option, if we're sufficiently certain that no OSes
will ever require two or more ranges.

Jan


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

Re: [Xen-devel] [PATCH v3 9/9] livepach: Add .livepatch.hooks functions and test-case

[Jan Beulich]

>>> On 14.08.16 at 23:52,  wrote:
> v4..v11: Defered for v4.8
> v12: s/xsplice/livepatch/
> v13: Clarify the comments about spin_debug_enable

(Side note: v13 here vs v3 in the subject.)

>  Rename one of the hooks to lower-case (Z->z) to guarantee it being
>  called last.

Does lower case z really guarantee that? Wouldn't it be better to
use a sort order independent mechanism, like using another object
file (iirc object file order defines placement-within-sections unless
options get handed to the linker which specifically allow it to
shuffle things around)?

> @@ -72,7 +73,11 @@ struct payload {
>  struct livepatch_build_id dep;   /* 
> ELFNOTE_DESC(.livepatch.depends). */
>  void *bss;   /* .bss of the payload. */
>  size_t bss_size; /* and its size. */
> -char name[XEN_LIVEPATCH_NAME_SIZE]; /* Name of it. */
> +livepatch_loadcall_t **load_funcs;   /* The array of funcs to call after 
> */
> +livepatch_unloadcall_t **unload_funcs;/* load and unload of the payload. 
> */

Considering above you said "Learned a lot of about 'const'", where
are they? (Interestingly, LIVEPATCH_{,UN}LOAD_HOOK() below look
correct now, so effectively you lose constness here.)

> @@ -1065,6 +1089,18 @@ static int apply_payload(struct payload *data)
>  
>  arch_livepatch_quiesce();
>  
> +/*
> + * Since we are running with IRQs disabled and the hooks may call common
> + * code - which expects the spinlocks to run with IRQs enabled - we 
> temporarly
> + * disable the spin locks IRQ state checks.
> + */

Much better, but a little further to go: I'd suggest
s/the spinlocks/certain spinlocks/ or some such. Also - "temporarily".

Jan


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

Re: [Xen-devel] [PATCH v3 8/9] symbols: Generate an xen-sym.map

[Jan Beulich]

>>> On 14.08.16 at 23:52,  wrote:
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -136,8 +136,12 @@ $(TARGET)-syms: prelink.o xen.lds 
> $(BASEDIR)/common/symbols-dummy.o
>   | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort 
> --warn-dup \
>   >$(@D)/.$(@F).1.S
>   $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
> + $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
>   $(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
>   $(@D)/.$(@F).1.o -o $@
> + $(NM) -pa --format=sysv $(@D)/$(@F) \
> + | $(BASEDIR)/tools/symbols --xensyms --sysv --sort --warn-dup \
> + >$(@D)/$(@F).map
>   rm -f $(@D)/.$(@F).[0-9]*

So what about the xen.efi case? Is the map file there not of similar
relevance?

Also I don't think you want the --warn-dup here.

Jan


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

Re: [Xen-devel] [PATCH v3 7/9] livepatch: NOP if func->new_[addr, size] is zero.

[Jan Beulich]

>>> On 14.08.16 at 23:52,  wrote:
> The NOP functionality will NOP any of the code at
> the 'old_addr' or at 'name' if the 'new_addr' and 'new_size'
> are both zero. The purpose of this is to NOP out calls, such as:
> 
>  e9 <4-bytes-offset>

Except that E9 is JMP; CALL is E8.

> (5 byte insn), or on ARM a 4 byte insn for branching.
> 
> We need the EIP of where we need to the NOP, and that can
> be provided via the `old_addr` or `name`.
> 
> If the `old_addr` is provided we will NOP
> 5 instructions (on x86) at that location.
> 
> If `name` is provided with the symbol+0x
> we make sure that  is 5 (on x86) and upon retrieving the
> EIP based on `name` will NOP that location.

So why does this need to be restricted to 5-byte (on x86) code
blocks? I.e. what's wrong with NOP-ing out other code.

> @@ -46,18 +42,27 @@ int arch_livepatch_verify_func(const struct 
> livepatch_func *func)
>  
>  void arch_livepatch_apply_jmp(struct livepatch_func *func)
>  {
> -int32_t val;
>  uint8_t *old_ptr;
> +uint8_t insn[PATCH_INSN_SIZE];
>  
>  BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->opaque));
> -BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof(val)));
>  
>  old_ptr = func->old_addr;
>  memcpy(func->opaque, old_ptr, PATCH_INSN_SIZE);
>  
> -*old_ptr++ = 0xe9; /* Relative jump */
> -val = func->new_addr - func->old_addr - PATCH_INSN_SIZE;
> -memcpy(old_ptr, , sizeof(val));
> +if ( func->new_size )
> +{
> +int32_t val;
> +
> +BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof(val)));
> +
> +insn[0] = 0xe9;
> +val = func->new_addr - func->old_addr - PATCH_INSN_SIZE;
> +memcpy([1], , sizeof(val));
> +} else

Style.

> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -561,11 +561,15 @@ static int prepare_payload(struct payload *payload,
>  return -EOPNOTSUPP;
>  }
>  
> -if ( !f->new_addr || !f->new_size )
> +/* If both are zero then we are NOPing. */
> +if ( (!f->new_addr || !f->new_size) )

Comment and condition contradict one another. And you're adding
unnecessary parentheses.

Jan


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

Re: [Xen-devel] [PATCH 3/3] Significant changes to decision making; some new roles and minor changes

[Tim Deegan]

Hi,

At 00:13 +0100 on 12 Aug (1470960826), Lars Kurth wrote:
> +### Conflict Resolution {#conflict}
> +
> +Sub-projects and teams hosted on Xenproject.org are not democracies but 
> +meritocracies. In situations where there is disagreement on issues related 
> to 
> +the day-to-day running of the project, the [project leadership 
> +team](#leadership) is expected to act as referee and make a decision on 
> behalf 
> +of the community. Projects leadership teams can choose to delegate entire 
> +classes of conflict resolution issues to community members and/or the 
> project 
> +lead (e.g. the project can choose to delegate refereeing on committer 
> +disagreements to the project lead; or it could choose a specific committer 
> to 
> +always act as referee amongst a group of committers). Any such delegation 
> needs 
> +to be approved as normal and has to be documented.
> +
> +Should a project leadership team become dysfunctional or paralysed, the 
> project 
> +leadership team or project lead should work with the community manager or 
> +advisory board to find a way forward.
> +
> +In situations where there is significant disagreement between sub-projects, 
> the 
> +issue is deferred to the [Xen Project Advisory Board](/join.html).

This looks like a pretty significant shift of responsibilty to the AB.
As I read it, the current governance requires a _vote_ if subprojects
disagree, with the AB only called to break a tie.

It also seems to conflict with the wording that the AB "leaves all
technical decisions to the open source meritocracy".

IMO if this is to be changed it should be to something more concrete
than "significant disagreement".

> +-   Some sections of this document such as [Xen Project wide 
> +roles](#roles-global) and [making contributions](#contributions) **cannot be 
> +changed by the community** without obtaining additional approval from the 
> +Advisory Board and/or the Linux Foundation, if these conflict requirements 
> that 
> +stem from being part of a Linux Foundation Collaborative Project (e.g 
> requiring 
> +a contributor license agreement). Areas with such requirements cover 
> +trademarks, legal oversight, financial oversight and project funding.

Again, this is a change from the current governance, which just
delegates those things to the AB and leaves it at that (with the
implication that the project as a whole controls its own governance).

IMO it would be better to leave the AB wording as it is, and refer to
a _specific_ LF policy document in the section on the LF.

Or if people want a section like this then it should be a clear list
of exactly which things require approval from which bodies, with no
"such as" or similar, so there is no confusion later.

Cheers,

Tim.

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

Re: [Xen-devel] [XTF PATCH v2] xtf-runner: support two modes for getting output

[Wei Liu]

On Fri, Aug 12, 2016 at 10:51:37AM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [XTF PATCH v2] xtf-runner: support two modes for getting 
> output"):
> > On Thu, Aug 11, 2016 at 07:29:00PM +0100, Ian Jackson wrote:
> > > We don't care when xenconsoled closes the logfile.  We care about when
> > > it last calls write() (with a nonempty buffer).
> > 
> > In logfile mode, no console client is spawned, because it is not
> > reliable -- that's why we use logfile mode in the first place.
> > 
> > And I would rather just add a bodge (wait 1 or 2 seconds)
> 
> That would increase the duration of each test by that 1 or 2 seconds.
> I suppose you could conclude the logfile is complete if it contains
> the expected end-of-run message from the vm, and only poll if not.
> 
> But, it's worse:
> 
> I went to read the xenconsoled source code, and it can handle a domain
> death event before finishing reading all of the data out of a domain's
> ring.
> 
> Maybe this will be mitigated by XTF's printf waiting for the
> xenconsoled ring pointer to catch up ?
> 
> xenconsoled advances the ring pointer before writing to the logfile,
> but (assuming there's no overflow), the write will happen before it
> goes back into the event loop, so it won't be lost.
> 
> > than to rely on sophisticated hack.
> 
> To my mind polling the logfile looking for the final message from the
> vm is something of a hack.
> 
> But indeed I can't see another way to wait for xenconsoled, than
> going behind libxl's back with something like
>  * spawn xl create -p -F
>  * look for the console tty in xenstore
>  * open the console tty
>  * unpause
>  * wait for the console tty to get eof
> 
> This is not a logfile mode at all, of course.  It probably wouldn't be
> easily adaptable to other toolstacks (eg XenServer).
> 

It's only empirical, but my experience of running this patch in logfile
mode (a few dozen times) hasn't seen a single error due to missing output.

I can go for the polling mode but with a timeout of 1 second. A bit
hacky, but hopefully should work good enough.

I want to get something working which I can run in osstest. We can go
with something more sophisticated if the above scheme is deemed not
suitable for osstest.

Wei.

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

Re: [Xen-devel] [PATCH v3 6/9] livepatch: Add parsing for the symbol+0x/

[Jan Beulich]

>>> On 14.08.16 at 23:52,  wrote:
> in case we want to patch at specific offsets inside
> a function. (for example if we want to do NOP patching).
> 
> We also assume that the 'len' is only the size of
> an isns that would be for a call opcode (so 5 bytes
> on x86, and 4 on ARM 32/64).

Which makes the notation using a slash quite confusing: Normally
if we see +/ the length part is assumed
to be the overall size of the object/function the name refers to.
Could I talk you into using a different separator, like colon or
semicolon?

> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -234,14 +234,46 @@ static const char *livepatch_symbols_lookup(unsigned 
> long addr,
>  
>  static int lookup_symbol(struct livepatch_func *f, struct livepatch_elf *elf)
>  {
> +const char *s;
> +char *plus = NULL, *slash = NULL;

Pointless initializers, afaict. Also the latter can be const.

> +unsigned long offset = 0;
> +
>  if ( f->old_addr )
>  return 0;
>  
> +s = f->name;
> +/* +/ */
> +plus = strchr(f->name, '+');
> +if ( plus )
> +{
> +slash = strchr(plus, '/');
> +
> +if ( slash )
> +{
> +const char *endp = NULL;
> +unsigned int len;
> +
> +offset = simple_strtoul(plus+1, , 16);

Missing blanks around +.

> +if ( endp != slash )
> +return -EINVAL;
> +
> +len = simple_strtoul(slash+1, NULL, 16);

Would you perhaps want to be even more precise and verify that
nothing follows this number?

Jan


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

[Xen-devel] [RFC PATCH] tools: remove blktap2 related code and documentation

[Wei Liu]

Blktap2 is effectively dead code for a few years.

Notable changes in this patch:

0. Unhook blktap2 from build system
1. Now libxl no longer supports TAP ask backend, appropriate assertions
   are added and some code paths now return ERROR_FAIL
2. Tap is no longer a supported backend in doc
3. Remove relevant entries in MAINTAINERS

A patch to actually remove blktap2 directory will come later.

Signed-off-by: Wei Liu 
---
Compile-test only at this stage.

Ross, do you have any objection for this? I haven't seen update from the
joint blktap2 maintenance for a few months.

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Shriram Rajagopalan 
Cc: Yang Hongyang 
Cc: Ross Philipson 
Cc: Lars Kurth 
---
 .gitignore  | 14 --
 INSTALL |  4 --
 MAINTAINERS |  2 -
 config/Tools.mk.in  |  1 -
 docs/misc/xl-disk-configuration.txt |  2 +-
 tools/Makefile  |  1 -
 tools/Rules.mk  | 17 +--
 tools/config.h.in   |  6 ---
 tools/configure | 83 
 tools/configure.ac  | 22 -
 tools/libxl/Makefile|  8 +---
 tools/libxl/check-xl-disk-parse |  2 +-
 tools/libxl/libxl.c | 25 ++
 tools/libxl/libxl_blktap2.c | 94 -
 tools/libxl/libxl_device.c  | 32 ++---
 tools/libxl/libxl_dm.c  | 17 ++-
 tools/libxl/libxl_internal.h| 19 
 tools/libxl/libxl_noblktap2.c   | 42 -
 tools/xenstore/hashtable.c  |  5 --
 tools/xenstore/hashtable.h  |  5 --
 tools/xenstore/hashtable_private.h  |  5 --
 21 files changed, 13 insertions(+), 393 deletions(-)
 delete mode 100644 tools/libxl/libxl_blktap2.c
 delete mode 100644 tools/libxl/libxl_noblktap2.c

diff --git a/.gitignore b/.gitignore
index d193820..ea2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -97,19 +97,6 @@ tools/libs/evtchn/headers.chk
 tools/libs/gnttab/headers.chk
 tools/libs/call/headers.chk
 tools/libs/foreignmemory/headers.chk
-tools/blktap2/daemon/blktapctrl
-tools/blktap2/drivers/img2qcow
-tools/blktap2/drivers/lock-util
-tools/blktap2/drivers/qcow-create
-tools/blktap2/drivers/qcow2raw
-tools/blktap2/drivers/tapdisk
-tools/blktap2/drivers/tapdisk-client
-tools/blktap2/drivers/tapdisk-diff
-tools/blktap2/drivers/tapdisk-stream
-tools/blktap2/drivers/tapdisk2
-tools/blktap2/drivers/td-util
-tools/blktap2/vhd/vhd-update
-tools/blktap2/vhd/vhd-util
 tools/console/xenconsole
 tools/console/xenconsoled
 tools/console/client/_paths.h
@@ -327,7 +314,6 @@ tools/libxl/*.pyc
 tools/libxl/libxl-save-helper
 tools/libxl/test_timedereg
 tools/libxl/test_fdderegrace
-tools/blktap2/control/tap-ctl
 tools/firmware/etherboot/eb-roms.h
 tools/firmware/etherboot/gpxe-git-snapshot.tar.gz
 tools/misc/xenwatchdogd
diff --git a/INSTALL b/INSTALL
index 9759354..3b255c7 100644
--- a/INSTALL
+++ b/INSTALL
@@ -144,10 +144,6 @@ this detection and the sysv runlevel scripts have to be 
used.
   --with-systemd=DIR
   --with-systemd-modules-load=DIR
 
-The old backend drivers are disabled because qdisk is now the default.
-This option can be used to build them anyway.
-  --enable-blktap2
-
 Build various stubom components, some are only example code. Its usually
 enough to specify just --enable-stubdom and leave these options alone.
   --enable-ioemu-stubdom
diff --git a/MAINTAINERS b/MAINTAINERS
index 97720a8..d54795b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -322,8 +322,6 @@ M:  Shriram Rajagopalan 
 M: Yang Hongyang 
 S: Maintained
 F: docs/README.remus
-F: tools/blktap2/drivers/block-remus.c
-F: tools/blktap2/drivers/hashtable*
 F: tools/libxl/libxl_remus_*
 F: tools/libxl/libxl_netbuffer.c
 F: tools/libxl/libxl_nonetbuffer.c
diff --git a/config/Tools.mk.in b/config/Tools.mk.in
index 0f79f4e..511406c 100644
--- a/config/Tools.mk.in
+++ b/config/Tools.mk.in
@@ -56,7 +56,6 @@ CONFIG_ROMBIOS  := @rombios@
 CONFIG_SEABIOS  := @seabios@
 CONFIG_QEMU_TRAD:= @qemu_traditional@
 CONFIG_QEMU_XEN := @qemu_xen@
-CONFIG_BLKTAP2  := @blktap2@
 CONFIG_QEMUU_EXTRA_ARGS:= @EXTRA_QEMUU_CONFIGURE_ARGS@
 CONFIG_LIBNL:= @libnl@
 
diff --git a/docs/misc/xl-disk-configuration.txt 
b/docs/misc/xl-disk-configuration.txt
index b3402bc..2e9345c 100644
--- a/docs/misc/xl-disk-configuration.txt
+++ b/docs/misc/xl-disk-configuration.txt
@@ -155,7 +155,7 @@ backendtype=
 

Re: [Xen-devel] [PATCH v2 6/7] libxenstat: honour XEN_RUN_DIR

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v2 6/7] libxenstat: honour XEN_RUN_DIR"):
> On Wed, Jul 20, 2016 at 03:40:56PM +0100, Ian Jackson wrote:
> > Wei Liu writes ("[PATCH v2 6/7] libxenstat: honour XEN_RUN_DIR"):
> > > Backport candidate.
> > 
> > Thanks.  Can you let me know when it's applied to staging ?
> 
> $ git describe --contains eb141d23a00a349dc1474bb982e7812fc091f845
> 4.6.0-rc1~844
> 
> So backport to 4.6 and 4.7.

I have applied it to staging-4.7, as ab75cdf.  In staging-4.6 I had to
fix it up, which was easy.  See below.

From 77a9be97f5238a48f2b73b145c0a7ceaef22e0d6 Mon Sep 17 00:00:00 2001
From: Wei Liu 
Date: Mon, 11 Jul 2016 18:28:08 +0100
Subject: [PATCH] libxenstat: honour XEN_RUN_DIR

This is because libxl uses XEN_RUN_DIR to generate the socket path for
libxenstat while libxenstat itself uses hard-coded path, which is not
necessarily the same path as XEN_RUN_DIR.  The default configuration
happened to work because XEN_RUN_DIR defaulted to /var/run/xen, which
matched the hard-coded path.

We should make libxenstat use XEN_RUN_DIR so that it works with
non-default configuration.

Generate a _paths.h because it is required to make this change work.

Signed-off-by: Wei Liu 
Acked-by: Ian Jackson 
(cherry picked from commit dedb221889dbdd96f1d3c1155c3eb492d329bb53)

(cherry picked from commit ab75cdf635da31012f822260c607f5b4ceaf7668)
Conflicts:
tools/xenstat/libxenstat/src/xenstat_qmp.c
Signed-off-by: Ian Jackson 
---
 .gitignore | 1 +
 tools/xenstat/libxenstat/Makefile  | 7 ++-
 tools/xenstat/libxenstat/src/xenstat_qmp.c | 3 ++-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index 9ead7c4..8c98b01 100644
--- a/.gitignore
+++ b/.gitignore
@@ -199,6 +199,7 @@ tools/xenmon/xentrace_setmask
 tools/xenmon/xenbaked
 tools/xenpaging/xenpaging
 tools/xenpmd/xenpmd
+tools/xenstat/libxenstat/src/_paths.h
 tools/xenstat/xentop/xentop
 tools/xenstore/init-xenstore-domain
 tools/xenstore/xenstore
diff --git a/tools/xenstat/libxenstat/Makefile 
b/tools/xenstat/libxenstat/Makefile
index 850d24a..213d998 100644
--- a/tools/xenstat/libxenstat/Makefile
+++ b/tools/xenstat/libxenstat/Makefile
@@ -40,6 +40,8 @@ LDLIBS-$(CONFIG_SunOS) += -lkstat
 .PHONY: all
 all: $(LIB) $(SHLIB) $(SHLIB_LINKS)
 
+$(OBJECTS-y): src/_paths.h
+
 $(LIB): $(OBJECTS-y)
$(AR) rc $@ $^
$(RANLIB) $@
@@ -135,9 +137,12 @@ endif
 .PHONY: clean
 clean:
rm -f $(LIB) $(SHLIB) $(SHLIB_LINKS) $(OBJECTS-y) \
- $(BINDINGS) $(BINDINGSRC) $(DEPS)
+ $(BINDINGS) $(BINDINGSRC) $(DEPS) src/_paths.h
 
 .PHONY: distclean
 distclean: clean
 
 -include $(DEPS)
+
+genpath-target = $(call buildmakevars2header,src/_paths.h)
+$(eval $(genpath-target))
diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c 
b/tools/xenstat/libxenstat/src/xenstat_qmp.c
index 5e261af..8c35983 100644
--- a/tools/xenstat/libxenstat/src/xenstat_qmp.c
+++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c
@@ -23,6 +23,7 @@
 #include 
 
 #include "xenstat_priv.h"
+#include "_paths.h"
 
 #ifdef HAVE_YAJL_YAJL_VERSION_H
 #  include 
@@ -418,7 +419,7 @@ void read_attributes_qdisk(xenstat_node * node)
free(val);
 
/* Connect to this VMs QMP socket */
-   snprintf(path, sizeof(path), "/var/run/xen/qmp-libxenstat-%i", 
dominfo[i].domain);
+   snprintf(path, sizeof(path), XEN_RUN_DIR "/qmp-libxenstat-%i", 
dominfo[i].domain);
if ((qfd = qmp_connect(path)) < 0) {
continue;
}
-- 
2.1.4


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

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[George Dunlap]

On 15/08/16 11:19, Jan Beulich wrote:
 On 15.08.16 at 11:39,  wrote:
>> On 12/08/16 12:50, Jan Beulich wrote:
>> On 12.08.16 at 11:44,  wrote:
 On 09/08/16 12:30, Jan Beulich wrote:
 On 09.08.16 at 12:48,  wrote:
>> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
>> depriv)"):
>>> Actually, having thought about this some more, and taking this
>>> together with the expectations to the privcmd driver previously
>>> outlined, I think this part is problematic: If all the driver is to know
>>> is the position (within the interface structure) of the target domain
>>> ID, then any guest handles embedded in the interface structure
>>> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
>>> validated, and hence user mode code would have a way to access
>>> or modify kernel memory.
>>
>> Could the hypervisor know the difference between user and kernel
>> memory, in principle ?
>
> Not without further new hypercalls, as the guest kernel would need
> to tell Xen what address ranges are kernel vs user (and that implies
> that any OS wishing to be able to act as Dom0 has a uniform
> separation of address spaces).

 Couldn't Xen tell from the guest pagetables whether the memory being
 accessed was user-mode or kernel mode?
>>>
>>> That would be possible, but would feel like adding heuristics instead
>>> of a proper distinction. Clearly we'd already be in some trouble if
>>> there were cases where some structure doesn't get written to (and
>>> hence could live in user-r/o mapped space), but others would need
>>> to be verified to be user-r/w mapped. A lot of special casing, that is,
>>> and hence of lot of things to be got wrong.
>>>
>>> And then there is the problem of calling code being in rings 1 or 2:
>>> Page tables don't guard ring 0 against such, and we don't even have
>>> the notion of selectors (and hence address ranges) bounding
>>> accessible regions. We can't even say we assume all of them to be
>>> %ds-relative, as it would certainly be legitimate for such a structure
>>> to e.g. live on the stack. Of course an option would be to require
>>> the kernel driver to not allow requests from other than ring 3.
>>>
>>> Plus finally - how would we tell interface structures coming from a
>>> kernel invoked hypercall from those originating from user mode?
>>> There would need to be at least some kind of flag then, which the
>>> privcmd driver set, but normal hypercalls originating in the kernel
>>> don't. Or would you envision to allow this DMOP hypercall to only
>>> be made by user mode tools? If so, does stubdom run its qemu in
>>> ring 3 or rather in ring 0?
>>>
>>> [breaking the order of quoting]
 And unless we're positive the guest kernel will never need these
 hypercalls, we probably need a flag that allows kernel-mode pointers.
>>>
>>> Ah, you actually mention that already.
>>>
>>  (Would it be sufficient to check the starts, or would
>> the ends need to be checked too?)
>
> Both would need to be checked, so the size field(s) would need to
> be locatable too.

 We could have the "fixed" part of the structure contain domid and an
 array of  which the privcmd driver could check.  I don't think
 that would be terrible.
>>>
>>> Doable, yes, but not really nice, especially for the party invoking
>>> the hypercall as well as the backing implementation in Xen (as
>>> opposed to the privcmd driver, for which such a model would likely
>>> work quite well), as they  then can't use the normal, simple reading
>>> of structure fields, but instead would need to populate array
>>> elements in the right order.
>>
>> So on the whole, what would be your suggestion for how to solve the
>> userspace-pointer problem?
> 
> Well, none of the options considered so far are really nice or
> readily available. I think the easiest to use for both the caller and
> the implementation of the hypercall would be the auxiliary
> hypercall for a kernel to indicate user/kernel boundaries plus a
> flag on the DMOP one for the kernel mode driver to indicate its
> user mode origin. The main (purely theoretical afaict) downside
> of this is the difficulty to use it in OSes with variable user/kernel
> boundaries.

What about including in the "fixed" part of the hypercall a virtual
address range that all pointers must be in?  That wouldn't even require
a user/kernel flag actually; and could conceivably be used by the caller
(either userspace or kernel space) to thwart certain kinds of potential
attacks.

It would take changing the copy_guest() macros to include a potential
range argument, but that shouldn't be too intrusive on the whole, I
wouldn't think.

 -George


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

Re: [Xen-devel] [PATCH v3 4/9] livepatch: Sync cache of build-id before using it first time.

[Andrew Cooper]

On 15/08/16 11:38, Jan Beulich wrote:
 On 14.08.16 at 23:52,  wrote:
>> We don't print at bootup time the build-id. The reason is
>> that xen_build_init and livepatch_init are both __initcall
>> type routines. This meant that when livepatch_init called
>> xen_build_id, it would return -ENODATA as build_id_len was
>> not setup yet (b/c xen_build_init would be called later).
>>
>> We fix this by calling xen_build_init in livepatch_init which
>> allows us to print the build-id of the hypervisor.
>>
>> We also keep xen_build_init as __initcall because build-id
>> can be built without livepatching being enabled (so
>> no livepatch_init being called).
> Now if the build ID is potentially useful outside of live patching,
> shouldn't its logging also be done outside of livepatch code?

IMO the Build ID should be available and printed irrespective of live
patching.

It is useful information simply to identify the hypervisor.

~Andrew

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

[Xen-devel] [OSSTEST PATCH] ts-coverity-upload: Double coverity upload timeout

[Ian Jackson]

From 3600s to 7200s.  It failed in flight 100483 in the Mass colo,
with a timeout, having uploaded 95% (151Mby) in 3600s.

Ideally we would like to release the build host, but there is no
infrastructure for doing that right now.

Signed-off-by: Ian Jackson 
---
 ts-coverity-upload | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ts-coverity-upload b/ts-coverity-upload
index 6d43cf8..219c8be 100755
--- a/ts-coverity-upload
+++ b/ts-coverity-upload
@@ -43,7 +43,7 @@ sub upload() {
 
 my @args = map { ("--form", $_) } @form_args;
 
-push @args, qw(--max-time 3600);
+push @args, qw(--max-time 7200);
 push @args, qw(--fail); # turn 404 etc into a failure.
 push @args, $r{coverity_submit_url};
 
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v3 5/9] livepatch: Move code from prepare_payload to own routine

[Jan Beulich]

>>> On 14.08.16 at 23:52,  wrote:
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -232,6 +232,29 @@ static const char *livepatch_symbols_lookup(unsigned 
> long addr,
>  return n;
>  }
>  
> +static int lookup_symbol(struct livepatch_func *f, struct livepatch_elf *elf)

The latter one can and hence should be const.

Jan


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

Re: [Xen-devel] [PATCH v3 4/9] livepatch: Sync cache of build-id before using it first time.

[Jan Beulich]

>>> On 14.08.16 at 23:52,  wrote:
> We don't print at bootup time the build-id. The reason is
> that xen_build_init and livepatch_init are both __initcall
> type routines. This meant that when livepatch_init called
> xen_build_id, it would return -ENODATA as build_id_len was
> not setup yet (b/c xen_build_init would be called later).
> 
> We fix this by calling xen_build_init in livepatch_init which
> allows us to print the build-id of the hypervisor.
> 
> We also keep xen_build_init as __initcall because build-id
> can be built without livepatching being enabled (so
> no livepatch_init being called).

Now if the build ID is potentially useful outside of live patching,
shouldn't its logging also be done outside of livepatch code?

Jan


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

Re: [Xen-devel] [PATCH v3 3/9] version/livepatch: Move xen_build_id_check to version.h

[Jan Beulich]

>>> On 14.08.16 at 23:52,  wrote:
> It makes more sense for it to be there. However that
> means the version.h has now a dependency on 
> as the Elf_Note is a macro.
> 
> The elfstructs.h has a dependency on types.h as well so
> we need that. We cannot put that #include 
> in elfstructs.h as that file is used by tools and they
> do not have such file.

I think that's acceptable, as the number of places this header gets
included is pretty limited. Hence the alternative of making the
declaration conditional upon Elf_Note being defined is likely the
uglier one (as it would impose ordering constraints on the #include-s
used by respective source files).

> Signed-off-by: Konrad Rzeszutek Wilk 

Reviewed-by: Jan Beulich 


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

Re: [Xen-devel] [PATCH v3 2/9] livepatch: Deal with payloads without any .text

[Jan Beulich]

>>> On 14.08.16 at 23:52,  wrote:
> It is possible. Especially if the only thing they do is
> NOP functions - in which case there is only .livepatch.funcs
> sections.
> 
> Signed-off-by: Konrad Rzeszutek Wilk 

Reviewed-by: Jan Beulich 


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

Re: [Xen-devel] [PATCH v3 1/9] livepatch: Clear .bss when payload is reverted

[Jan Beulich]

>>> On 14.08.16 at 23:52,  wrote:
> @@ -374,7 +376,18 @@ static int move_payload(struct payload *payload, struct 
> livepatch_elf *elf)
>  elf->name, elf->sec[i].name, elf->sec[i].load_addr);
>  }
>  else
> -memset(elf->sec[i].load_addr, 0, elf->sec[i].sec->sh_size);
> +{
> +/* We expect only one BSS. */
> +if ( payload->bss )
> +{
> +rc = -EINVAL;

-EOPNOTSUPP according to e.g. the only- one-symbol-table code.

> +goto out;
> +}
> +payload->bss = elf->sec[i].load_addr;
> +payload->bss_size = elf->sec[i].sec->sh_size;
> +
> +memset(payload->bss, 0, payload->bss_size);
> +}
>  }
>  }
>  
> @@ -1034,6 +1047,9 @@ static int revert_payload(struct payload *data)
>  list_del_rcu(>applied_list);
>  unregister_virtual_region(>region);
>  
> +/* And clear the BSS for subsequent operation. */
> +memset(data->bss, 0, data->bss_size);

Instead of doing it in two places, how about moving this into
apply_payload()?

Jan


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

Re: [Xen-devel] [RFC 00/22] xen/arm: Rework the P2M code to follow break-before-make sequence

[Julien Grall]

Hi Stefano,

Do you have any comments on this series?

Cheers,

On 28/07/2016 16:51, Julien Grall wrote:

Hello all,

The ARM architecture mandates the use of a break-before-make sequence when
changing translation entries if the page table is shared between multiple
CPUs whenever a valid entry is replaced by another valid entry (see D4.7.1
in ARM DDI 0487A.j for more details).

The current P2M code does not respect this sequence and may result to
break coherency on some processors.

Adapting the current implementation to use break-before-make sequence would
imply some code duplication and more TLBs invalidations than necessary.
For instance, if we are replacing a 4KB page and the current mapping in
the P2M is using a 1GB superpage, the following steps will happen:
1) Shatter the 1GB superpage into a series of 2MB superpages
2) Shatter the 2MB superpage into a series of 4KB superpages
3) Replace the 4KB page

As the current implementation is shattering while descending and install
the mapping before continuing to the next level, Xen would need to issue 3
TLB invalidation instructions which is clearly inefficient.

Furthermore, all the operations which modify the page table are using the
same skeleton. It is more complicated to maintain different code paths than
having a generic function that set an entry and take care of the break-before-
make sequence.

The new implementation is based on the x86 EPT one which, I think, fits
quite well for the break-before-make sequence whilst keeping the code
simple.

I sent this patch series as an RFC because there are still some TODOs
in the code (mostly sanity check and possible optimization) and I have
done limited testing. However, I think it is a good shape to start reviewing,
get more feedback and have wider testing on different board.

Also, I need to figure out the impact on ARM32 because the domheap is not
always mapped.

This series has dependencies on some rework sent separately ([1] and [2]).
I have provided a branch with all the dependencies and this series applied:

git://xenbits.xen.org/people/julieng/xen-unstable.git branch p2m-rfc

Comments are welcome.

Yours sincerely,

Cc: Razvan Cojocaru 
Cc: Tamas K Lengyel 
Cc: Shanker Donthineni 
Cc: Dirk Behme 
Cc: Edgar E. Iglesias 

[1] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg02936.html
[2] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg02830.html

Julien Grall (22):
  xen/arm: do_trap_instr_abort_guest: Move the IPA computation out of
the switch
  xen/arm: p2m: Store in p2m_domain whether we need to clean the entry
  xen/arm: p2m: Rename parameter in p2m_{remove,write}_pte...
  xen/arm: p2m: Use typesafe gfn in p2m_mem_access_radix_set
  xen/arm: traps: Move MMIO emulation code in a separate helper
  xen/arm: traps: Check the P2M before injecting a data/instruction
abort
  xen/arm: p2m: Rework p2m_put_l3_page
  xen/arm: p2m: Invalidate the TLBs when write unlocking the p2m
  xen/arm: p2m: Change the type of level_shifts from paddr_t to unsigned
int
  xen/arm: p2m: Move the lookup helpers at the top of the file
  xen/arm: p2m: Introduce p2m_get_root_pointer and use it in
__p2m_lookup
  xen/arm: p2m: Introduce p2m_get_entry and use it to implement
__p2m_lookup
  xen/arm: p2m: Replace all usage of __p2m_lookup with p2m_get_entry
  xen/arm: p2m: Re-implement p2m_cache_flush using p2m_get_entry
  xen/arm: p2m: Re-implement relinquish_p2m_mapping using p2m_get_entry
  xen/arm: p2m: Make p2m_{valid,table,mapping} helpers inline
  xen/arm: p2m: Introduce a helper to check if an entry is a superpage
  xen/arm: p2m: Introduce p2m_set_entry and __p2m_set_entry
  xen/arm: p2m: Re-implement p2m_remove_using using p2m_set_entry
  xen/arm: p2m: Re-implement p2m_insert_mapping using p2m_set_entry
  xen/arm: p2m: Re-implement p2m_set_mem_access using
p2m_{set,get}_entry
  xen/arm: p2m: Do not handle shattering in p2m_create_table

 xen/arch/arm/domain.c  |8 +-
 xen/arch/arm/p2m.c | 1274 ++--
 xen/arch/arm/traps.c   |  126 +++--
 xen/include/asm-arm/p2m.h  |   14 +
 xen/include/asm-arm/page.h |4 +
 5 files changed, 742 insertions(+), 684 deletions(-)



--
Julien Grall

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

Re: [Xen-devel] [PATCH v2] Remove ambiguities in the COPYING file; add CONTRIBUTING file

[Andrew Cooper]

On 12/08/16 10:37, Lars Kurth wrote:
> COPYING file:
> The motivation of this change is to make it easier for new
> contributors to conduct a license and patent review, WITHOUT
> changing any licenses.
> - Remove references to BSD-style licenses as we have more
>   common license exceptions and replace with "other license
>   stanzas"
> - List the most common situations under which code is licensed
>   under licenses other than GPLv2 (section "Licensing Exceptions")
> - List the most common non-GPLv2 licenses that are in use in
>   this repository based on a recent FOSSology scan (section
>   "Licensing Exceptions")
> - List other license related conventions within the project
>   to make it easier to conduct a license review.
> - Clarify the incoming license as its omission has confused
>   past contributors (section "Contributions")
>
> CONTRIBUTION file:
> The motivation of this file is to make it easier for contributors
> to find contribution related resources. Add information on existing
> license related conventions to avoid unintentional future licensing
> issues. Provide templates for copyright headers for the most commonly
> used licenses in this repository.
>
> Signed-off-by: Lars Kurth 

Reviewed-by: Andrew Cooper , with one style
correction.

> diff --git a/CONTRIBUTING b/CONTRIBUTING
> new file mode 100644
> index 000..67ecdb7
> --- /dev/null
> +++ b/CONTRIBUTING
> @@ -0,0 +1,210 @@
> 
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
> + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY 
> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, 
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE 
> + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + */
> \ No newline at end of file

Newline at the end.

~Andrew

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

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 15.08.16 at 11:39,  wrote:
> On 12/08/16 12:50, Jan Beulich wrote:
> On 12.08.16 at 11:44,  wrote:
>>> On 09/08/16 12:30, Jan Beulich wrote:
>>> On 09.08.16 at 12:48,  wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> Actually, having thought about this some more, and taking this
>> together with the expectations to the privcmd driver previously
>> outlined, I think this part is problematic: If all the driver is to know
>> is the position (within the interface structure) of the target domain
>> ID, then any guest handles embedded in the interface structure
>> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
>> validated, and hence user mode code would have a way to access
>> or modify kernel memory.
>
> Could the hypervisor know the difference between user and kernel
> memory, in principle ?

 Not without further new hypercalls, as the guest kernel would need
 to tell Xen what address ranges are kernel vs user (and that implies
 that any OS wishing to be able to act as Dom0 has a uniform
 separation of address spaces).
>>>
>>> Couldn't Xen tell from the guest pagetables whether the memory being
>>> accessed was user-mode or kernel mode?
>> 
>> That would be possible, but would feel like adding heuristics instead
>> of a proper distinction. Clearly we'd already be in some trouble if
>> there were cases where some structure doesn't get written to (and
>> hence could live in user-r/o mapped space), but others would need
>> to be verified to be user-r/w mapped. A lot of special casing, that is,
>> and hence of lot of things to be got wrong.
>> 
>> And then there is the problem of calling code being in rings 1 or 2:
>> Page tables don't guard ring 0 against such, and we don't even have
>> the notion of selectors (and hence address ranges) bounding
>> accessible regions. We can't even say we assume all of them to be
>> %ds-relative, as it would certainly be legitimate for such a structure
>> to e.g. live on the stack. Of course an option would be to require
>> the kernel driver to not allow requests from other than ring 3.
>> 
>> Plus finally - how would we tell interface structures coming from a
>> kernel invoked hypercall from those originating from user mode?
>> There would need to be at least some kind of flag then, which the
>> privcmd driver set, but normal hypercalls originating in the kernel
>> don't. Or would you envision to allow this DMOP hypercall to only
>> be made by user mode tools? If so, does stubdom run its qemu in
>> ring 3 or rather in ring 0?
>> 
>> [breaking the order of quoting]
>>> And unless we're positive the guest kernel will never need these
>>> hypercalls, we probably need a flag that allows kernel-mode pointers.
>> 
>> Ah, you actually mention that already.
>> 
>  (Would it be sufficient to check the starts, or would
> the ends need to be checked too?)

 Both would need to be checked, so the size field(s) would need to
 be locatable too.
>>>
>>> We could have the "fixed" part of the structure contain domid and an
>>> array of  which the privcmd driver could check.  I don't think
>>> that would be terrible.
>> 
>> Doable, yes, but not really nice, especially for the party invoking
>> the hypercall as well as the backing implementation in Xen (as
>> opposed to the privcmd driver, for which such a model would likely
>> work quite well), as they  then can't use the normal, simple reading
>> of structure fields, but instead would need to populate array
>> elements in the right order.
> 
> So on the whole, what would be your suggestion for how to solve the
> userspace-pointer problem?

Well, none of the options considered so far are really nice or
readily available. I think the easiest to use for both the caller and
the implementation of the hypercall would be the auxiliary
hypercall for a kernel to indicate user/kernel boundaries plus a
flag on the DMOP one for the kernel mode driver to indicate its
user mode origin. The main (purely theoretical afaict) downside
of this is the difficulty to use it in OSes with variable user/kernel
boundaries.

Jan

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

Re: [Xen-devel] [RFC 18/22] xen/arm: p2m: Introduce p2m_set_entry and __p2m_set_entry

[Sergej Proskurin]

Hi Julien,

[...]

> +static int p2m_set_entry(struct p2m_domain *p2m,
> + gfn_t sgfn,
> + unsigned long todo,
> + mfn_t smfn,
> + p2m_type_t t,
> + p2m_access_t a)
> +{
> +int rc = 0;
> +
> +while ( todo )
> +{
> +unsigned long mask = gfn_x(sgfn) | mfn_x(smfn) | todo;
> +unsigned long order;
> +
> +/* Always map 4k by 4k when memaccess is enabled */
> +if ( unlikely(p2m->mem_access_enabled) )
> +order = THIRD_ORDER;
> +if ( !(mask & ((1UL << FIRST_ORDER) - 1)) )
> +order = FIRST_ORDER;

The 2nd outer if statement does not consider p2m->mem_access_enabled.
That is, even if p2m->mem_access_enabled is set, the 2nd if statement
would set an order, which is potentially not equal to the THIRD_ORDER
(please use the THIRD_ORDER define in the ASSERT -- see ASSERT beyond).

As far as I understand, p2m->mem_access_enabled should prevent mapping
of superpages. With this implementation, however, this would not be the
case. Even worse: the ASSERT in __p2m_set_entry would crash the system,
if p2m->mem_access_enabled was set and order was not equal to the
THIRD_ORDER:

+/*
+ * The radix-tree can only work on 4KB. This is only used when
+ * memaccess is enabled.
+ */
+ASSERT(!p2m->mem_access_enabled || page_order == 0);

This can be fixed, by simply unifying both outer if statements:

+/* Always map 4k by 4k when memaccess is enabled */
+if ( unlikely(p2m->mem_access_enabled) )
+order = THIRD_ORDER;
+else if ( !(mask & ((1UL << FIRST_ORDER) - 1)) )
+order = FIRST_ORDER;
+else if ( !(mask & ((1UL << SECOND_ORDER) - 1)) )
+order = SECOND_ORDER;
+else
+order = THIRD_ORDER;


> +else if ( !(mask & ((1UL << SECOND_ORDER) - 1)) )
> +order = SECOND_ORDER;
> +else
> +order = THIRD_ORDER;
> +
> +rc = __p2m_set_entry(p2m, sgfn, order, smfn, t, a);
> +if ( rc )
> +break;
> +
> +sgfn = gfn_add(sgfn, (1 << order));
> +if ( !mfn_eq(smfn, INVALID_MFN) )
> +   smfn = mfn_add(smfn, (1 << order));
> +
> +todo -= (1 << order);
> +}
> +
> +return rc;
> +}
> +#endif
> +
>  /*
>   * Returns true if start_gpaddr..end_gpaddr contains at least one
>   * suitably aligned level_size mappping of maddr.
> 

Best regards,
~Sergej

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

Re: [Xen-devel] [PATCH 2/2] make use of .startof.() and .sizeof.() assembler expressions

[Jan Beulich]

>>> On 15.08.16 at 11:43,  wrote:
> On 12/08/16 15:48, Jan Beulich wrote:
>> Section start symbols frequently obscure the actual symbol name living
>> at the start of the section. Eliminate them where they can be replaced
>> by linker resolved .startof.* symbols. (Section end symbols may have
>> the same undesirable effect, but they're less easy to eliminate, as
>> they'd need to be represented by the sum of .startof. and
>> .sizeof.).
>>
>> Along those lines use .sizeof.* where section sizes get calculated
>> from the difference of section and and section start symbols.
>>
>> Note that this would be nice for the build-id section too, but:
>> - The generated (by ld) section names differ between ELF and COFF/PE
>>   (ELF: .note.gnu.build-id; COFF/PE[2.26+]: .buildid), yet we compile
>>   version.c just once (and hence can use only either of the necessary
>>   .startof./.sizeof. forms).
>> - The ELF section name needs to be quoted in the .startof./.sizeof.
>>   expressions, yet that quoting is supported only by gas 2.26+.
>>
>> Signed-off-by: Jan Beulich 
> 
> Is there any documentation of these?  I cant seem to find any.

Indeed I can't find any either, at least not inside the binutils
repo. But the support for this has been there for a long time
(it is present in 2.16.1, which is the oldest I have readily available).

> Does this work on Clang at all?

To be honest - I have no idea.

Jan


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

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

[osstest service owner]

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

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  01fe4da6243be692b972de4ec7b22ec92527c0f5
baseline version:
 xen  a55ad65d3a30d5b3a026a7481ce05f28065920f0

Last test of basis   100468  2016-08-12 16:00:59 Z2 days
Testing same since   100490  2016-08-15 09:01:58 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 

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=01fe4da6243be692b972de4ec7b22ec92527c0f5
+ . ./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 
01fe4da6243be692b972de4ec7b22ec92527c0f5
+ branch=xen-unstable-smoke
+ revision=01fe4da6243be692b972de4ec7b22ec92527c0f5
+ . ./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-4.7-testing
+ '[' x01fe4da6243be692b972de4ec7b22ec92527c0f5 = 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 

Re: [Xen-devel] [PATCH 6/7] arm64: xen: Enable user access before a privcmd hvc call

[Julien Grall]

Hi Catalin,

I have CCed Stefano who is maintaining the Xen ARM code in Linux.

On 12/08/2016 17:27, Catalin Marinas wrote:

Privcmd calls are issued by the userspace. The kernel needs to enable
access to TTBR0_EL1 as the hypervisor would issue stage 1 translations
to user memory via AT instructions. Since AT instructions are not
affected by the PAN bit (ARMv8.1), we only need the explicit
uaccess_enable/disable if the TTBR0 PAN option is enabled.

Cc: Julien Grall 
Cc: Will Deacon 
Cc: James Morse 
Cc: Kees Cook 
Signed-off-by: Catalin Marinas 


Reviewed-by: Julien Grall 

Regards,


---
 arch/arm64/xen/hypercall.S | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
index 329c8027b0a9..4c509f4f4dcc 100644
--- a/arch/arm64/xen/hypercall.S
+++ b/arch/arm64/xen/hypercall.S
@@ -91,6 +91,24 @@ ENTRY(privcmd_call)
mov x2, x3
mov x3, x4
mov x4, x5
+#ifdef CONFIG_ARM64_TTBR0_PAN
+   /*
+* Privcmd calls are issued by the userspace. The kernel needs to
+* enable access to TTBR0_EL1 as the hypervisor would issue stage 1
+* translations to user memory via AT instructions. Since AT
+* instructions are not affected by the PAN bit (ARMv8.1), we only
+* need the explicit uaccess_enable/disable if the TTBR0 PAN option is
+* enabled.
+*/
+   uaccess_enable x6, x7, x8
+#endif
hvc XEN_IMM
+
+#ifdef CONFIG_ARM64_TTBR0_PAN
+   /*
+* Disable userspace access from kernel once the hyp call completed.
+*/
+   uaccess_disable x6
+#endif
ret
 ENDPROC(privcmd_call);

___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 1/2] x86emul: fold SrcImmByte fetching

[Andrew Cooper]

On 12/08/16 16:07, Jan Beulich wrote:
> There's no need for having identical code spelled out twice.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

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