Re: [Xen-devel] [PATCH v1 04/13] x86: implement data structure and CPU init flow for MBA

[Yi Sun]

On 17-08-15 11:50:15, Wei Liu wrote:
> On Wed, Aug 09, 2017 at 03:41:43PM +0800, Yi Sun wrote:
> > -#define PSR_CAT(1<<1)
> > -#define PSR_CDP(1<<2)
> > +#define PSR_CMT(1u << 0)
> > +#define PSR_CAT(1u << 1)
> > +#define PSR_CDP(1u << 2)
> > +#define PSR_MBA(1u << 3)
> 
> I would split this part out to a separate patch so that it can be
> applied immediately.
> 
A fix patch has been sent out.

> >  struct feat_node {
> > -/* cos_max and cbm_len are common values for all features so far. */
> > +/* cos_max is common values for all features so far. */
> >  unsigned int cos_max;
> > -unsigned int cbm_len;
> > +
> > +/* Feature specific HW info. */
> > +union {
> > +struct {
> > +/* The length of CBM got through CPUID. */
> > +unsigned int cbm_len;
> > +} cat_info;
> > +
> > +struct {
> > +/* The max throttling value got through CPUID. */
> > +unsigned int thrtl_max;
> > +unsigned int linear;
> > +} mba_info;
> > +};
> > +
> 
> I suggest you add a tag to specify which struct is in effect in the
> union and ASSERT accordingly in their respective type specific
> functions.

Before using struct in this union at all places, there is check to make sure the
correct feature node is used. So, is it necessary to add an additional flag to
check? Thanks!

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

Re: [Xen-devel] [patch v6] vt-d: fix vf of rc integrated pf matched to wrong vt-d unit

[Roger Pau Monné]

on wed, aug 16, 2017 at 01:12:24pm +0800, chao gao wrote:
> the problem is for a vf of rc integrated pf (e.g. pf's bdf is
> 00:02.0), we would wrongly use 00:00.0 to search vt-d unit.
> 
> if a pf is an extended function, the bdf of a traditional function
> within the same device should be used to search vt-d unit. otherwise,
> the real bdf of pf should be used. according pci-e spec, an extended
> function is a function within an ari device and function number is
> greater than 7.

AFAIK, extended functions simply remove the slot and extend the
function number to [0, 255], so it seems correct to expect that the
VT-d unit search should be done using the bus and extended function
parameters, and assume slot is 0. Is this some kind of limitation of
VT-d?

> The original code tried to tell apart Extended
> Function and non-Extended Function through checking PCI_SLOT(),
> missing counterpart of pci_ari_enabled() (this function exists in
> linux kernel) compared to linux kernel. Without checking whether ARI
> is enabled, it incurs a RC integrated PF with PCI_SLOT() >0 is wrongly
> classified to an extended function. Note that a RC integrated function
> isn't within an ARI device and thus cannot be extended function and in
> this case the real BDF should be used.
> 
> This patch introduces a new field, pf_is_extfn, in struct
> pci_dev_info, to indicate whether the physical function is an extended
> function. The new field helps to generate correct BDF to search VT-d
> unit.

[...]
> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
> index 59b6e8a..9e76aa0 100644
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -40,6 +40,7 @@
>  
>  struct pci_dev_info {
>  bool_t is_extfn;
> +bool_t pf_is_extfn; /* Only valid for virtual function */

Can't you just re-use is_virtfn and is_extfn, and when both are true
it means the pf where this vf belongs is an extended function?

>  bool_t is_virtfn;
>  struct {
>  u8 bus;
> -- 
> 1.8.3.1
> 

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

Re: [Xen-devel] [patch v6] vt-d: fix vf of rc integrated pf matched to wrong vt-d unit

[Chao Gao]

On Wed, Aug 16, 2017 at 09:17:46AM +0100, Roger Pau Monné wrote:
>on wed, aug 16, 2017 at 01:12:24pm +0800, chao gao wrote:
>> the problem is for a vf of rc integrated pf (e.g. pf's bdf is
>> 00:02.0), we would wrongly use 00:00.0 to search vt-d unit.
>> 
>> if a pf is an extended function, the bdf of a traditional function
>> within the same device should be used to search vt-d unit. otherwise,
>> the real bdf of pf should be used. according pci-e spec, an extended
>> function is a function within an ari device and function number is
>> greater than 7.
>
>AFAIK, extended functions simply remove the slot and extend the
>function number to [0, 255], so it seems correct to expect that the
>VT-d unit search should be done using the bus and extended function
>parameters, and assume slot is 0. Is this some kind of limitation of
>VT-d?

VT-d spec makes such provision for VT-d unit search without any
explaination. But I think it isn't. Whether we can find the right VT-d unit
depends on DMAR. So I would rather regard it as firmware doesn't prepare
entries for extended functions in DMAR.

>
>> The original code tried to tell apart Extended
>> Function and non-Extended Function through checking PCI_SLOT(),
>> missing counterpart of pci_ari_enabled() (this function exists in
>> linux kernel) compared to linux kernel. Without checking whether ARI
>> is enabled, it incurs a RC integrated PF with PCI_SLOT() >0 is wrongly
>> classified to an extended function. Note that a RC integrated function
>> isn't within an ARI device and thus cannot be extended function and in
>> this case the real BDF should be used.
>> 
>> This patch introduces a new field, pf_is_extfn, in struct
>> pci_dev_info, to indicate whether the physical function is an extended
>> function. The new field helps to generate correct BDF to search VT-d
>> unit.
>
>[...]
>> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
>> index 59b6e8a..9e76aa0 100644
>> --- a/xen/include/xen/pci.h
>> +++ b/xen/include/xen/pci.h
>> @@ -40,6 +40,7 @@
>>  
>>  struct pci_dev_info {
>>  bool_t is_extfn;
>> +bool_t pf_is_extfn; /* Only valid for virtual function */
>
>Can't you just re-use is_virtfn and is_extfn, and when both are true
>it means the pf where this vf belongs is an extended function?

Yes. Reuse vf's is_extfn field is possible.

Thanks
Chao

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

Re: [Xen-devel] [PATCH v8 11/13] arm/mem_access: Add long-descriptor based gpt

[Sergej Proskurin]

Hi all,

On 08/16/2017 12:28 AM, Andrew Cooper wrote:
> On 15/08/2017 23:25, Stefano Stabellini wrote:
>> On Tue, 15 Aug 2017, Julien Grall wrote:
>>> On 14/08/17 22:03, Sergej Proskurin wrote:
 Hi Julien,

 On 08/14/2017 07:37 PM, Julien Grall wrote:
> Hi Sergej,
>
> On 09/08/17 09:20, Sergej Proskurin wrote:
>> +/*
>> + * According to to ARM DDI 0487B.a J1-5927, we return an error if
>> the found
> Please drop one of the 'to'. The rest looks good to me.
>
 Great, thanks. I will remove the second "to" in v9. Would that be an
 Acked-by or shall I tag this patch with a Reviewed-by you?
>>> Acked-by. FIY, you still missing an acked from "The REST" for patch #7, the
>>> rest looks fully acked.
>> I acked patch #7, but patch #8 breaks the build on ARM:
>>
>>
>> In file included from 
>> /local/repos/xen-upstream/xen/include/xen/guest_access.h:10:0,
>>  from device_tree.c:15:
>> /local/repos/xen-upstream/xen/include/asm/guest_access.h:14:32: error: 
>> 'struct domain' declared inside parameter list [-Werror]
>> uint32_t size, bool_t is_write);
>> ^
>> /local/repos/xen-upstream/xen/include/asm/guest_access.h:14:32: error: its 
>> scope is only this definition or declaration, which is probably not what you 
>> want [-Werror]
>> cc1: all warnings being treated as errors
>> make[4]: *** [device_tree.o] Error 1
>>
>>
>> Am I missing anything?
> Possibly a result of Wei's recent patch
> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=de62402a9c2e403b049aa238b4fa4e2d618e8870
> which is newer than the posting of this series.
>

Thank you for bringing that up. Since Wei has removed a forward
declaration to struct domain in , my patch series failed to
build right after rebasing to staging. By following Wei's approach,
adding a forward declaration to struct domain in 
fixes the upper issue. I will address this issue separately in patch 08/13.

Thanks,
~Sergej


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

Re: [Xen-devel] [patch v6] vt-d: fix vf of rc integrated pf matched to wrong vt-d unit

[Roger Pau Monné]

On Wed, Aug 16, 2017 at 04:50:51PM +0800, Chao Gao wrote:
> On Wed, Aug 16, 2017 at 09:17:46AM +0100, Roger Pau Monné wrote:
> >on wed, aug 16, 2017 at 01:12:24pm +0800, chao gao wrote:
> >> the problem is for a vf of rc integrated pf (e.g. pf's bdf is
> >> 00:02.0), we would wrongly use 00:00.0 to search vt-d unit.
> >> 
> >> if a pf is an extended function, the bdf of a traditional function
> >> within the same device should be used to search vt-d unit. otherwise,
> >> the real bdf of pf should be used. according pci-e spec, an extended
> >> function is a function within an ari device and function number is
> >> greater than 7.
> >
> >AFAIK, extended functions simply remove the slot and extend the
> >function number to [0, 255], so it seems correct to expect that the
> >VT-d unit search should be done using the bus and extended function
> >parameters, and assume slot is 0. Is this some kind of limitation of
> >VT-d?
> 
> VT-d spec makes such provision for VT-d unit search without any
> explaination. But I think it isn't. Whether we can find the right VT-d unit
> depends on DMAR. So I would rather regard it as firmware doesn't prepare
> entries for extended functions in DMAR.

Looking again at the proposed changes in acpi_find_matched_drhd_unit,
it seems fine. If the VF belongs to a PF that uses extended functions
just use 0 as devfn, which is the devfn of the PF itself unless I'm
mistaken. It's just that I find the commit log very hard to
parse/understand.

Roger.

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

Re: [Xen-devel] [PATCH for-2.10 v2 1/2] hw/acpi: Call acpi_set_pci_info when no ACPI tables needed

[Igor Mammedov]

On Tue, 15 Aug 2017 22:24:08 +0300
"Michael S. Tsirkin"  wrote:

> On Tue, Aug 15, 2017 at 02:07:51PM +0200, Igor Mammedov wrote:
> > On Tue, 15 Aug 2017 12:15:48 +0100
> > Anthony PERARD  wrote:
> >   
> > > To do PCI passthrough with Xen, the property acpi-pcihp-bsel needs to be
> > > set, but this was done only when ACPI tables are built which is not
> > > needed for a Xen guest. The need for the property starts with commit
> > > "pc: pcihp: avoid adding ACPI_PCIHP_PROP_BSEL twice"
> > > (f0c9d64a68b776374ec4732424a3e27753ce37b6).
> > > 
> > > Set pci info before checking for the needs to build ACPI tables.
> > > 
> > > Assign bsel=0 property only to the root bus on Xen as there is no
> > > support in the Xen ACPI tables for a different value.  
> > 
> > looking at hw/acpi/pcihp.c and bsel usage there it looks like
> > bsel property is owned by it and not by ACPI tables, so instead of
> > shuffling it in acpi_setup(), how about moving bsel initialization
> > to hw/acpi/pcihp.c and initialize it there unconditionally?
> > 
> > It could be as simple as moving acpi_set_pci_info()/acpi_set_bsel()
> > there and calling it from acpi_pcihp_reset().
> > 
> > Then there won't be need for Xen specific branches, as root bus
> > will have bsel set automatically which is sufficient for Xen and
> > the rest of bsel-s (bridges) will be just unused by Xen,
> > which could later extend its ACPI table implementation to utilize them.   
> 
> Later is exactly what I'd like to try to avoid.
> Whoever wants acpi hotplug for bridges needs to get
> the bsel info from qemu supplied acpi tables.

I'd prefer to have only one behavior in QEMU (on hw interface)
side and let Xen to maintain their own ACPI tables  dealing
with issues that arise from it since they insist on doing job twice.

The point is bsel is so embedded in HW part of impl.
that it should be allocated/manged there, otherwise it leads
to hacks where acpi_setup() is called but does partial init
and then bails out to fix code pcihp.c that depend on it being run,
pcihp.c (hw part) shouldn't depend on on ACPI tables generation
(bios part).

Anyway if you insist on capping Xen, it probably could be done
with comat machinery, something like this:

(where the 1st hunk should been there since, we've introduced
"acpi-pci-hotplug-with-bridge-support")

diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index c420a38..a55f022 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -273,7 +273,7 @@ static void pci_write(void *opaque, hwaddr addr, uint64_t 
data,
   addr, data);
 break;
 case PCI_SEL_BASE:
-s->hotplug_select = data;
+s->hotplug_select = s->legacy_piix ? 0 : data;
 ACPI_PCIHP_DPRINTF("pcisel write %" HWADDR_PRIx " <== %" PRIu64 "\n",
   addr, data);
 default:
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 22dbef6..81b8c3e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -1117,6 +1117,13 @@ static void xenfv_machine_options(MachineClass *m)
 m->max_cpus = HVM_MAX_VCPUS;
 m->default_machine_opts = "accel=xen";
 m->hot_add_cpu = pc_hot_add_cpu;
+SET_MACHINE_COMPAT(m,
+{\
+.driver   = "PIIX4_PM",\
+.property = "acpi-pci-hotplug-with-bridge-support",\
+.value= "off",\
+},
+);
 }
 
 DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,



> > > Reported-by: Sander Eikelenboom 
> > > Signed-off-by: Anthony PERARD 
> > > 
> > > ---
> > > Changes in V2:
> > >   - check for acpi_enabled before calling acpi_set_pci_info.
> > >   - set the property on the root bus only.
> > > 
> > > This patch would be a canditade to backport to 2.9.
> > > 
> > > CC: Stefano Stabellini 
> > > CC: Bruce Rogers 
> > > ---
> > >  hw/i386/acpi-build.c | 25 -
> > >  1 file changed, 16 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index 98dd424678..c0483b96cf 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -46,6 +46,7 @@
> > >  #include "sysemu/tpm_backend.h"
> > >  #include "hw/timer/mc146818rtc_regs.h"
> > >  #include "sysemu/numa.h"
> > > +#include "hw/xen/xen.h"
> > >  
> > >  /* Supported chipsets: */
> > >  #include "hw/acpi/piix4.h"
> > > @@ -518,8 +519,13 @@ static void acpi_set_pci_info(void)
> > >  unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT;
> > >  
> > >  if (bus) {
> > > -/* Scan all PCI buses. Set property to enable acpi based 
> > > hotplug. */
> > > -pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, 
> > > &bsel_alloc);
> > > +if (xen_enabled()) {
> > > +/* Assign BSEL property to root bus only. */
> > > +acpi_set_bsel(bus, &bsel_alloc);
> > > +} else {
> > > +/* Scan all PCI buses. Set property to enable acpi based 
> > > hotplug. */
> > > +pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, 
> > > &bsel_a

Re: [Xen-devel] [PATCH 07/11] xen/arm: traps: Export a bunch of helpers to handle emulation

[Bhupinder Thakur]

Hi Julien,

On 11 August 2017 at 23:32, Julien Grall  wrote:
> A follow-up patch will move some parts of traps.c in separate files.
> The will require to use helpers that are currently statically defined.
> Export the following helpers:
> - inject_undef64_exception
> - inject_undef_exception
> - check_conditional_instr
> - advance_pc
> - handle_raz_wi
> - handle_wo_wi
> - handle_ro_raz
>
> Note that asm-arm/arm32/traps.h is empty but it is to keep parity with
> the arm64 counterpart.
>
> Signed-off-by: Julien Grall 
>
> ---
>
> Cc: volodymyr_babc...@epam.com
> ---
>  xen/arch/arm/traps.c  | 43 
> +++
>  xen/include/asm-arm/arm32/traps.h | 13 
>  xen/include/asm-arm/arm64/traps.h | 15 ++
>  xen/include/asm-arm/traps.h   | 35 +++
>  4 files changed, 84 insertions(+), 22 deletions(-)
>  create mode 100644 xen/include/asm-arm/arm32/traps.h
>  create mode 100644 xen/include/asm-arm/arm64/traps.h
>  create mode 100644 xen/include/asm-arm/traps.h
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index d79e9605b5..ab56958717 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -49,6 +49,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> @@ -545,7 +546,7 @@ static vaddr_t exception_handler64(struct cpu_user_regs 
> *regs, vaddr_t offset)
>  }
>
>  /* Inject an undefined exception into a 64 bit guest */
> -static void inject_undef64_exception(struct cpu_user_regs *regs, int 
> instr_len)
> +void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len)
>  {
>  vaddr_t handler;
>  const union hsr esr = {
> @@ -618,8 +619,7 @@ static void inject_iabt64_exception(struct cpu_user_regs 
> *regs,
>
>  #endif
>
> -static void inject_undef_exception(struct cpu_user_regs *regs,
> -   const union hsr hsr)
> +void inject_undef_exception(struct cpu_user_regs *regs, const union hsr hsr)
>  {
>  if ( is_32bit_domain(current->domain) )
>  inject_undef32_exception(regs);
> @@ -1712,8 +1712,7 @@ static const unsigned short cc_map[16] = {
>  0   /* NV */
>  };
>
> -static int check_conditional_instr(struct cpu_user_regs *regs,
> -   const union hsr hsr)
> +int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr)
>  {
>  unsigned long cpsr, cpsr_cond;
>  int cond;
> @@ -1758,7 +1757,7 @@ static int check_conditional_instr(struct cpu_user_regs 
> *regs,
>  return 1;
>  }
>
> -static void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
> +void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
>  {
>  unsigned long itbits, cond, cpsr = regs->cpsr;
>
> @@ -1799,11 +1798,11 @@ static void advance_pc(struct cpu_user_regs *regs, 
> const union hsr hsr)
>  }
>
>  /* Read as zero and write ignore */
> -static void handle_raz_wi(struct cpu_user_regs *regs,
> -  int regidx,
> -  bool read,
> -  const union hsr hsr,
> -  int min_el)
> +void handle_raz_wi(struct cpu_user_regs *regs,
> +   int regidx,
> +   bool read,
> +   const union hsr hsr,
> +   int min_el)
>  {
>  ASSERT((min_el == 0) || (min_el == 1));
>
> @@ -1817,12 +1816,12 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
>  advance_pc(regs, hsr);
>  }
>
> -/* Write only as write ignore */
> -static void handle_wo_wi(struct cpu_user_regs *regs,
> - int regidx,
> - bool read,
> - const union hsr hsr,
> - int min_el)
> +/* write only as write ignore */
> +void handle_wo_wi(struct cpu_user_regs *regs,
> +  int regidx,
> +  bool read,
> +  const union hsr hsr,
> +  int min_el)
>  {
>  ASSERT((min_el == 0) || (min_el == 1));
>
> @@ -1837,11 +1836,11 @@ static void handle_wo_wi(struct cpu_user_regs *regs,
>  }
>
>  /* Read only as read as zero */
> -static void handle_ro_raz(struct cpu_user_regs *regs,
> -  int regidx,
> -  bool read,
> -  const union hsr hsr,
> -  int min_el)
> +void handle_ro_raz(struct cpu_user_regs *regs,
> +   int regidx,
> +   bool read,
> +   const union hsr hsr,
> +   int min_el)
>  {
>  ASSERT((min_el == 0) || (min_el == 1));
>
> diff --git a/xen/include/asm-arm/arm32/traps.h 
> b/xen/include/asm-arm/arm32/traps.h
> new file mode 100644
> index 00..e3c4a8b473
> --- /dev/null
> +++ b/xen/include/asm-arm/arm32/traps.h
> @@ -0,0 +1,13 @@
> +#ifndef __ASM_ARM32_TRAPS__
> +

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 111538

Regressions which are regarded as allowable (not blocking):
 build-arm64   2 hosts-allocate broken REGR. vs. 111538
 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 111538
 build-arm64-xsm   2 hosts-allocate broken REGR. vs. 111538
 test-armhf-armhf-xl-rtds  6 xen-install  fail REGR. vs. 111538

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64   3 capture-logs  broken blocked in 111538
 build-arm64-xsm   3 capture-logs  broken blocked in 111538
 build-arm64-pvops 3 capture-logs  broken blocked in 111538
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 111538
 build-amd64-prev  7 xen-build/dist-test  fail   never pass
 build-i386-prev   7 xen-build/dist-test  fail   never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-amd64-xl-pvh-intel 15 guest-saverestorefail  never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  12 guest-start  fail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 xen  f5211ce75821e0f2cc55effd28dfbe908226970f
baseline version:
 xen  1e6c88fafcb4664a501239d1d8665c34b5384648

Last test of basis   111538  2017-07-07 18:57:38 Z   39 days
Testing same since   112649  2017-08-15 13:44:29 Z0 days1 attempts

Re: [Xen-devel] [PATCH v3 1/2] gnttab: don't use possibly unbounded tail calls

[Jan Beulich]

>>> On 15.08.17 at 17:04,  wrote:
> On 15/08/17 14:49, Jan Beulich wrote:
>> @@ -2608,7 +2610,7 @@ static long gnttab_copy(
>>  {
>>  if ( i && hypercall_preempt_check() )
>>  {
>> -rc = i;
>> +rc = count - i;
> 
> Somewhere, probably as a comment for gnttab_copy(), we should have a
> comment explaining why the return value is different from all other ops,
> which will also go somewhat to explaining the "rc = count - rc;" logic.

Sure.

> I think it would also be wise to have an early exit in
> do_grant_table_op() for passing a count of 0.  As far as I can tell, we
> will get all the way into the subop handler before discovering a count of 0.

Well, that would collide with {get,set}_version which don't currently
honor count (and hence existing callers may be passing zero here).
Otherwise I would agree with what you propose.

Jan


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

Re: [Xen-devel] [XenSummit 2017] Shared coprocessor framework followup

[Andrii Anisov]

Hello Edgar,

I'm just wondering if you have had a chance to play with SCF?
Please do not hesitate to come up with questions and comments. We are 
extremely interested in them to make the thing better.


--

*Andrii Anisov*



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

Re: [Xen-devel] [PATCH v8 08/13] arm/guest_access: Move vgic_access_guest_memory to guest_access.h

[Sergej Proskurin]

Hi Julien,


On 08/09/2017 10:20 AM, Sergej Proskurin wrote:
> This commit moves the function vgic_access_guest_memory to guestcopy.c
> and the header asm/guest_access.h. No functional changes are made.
> Please note that the function will be renamed in the following commit.
>
> Signed-off-by: Sergej Proskurin 
> Acked-by: Julien Grall 
> ---
> Cc: Stefano Stabellini 
> Cc: Julien Grall 
> ---
> v6: We added this patch to our patch series.
>
> v7: Add Acked-by Julien Grall.

[...]

> diff --git a/xen/include/asm-arm/guest_access.h 
> b/xen/include/asm-arm/guest_access.h
> index 251e935597..49716501a4 100644
> --- a/xen/include/asm-arm/guest_access.h
> +++ b/xen/include/asm-arm/guest_access.h
> @@ -10,6 +10,9 @@ unsigned long raw_copy_to_guest_flush_dcache(void *to, 
> const void *from,
>  unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len);
>  unsigned long raw_clear_guest(void *to, unsigned len);
>  
> +int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
> + uint32_t size, bool_t is_write);
> +
>  #define __raw_copy_to_guest raw_copy_to_guest
>  #define __raw_copy_from_guest raw_copy_from_guest
>  #define __raw_clear_guest raw_clear_guest
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index d4ed23df28..e489d0bf21 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -217,9 +217,6 @@ extern void register_vgic_ops(struct domain *d, const 
> struct vgic_ops *ops);
>  int vgic_v2_init(struct domain *d, int *mmio_count);
>  int vgic_v3_init(struct domain *d, int *mmio_count);
>  
> -int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
> - uint32_t size, bool_t is_write);
> -
>  extern int domain_vgic_register(struct domain *d, int *mmio_count);
>  extern int vcpu_vgic_free(struct vcpu *v);
>  extern bool vgic_to_sgi(struct vcpu *v, register_t sgir,

As Stefano and Andrew mentioned in patch 11/13, due to a recent patch in
staging, the upper patch failes building due to a missing declaration of
struct domain in . This can be easily fixed by adding a
forward declaration to struct domain right above
vgic_access_guest_memory in  as you will find in the
following patch.

Although this change already fixed the build on my machine, according to
Travis CI one build (XEN_TARGET_ARCH=arm64
CROSS_COMPILE=aarch64-linux-gnu- XEN_CONFIG_EXPERT=y RANDCONFIG=y
debug=n) failed due to missing information of the types paddr_t,
uint32_t, bool_t etc. Which is the reason why I have included
.

The header  already includes . However,
as  gets included from gcov.h before 
it leads to the upper missing type information issues.

Would the following changes be ok with you or shall I remove your
Acked-by in v9?



diff --git a/xen/include/asm-arm/guest_access.h
b/xen/include/asm-arm/guest_access.h
index 251e935597..8038e885f4 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -3,6 +3,7 @@

 #include 
 #include 
+#include 

 unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len);
 unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
@@ -10,6 +11,10 @@ unsigned long raw_copy_to_guest_flush_dcache(void
*to, const void *from,
 unsigned long raw_copy_from_guest(void *to, const void *from, unsigned
len);
 unsigned long raw_clear_guest(void *to, unsigned len);

+struct domain;
+int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
+ uint32_t size, bool_t is_write);
+
 #define __raw_copy_to_guest raw_copy_to_guest
 #define __raw_copy_from_guest raw_copy_from_guest
 #define __raw_clear_guest raw_clear_guest
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index d4ed23df28..e489d0bf21 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -217,9 +217,6 @@ extern void register_vgic_ops(struct domain *d,
const struct vgic_ops *ops);
 int vgic_v2_init(struct domain *d, int *mmio_count);
 int vgic_v3_init(struct domain *d, int *mmio_count);

-int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
- uint32_t size, bool_t is_write);
-
 extern int domain_vgic_register(struct domain *d, int *mmio_count);
 extern int vcpu_vgic_free(struct vcpu *v);
 extern bool vgic_to_sgi(struct vcpu *v, register_t sgir,
--
2.13.3



Thanks,
~Sergej


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

Re: [Xen-devel] [PATCH v3 1/2] gnttab: don't use possibly unbounded tail calls

[Andrew Cooper]

On 16/08/17 10:52, Jan Beulich wrote:
 On 15.08.17 at 17:04,  wrote:
>> On 15/08/17 14:49, Jan Beulich wrote:
>>> @@ -2608,7 +2610,7 @@ static long gnttab_copy(
>>>  {
>>>  if ( i && hypercall_preempt_check() )
>>>  {
>>> -rc = i;
>>> +rc = count - i;
>> Somewhere, probably as a comment for gnttab_copy(), we should have a
>> comment explaining why the return value is different from all other ops,
>> which will also go somewhat to explaining the "rc = count - rc;" logic.
> Sure.
>
>> I think it would also be wise to have an early exit in
>> do_grant_table_op() for passing a count of 0.  As far as I can tell, we
>> will get all the way into the subop handler before discovering a count of 0.
> Well, that would collide with {get,set}_version which don't currently
> honor count (and hence existing callers may be passing zero here).
> Otherwise I would agree with what you propose.

Lovely :(

We've also got a number of other issues, like the fact that count, being
unsigned int, gets silently truncated in the non-compat case.

~Andrew

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

Re: [Xen-devel] [PATCH v8 08/13] arm/guest_access: Move vgic_access_guest_memory to guest_access.h

[Julien Grall]

On 16/08/17 10:58, Sergej Proskurin wrote:

Hi Julien,


On 08/09/2017 10:20 AM, Sergej Proskurin wrote:

This commit moves the function vgic_access_guest_memory to guestcopy.c
and the header asm/guest_access.h. No functional changes are made.
Please note that the function will be renamed in the following commit.

Signed-off-by: Sergej Proskurin 
Acked-by: Julien Grall 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
---
v6: We added this patch to our patch series.

v7: Add Acked-by Julien Grall.


[...]


diff --git a/xen/include/asm-arm/guest_access.h 
b/xen/include/asm-arm/guest_access.h
index 251e935597..49716501a4 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -10,6 +10,9 @@ unsigned long raw_copy_to_guest_flush_dcache(void *to, const 
void *from,
 unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len);
 unsigned long raw_clear_guest(void *to, unsigned len);

+int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
+ uint32_t size, bool_t is_write);
+
 #define __raw_copy_to_guest raw_copy_to_guest
 #define __raw_copy_from_guest raw_copy_from_guest
 #define __raw_clear_guest raw_clear_guest
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index d4ed23df28..e489d0bf21 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -217,9 +217,6 @@ extern void register_vgic_ops(struct domain *d, const 
struct vgic_ops *ops);
 int vgic_v2_init(struct domain *d, int *mmio_count);
 int vgic_v3_init(struct domain *d, int *mmio_count);

-int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
- uint32_t size, bool_t is_write);
-
 extern int domain_vgic_register(struct domain *d, int *mmio_count);
 extern int vcpu_vgic_free(struct vcpu *v);
 extern bool vgic_to_sgi(struct vcpu *v, register_t sgir,


As Stefano and Andrew mentioned in patch 11/13, due to a recent patch in
staging, the upper patch failes building due to a missing declaration of
struct domain in . This can be easily fixed by adding a
forward declaration to struct domain right above
vgic_access_guest_memory in  as you will find in the
following patch.


Why the forward declaration and not directly including xen/sched.h?

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 1/2] gnttab: don't use possibly unbounded tail calls

[Jan Beulich]

>>> On 16.08.17 at 12:01,  wrote:
> On 16/08/17 10:52, Jan Beulich wrote:
> On 15.08.17 at 17:04,  wrote:
>>> On 15/08/17 14:49, Jan Beulich wrote:
 @@ -2608,7 +2610,7 @@ static long gnttab_copy(
  {
  if ( i && hypercall_preempt_check() )
  {
 -rc = i;
 +rc = count - i;
>>> Somewhere, probably as a comment for gnttab_copy(), we should have a
>>> comment explaining why the return value is different from all other ops,
>>> which will also go somewhat to explaining the "rc = count - rc;" logic.
>> Sure.
>>
>>> I think it would also be wise to have an early exit in
>>> do_grant_table_op() for passing a count of 0.  As far as I can tell, we
>>> will get all the way into the subop handler before discovering a count of 0.
>> Well, that would collide with {get,set}_version which don't currently
>> honor count (and hence existing callers may be passing zero here).
>> Otherwise I would agree with what you propose.
> 
> Lovely :(
> 
> We've also got a number of other issues, like the fact that count, being
> unsigned int, gets silently truncated in the non-compat case.

Truncated? I don't see any such case (nor why this would be
non-compat specific). There is a check right at the start of the
function to make sure huge values can't be mistaken as error
values returned by the helper functions. You're not referring
to the fact that a caller might be passing an unsigned long
count, are you? That would be a problem with any unsigned
int hypercall parameters (e.g. also with "cmd" here), but I
don't view this as a problem at all: These are defined to take
32-bit parameters only. For example, Linux'es 
HYPERVISOR_grant_table_op() also properly has both as
unsigned int.

Jan


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

[Xen-devel] [PATCH v4 0/2] XSA-226

[Jan Beulich]

XSA-226 went out with just a workaround patch. The pair of patches
here became ready too late to be reasonably included in the XSA.
Nevertheless they aim at fixing the underlying issues, ideally making
the workaround unnecessary.

1: gnttab: don't use possibly unbounded tail calls
2: gnttab: fix transitive grant handling

Signed-off-by: Jan Beulich 


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

[Xen-devel] [PATCH v4 1/2] gnttab: don't use possibly unbounded tail calls

[Jan Beulich]

There is no guarantee that the compiler would actually translate them
to branches instead of calls, so only ones with a known recursion limit
are okay:
- __release_grant_for_copy() can call itself only once, as
  __acquire_grant_for_copy() won't permit use of multi-level transitive
  grants,
- __acquire_grant_for_copy() is fine to call itself with the last
  argument false, as that prevents further recursion,
- __acquire_grant_for_copy() must not call itself to recover from an
  observed change to the active entry's pin count

This is part of XSA-226.

Signed-off-by: Jan Beulich 
---
v4: Add comment.
v2: Zap *page prior to returning ERESTART. Fix i == 0 case in the exit
path being added to gnttab_copy().

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2109,8 +2109,10 @@ __release_grant_for_copy(
 
 if ( td != rd )
 {
-/* Recursive calls, but they're tail calls, so it's
-   okay. */
+/*
+ * Recursive calls, but they're bounded (acquire permits only a single
+ * level of transitivity), so it's okay.
+ */
 if ( released_write )
 __release_grant_for_copy(td, trans_gref, 0);
 else if ( released_read )
@@ -2262,10 +2264,11 @@ __acquire_grant_for_copy(
 return rc;
 }
 
-/* We dropped the lock, so we have to check that nobody
-   else tried to pin (or, for that matter, unpin) the
-   reference in *this* domain.  If they did, just give up
-   and try again. */
+/*
+ * We dropped the lock, so we have to check that nobody else tried
+ * to pin (or, for that matter, unpin) the reference in *this*
+ * domain.  If they did, just give up and tell the caller to retry.
+ */
 if ( act->pin != old_pin )
 {
 __fixup_status_for_copy_pin(act, status);
@@ -2273,9 +2276,8 @@ __acquire_grant_for_copy(
 active_entry_release(act);
 grant_read_unlock(rgt);
 put_page(*page);
-return __acquire_grant_for_copy(rd, gref, ldom, readonly,
-frame, page, page_off, length,
-allow_transitive);
+*page = NULL;
+return ERESTART;
 }
 
 /* The actual remote remote grant may or may not be a
@@ -2574,7 +2576,7 @@ static int gnttab_copy_one(const struct
 {
 gnttab_copy_release_buf(src);
 rc = gnttab_copy_claim_buf(op, &op->source, src, GNTCOPY_source_gref);
-if ( rc < 0 )
+if ( rc )
 goto out;
 }
 
@@ -2584,7 +2586,7 @@ static int gnttab_copy_one(const struct
 {
 gnttab_copy_release_buf(dest);
 rc = gnttab_copy_claim_buf(op, &op->dest, dest, GNTCOPY_dest_gref);
-if ( rc < 0 )
+if ( rc )
 goto out;
 }
 
@@ -2593,6 +2595,14 @@ static int gnttab_copy_one(const struct
 return rc;
 }
 
+/*
+ * gnttab_copy(), other than the various other helpers of
+ * do_grant_table_op(), returns (besides possible error indicators)
+ * "count - i" rather than "i" to ensure that even if no progress
+ * was made at all (perhaps due to gnttab_copy_one() returning a
+ * positive value) a non-zero value is being handed back (zero needs
+ * to be avoided, as that means "success, all done").
+ */
 static long gnttab_copy(
 XEN_GUEST_HANDLE_PARAM(gnttab_copy_t) uop, unsigned int count)
 {
@@ -2606,7 +2616,7 @@ static long gnttab_copy(
 {
 if ( i && hypercall_preempt_check() )
 {
-rc = i;
+rc = count - i;
 break;
 }
 
@@ -2616,13 +2626,20 @@ static long gnttab_copy(
 break;
 }
 
-op.status = gnttab_copy_one(&op, &dest, &src);
-if ( op.status != GNTST_okay )
+rc = gnttab_copy_one(&op, &dest, &src);
+if ( rc > 0 )
+{
+rc = count - i;
+break;
+}
+if ( rc != GNTST_okay )
 {
 gnttab_copy_release_buf(&src);
 gnttab_copy_release_buf(&dest);
 }
 
+op.status = rc;
+rc = 0;
 if ( unlikely(__copy_field_to_guest(uop, &op, status)) )
 {
 rc = -EFAULT;
@@ -3162,6 +3179,7 @@ do_grant_table_op(
 rc = gnttab_copy(copy, count);
 if ( rc > 0 )
 {
+rc = count - rc;
 guest_handle_add_offset(copy, rc);
 uop = guest_handle_cast(copy, void);
 }



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

[Xen-devel] [PATCH v4 2/2] gnttab: fix transitive grant handling

[Jan Beulich]

Processing of transitive grants must not use the fast path, or else
reference counting breaks due to the skipped recursive call to
__acquire_grant_for_copy() (its __release_grant_for_copy()
counterpart occurs independent of original pin count). Furthermore
after re-acquiring temporarily dropped locks we need to verify no grant
properties changed if the original pin count was non-zero; checking
just the pin counts is sufficient only for well-behaved guests. As a
result, __release_grant_for_copy() needs to mirror that new behavior.

Furthermore a __release_grant_for_copy() invocation was missing on the
retry path of __acquire_grant_for_copy(), and gnttab_set_version() also
needs to bail out upon encountering a transitive grant.

This is part of XSA-226.

Reported-by: Andrew Cooper 
Signed-off-by: Jan Beulich 
Reviewed-by: Andrew Cooper 
---
v4: Re-base.
v3: gnttab_set_version() also needs fixing.
v2: Also adjust __release_grant_for_copy(), as pointed out by Andrew.
Call __release_grant_for_copy() on __acquire_grant_for_copy() error
path. Also re-validate grant table version. Use _set_status_v2()
directly.
---
I was tempted to replace the open coded barrier(), but then thought
it's better to not do unrelated cleanup (other than formatting of the
code being moved). There's also "rrd" mentioned in a comment, which I
would have corrected if only I knew what is being meant - the code
subsequent to the comment deals with td ...

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2056,13 +2056,8 @@ __release_grant_for_copy(
 unsigned long r_frame;
 uint16_t *status;
 grant_ref_t trans_gref;
-int released_read;
-int released_write;
 struct domain *td;
 
-released_read = 0;
-released_write = 0;
-
 grant_read_lock(rgt);
 
 act = active_entry_acquire(rgt, gref);
@@ -2092,17 +2087,11 @@ __release_grant_for_copy(
 
 act->pin -= GNTPIN_hstw_inc;
 if ( !(act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask)) )
-{
-released_write = 1;
 gnttab_clear_flag(_GTF_writing, status);
-}
 }
 
 if ( !act->pin )
-{
 gnttab_clear_flag(_GTF_reading, status);
-released_read = 1;
-}
 
 active_entry_release(act);
 grant_read_unlock(rgt);
@@ -2110,13 +2099,10 @@ __release_grant_for_copy(
 if ( td != rd )
 {
 /*
- * Recursive calls, but they're bounded (acquire permits only a single
+ * Recursive call, but it is bounded (acquire permits only a single
  * level of transitivity), so it's okay.
  */
-if ( released_write )
-__release_grant_for_copy(td, trans_gref, 0);
-else if ( released_read )
-__release_grant_for_copy(td, trans_gref, 1);
+__release_grant_for_copy(td, trans_gref, readonly);
 
 rcu_unlock_domain(td);
 }
@@ -2190,8 +2176,108 @@ __acquire_grant_for_copy(
  act->domid, ldom, act->pin);
 
 old_pin = act->pin;
-if ( !act->pin ||
- (!readonly && !(act->pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask))) )
+if ( sha2 && (shah->flags & GTF_type_mask) == GTF_transitive )
+{
+if ( (!old_pin || (!readonly &&
+   !(old_pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask 
&&
+ (rc = _set_status_v2(ldom, readonly, 0, shah, act,
+  status)) != GNTST_okay )
+ goto unlock_out;
+
+if ( !allow_transitive )
+PIN_FAIL(unlock_out_clear, GNTST_general_error,
+ "transitive grant when transitivity not allowed\n");
+
+trans_domid = sha2->transitive.trans_domid;
+trans_gref = sha2->transitive.gref;
+barrier(); /* Stop the compiler from re-loading
+  trans_domid from shared memory */
+if ( trans_domid == rd->domain_id )
+PIN_FAIL(unlock_out_clear, GNTST_general_error,
+ "transitive grants cannot be self-referential\n");
+
+/*
+ * We allow the trans_domid == ldom case, which corresponds to a
+ * grant being issued by one domain, sent to another one, and then
+ * transitively granted back to the original domain.  Allowing it
+ * is easy, and means that you don't need to go out of your way to
+ * avoid it in the guest.
+ */
+
+/* We need to leave the rrd locked during the grant copy. */
+td = rcu_lock_domain_by_id(trans_domid);
+if ( td == NULL )
+PIN_FAIL(unlock_out_clear, GNTST_general_error,
+ "transitive grant referenced bad domain %d\n",
+ trans_domid);
+
+/*
+ * __acquire_grant_for_copy() could take the lock on the
+ * remote table (if rd == td), so we have to drop the lock
+ * here and reacquire.
+ */
+active_entry_release(act);
+grant_read_unlock(rgt);
+
+rc = __acqui

[Xen-devel] [PATCH v2 0/7] gnttab: recent XSA follow-up

[Jan Beulich]

1: avoid spurious maptrack handle allocation failures
2: type adjustments
3: drop pointless leading double underscores
4: re-arrange struct active_grant_entry
5: drop struct active_grant_entry's gfn field for release builds
6: clean up main switch in do_grant_table_op()
7: properly handle transfer slots when changing version

In case it matters this series applies on top of the v4 XSA-226 pair
of patches sent a couple of minutes ago.

Signed-off-by: Jan Beulich 



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

Re: [Xen-devel] [xen-unstable test] 112618: regressions - trouble: blocked/broken/fail/pass

[Julien Grall]

Hi Wei,

On 15/08/17 16:08, Wei Liu wrote:

On Mon, Aug 14, 2017 at 06:58:13PM +0100, Julien Grall wrote:

Hi,

On 14/08/17 00:20, osstest service owner wrote:

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-pvops 6 kernel-build   fail in 112608 REGR. vs. 112544


Something is a bit odd here. Why 112608 would affect this test?



See

https://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;h=e14a8160aac355dda2fa7b8636fb7162b70235b7;hb=refs/heads/master


Thank you for the link! The e-mail makes a lot more sense now :).

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v4 1/2] gnttab: don't use possibly unbounded tail calls

[Andrew Cooper]

On 16/08/17 11:20, Jan Beulich wrote:
> There is no guarantee that the compiler would actually translate them
> to branches instead of calls, so only ones with a known recursion limit
> are okay:
> - __release_grant_for_copy() can call itself only once, as
>   __acquire_grant_for_copy() won't permit use of multi-level transitive
>   grants,
> - __acquire_grant_for_copy() is fine to call itself with the last
>   argument false, as that prevents further recursion,
> - __acquire_grant_for_copy() must not call itself to recover from an
>   observed change to the active entry's pin count
>
> This is part of XSA-226.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

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

[Xen-devel] [PATCH v2 1/7] gnttab: avoid spurious maptrack handle allocation failures

[Jan Beulich]

When no memory is available in the hypervisor, rather than immediately
failing the request, try to steal a handle from another vCPU.

Reported-by: George Dunlap 
Signed-off-by: Jan Beulich 
---
v2: Adjust comment.

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -430,7 +430,7 @@ get_maptrack_handle(
 struct vcpu  *curr = current;
 unsigned int  i, head;
 grant_handle_thandle;
-struct grant_mapping *new_mt;
+struct grant_mapping *new_mt = NULL;
 
 handle = __get_maptrack_handle(lgt, curr);
 if ( likely(handle != -1) )
@@ -439,10 +439,15 @@ get_maptrack_handle(
 spin_lock(&lgt->maptrack_lock);
 
 /*
- * If we've run out of frames, try stealing an entry from another
- * VCPU (in case the guest isn't mapping across its VCPUs evenly).
+ * If we've run out of handles and still have frame headroom, try
+ * allocating a new maptrack frame.  If there is no headroom, or we're
+ * out of memory, try stealing an entry from another VCPU (in case the
+ * guest isn't mapping across its VCPUs evenly).
  */
-if ( nr_maptrack_frames(lgt) >= max_maptrack_frames )
+if ( nr_maptrack_frames(lgt) < max_maptrack_frames )
+new_mt = alloc_xenheap_page();
+
+if ( !new_mt )
 {
 spin_unlock(&lgt->maptrack_lock);
 
@@ -465,12 +470,6 @@ get_maptrack_handle(
 return steal_maptrack_handle(lgt, curr);
 }
 
-new_mt = alloc_xenheap_page();
-if ( !new_mt )
-{
-spin_unlock(&lgt->maptrack_lock);
-return -1;
-}
 clear_page(new_mt);
 
 /*




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

[Xen-devel] [PATCH v2 2/7] gnttab: type adjustments

[Jan Beulich]

In particular use grant_ref_t and grant_handle_t where appropriate.
Also switch other nearby type uses to their canonical variants where
appropriate and introduce INVALID_MAPTRACK_HANDLE.

Signed-by: Jan Beulich 
Reviewed-by: Andrew Cooper 
---
v2: Re-base.

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -96,7 +96,7 @@ struct gnttab_unmap_common {
 int16_t status;
 
 /* Shared state beteen *_unmap and *_unmap_complete */
-u16 done;
+uint16_t done;
 unsigned long frame;
 struct domain *rd;
 grant_ref_t ref;
@@ -118,11 +118,11 @@ struct gnttab_unmap_common {
  * table of these, indexes into which are returned as a 'mapping handle'.
  */
 struct grant_mapping {
-u32  ref;   /* grant ref */
-u16  flags; /* 0-4: GNTMAP_* ; 5-15: unused */
+grant_ref_t ref;/* grant ref */
+uint16_t flags; /* 0-4: GNTMAP_* ; 5-15: unused */
 domid_t  domid; /* granting domain */
-u32  vcpu;  /* vcpu which created the grant mapping */
-u32  pad;   /* round size to a power of 2 */
+uint32_t vcpu;  /* vcpu which created the grant mapping */
+uint32_t pad;   /* round size to a power of 2 */
 };
 
 #define MAPTRACK_PER_PAGE (PAGE_SIZE / sizeof(struct grant_mapping))
@@ -178,7 +178,7 @@ struct active_grant_entry {
 
 domid_t   domid;  /* Domain being granted access. */
 struct domain *trans_domain;
-uint32_t  trans_gref;
+grant_ref_t   trans_gref;
 unsigned long frame;  /* Frame being granted. */
 unsigned long gfn;/* Guest's idea of the frame being granted. */
 unsigned  is_sub_page:1; /* True if this is a sub-page grant. */
@@ -316,7 +316,9 @@ double_gt_unlock(struct grant_table *lgt
 grant_write_unlock(rgt);
 }
 
-static inline int
+#define INVALID_MAPTRACK_HANDLE UINT_MAX
+
+static inline grant_handle_t
 __get_maptrack_handle(
 struct grant_table *t,
 struct vcpu *v)
@@ -331,7 +333,7 @@ __get_maptrack_handle(
 if ( unlikely(head == MAPTRACK_TAIL) )
 {
 spin_unlock(&v->maptrack_freelist_lock);
-return -1;
+return INVALID_MAPTRACK_HANDLE;
 }
 
 /*
@@ -342,7 +344,7 @@ __get_maptrack_handle(
 if ( unlikely(next == MAPTRACK_TAIL) )
 {
 spin_unlock(&v->maptrack_freelist_lock);
-return -1;
+return INVALID_MAPTRACK_HANDLE;
 }
 
 prev_head = head;
@@ -364,8 +366,8 @@ __get_maptrack_handle(
  * each VCPU and to avoid two VCPU repeatedly stealing entries from
  * each other, the initial victim VCPU is selected randomly.
  */
-static int steal_maptrack_handle(struct grant_table *t,
- const struct vcpu *curr)
+static grant_handle_t steal_maptrack_handle(struct grant_table *t,
+const struct vcpu *curr)
 {
 const struct domain *currd = curr->domain;
 unsigned int first, i;
@@ -376,10 +378,10 @@ static int steal_maptrack_handle(struct
 do {
 if ( currd->vcpu[i] )
 {
-int handle;
+grant_handle_t handle;
 
 handle = __get_maptrack_handle(t, currd->vcpu[i]);
-if ( handle != -1 )
+if ( handle != INVALID_MAPTRACK_HANDLE )
 {
 maptrack_entry(t, handle).vcpu = curr->vcpu_id;
 return handle;
@@ -392,12 +394,12 @@ static int steal_maptrack_handle(struct
 } while ( i != first );
 
 /* No free handles on any VCPU. */
-return -1;
+return INVALID_MAPTRACK_HANDLE;
 }
 
 static inline void
 put_maptrack_handle(
-struct grant_table *t, int handle)
+struct grant_table *t, grant_handle_t handle)
 {
 struct domain *currd = current->domain;
 struct vcpu *v;
@@ -423,7 +425,7 @@ put_maptrack_handle(
 spin_unlock(&v->maptrack_freelist_lock);
 }
 
-static inline int
+static inline grant_handle_t
 get_maptrack_handle(
 struct grant_table *lgt)
 {
@@ -433,7 +435,7 @@ get_maptrack_handle(
 struct grant_mapping *new_mt = NULL;
 
 handle = __get_maptrack_handle(lgt, curr);
-if ( likely(handle != -1) )
+if ( likely(handle != INVALID_MAPTRACK_HANDLE) )
 return handle;
 
 spin_lock(&lgt->maptrack_lock);
@@ -458,8 +460,8 @@ get_maptrack_handle(
 if ( curr->maptrack_tail == MAPTRACK_TAIL )
 {
 handle = steal_maptrack_handle(lgt, curr);
-if ( handle == -1 )
-return -1;
+if ( handle == INVALID_MAPTRACK_HANDLE )
+return handle;
 spin_lock(&curr->maptrack_freelist_lock);
 maptrack_entry(lgt, handle).ref = MAPTRACK_TAIL;
 curr->maptrack_tail = handle;
@@ -480,6 +482,7 @@ get_maptrack_handle(
 
 for ( i = 0; i < MAPTRACK_PER_PAGE; i++ )
 {
+BUILD_BUG_ON(sizeof(new_mt->ref) < sizeof(handle));

[Xen-devel] [PATCH v2 3/7] gnttab: drop pointless leading double underscores

[Jan Beulich]

They're violating name space rules, and we don't really need them. When
followed by "gnttab_", also drop that.

Signed-by: Jan Beulich 
Reviewed-by: Andrew Cooper 
---
v2: Re-base. Minor formatting adjustment.

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -252,8 +252,9 @@ static inline void active_entry_release(
If rc == GNTST_okay, *page contains the page struct with a ref taken.
Caller must do put_page(*page).
If any error, *page = NULL, *frame = INVALID_MFN, no ref taken. */
-static int __get_paged_frame(unsigned long gfn, unsigned long *frame, struct 
page_info **page,
-int readonly, struct domain *rd)
+static int get_paged_frame(unsigned long gfn, unsigned long *frame,
+   struct page_info **page, bool readonly,
+   struct domain *rd)
 {
 int rc = GNTST_okay;
 #if defined(P2M_PAGED_TYPES) || defined(P2M_SHARED_TYPES)
@@ -319,9 +320,7 @@ double_gt_unlock(struct grant_table *lgt
 #define INVALID_MAPTRACK_HANDLE UINT_MAX
 
 static inline grant_handle_t
-__get_maptrack_handle(
-struct grant_table *t,
-struct vcpu *v)
+_get_maptrack_handle(struct grant_table *t, struct vcpu *v)
 {
 unsigned int head, next, prev_head;
 
@@ -380,7 +379,7 @@ static grant_handle_t steal_maptrack_han
 {
 grant_handle_t handle;
 
-handle = __get_maptrack_handle(t, currd->vcpu[i]);
+handle = _get_maptrack_handle(t, currd->vcpu[i]);
 if ( handle != INVALID_MAPTRACK_HANDLE )
 {
 maptrack_entry(t, handle).vcpu = curr->vcpu_id;
@@ -434,7 +433,7 @@ get_maptrack_handle(
 grant_handle_thandle;
 struct grant_mapping *new_mt = NULL;
 
-handle = __get_maptrack_handle(lgt, curr);
+handle = _get_maptrack_handle(lgt, curr);
 if ( likely(handle != INVALID_MAPTRACK_HANDLE) )
 return handle;
 
@@ -789,7 +788,7 @@ static unsigned int mapkind(
  * update, as indicated by the GNTMAP_contains_pte flag.
  */
 static void
-__gnttab_map_grant_ref(
+map_grant_ref(
 struct gnttab_map_grant_ref *op)
 {
 struct domain *ld, *rd, *owner = NULL;
@@ -888,8 +887,8 @@ __gnttab_map_grant_ref(
 shared_entry_v1(rgt, op->ref).frame :
 shared_entry_v2(rgt, op->ref).full_page.frame;
 
-rc = __get_paged_frame(gfn, &frame, &pg,
-!!(op->flags & GNTMAP_readonly), rd);
+rc = get_paged_frame(gfn, &frame, &pg,
+ op->flags & GNTMAP_readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
 act->gfn = gfn;
@@ -919,7 +918,7 @@ __gnttab_map_grant_ref(
 active_entry_release(act);
 grant_read_unlock(rgt);
 
-/* pg may be set, with a refcount included, from __get_paged_frame */
+/* pg may be set, with a refcount included, from get_paged_frame(). */
 if ( !pg )
 {
 pg = mfn_valid(_mfn(frame)) ? mfn_to_page(frame) : NULL;
@@ -1130,7 +1129,7 @@ gnttab_map_grant_ref(
 if ( unlikely(__copy_from_guest_offset(&op, uop, i, 1)) )
 return -EFAULT;
 
-__gnttab_map_grant_ref(&op);
+map_grant_ref(&op);
 
 if ( unlikely(__copy_to_guest_offset(uop, i, &op, 1)) )
 return -EFAULT;
@@ -1140,7 +1139,7 @@ gnttab_map_grant_ref(
 }
 
 static void
-__gnttab_unmap_common(
+unmap_common(
 struct gnttab_unmap_common *op)
 {
 domid_t  dom;
@@ -1200,8 +1199,8 @@ __gnttab_unmap_common(
 /*
  * This ought to be impossible, as such a mapping should not have
  * been established (see the nr_grant_entries(rgt) bounds check in
- * __gnttab_map_grant_ref()). Doing this check only in
- * __gnttab_unmap_common_complete() - as it used to be done - would,
+ * gnttab_map_grant_ref()). Doing this check only in
+ * gnttab_unmap_common_complete() - as it used to be done - would,
  * however, be too late.
  */
 rc = GNTST_bad_gntref;
@@ -1315,7 +1314,7 @@ __gnttab_unmap_common(
 }
 
 static void
-__gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
+unmap_common_complete(struct gnttab_unmap_common *op)
 {
 struct domain *ld, *rd = op->rd;
 struct grant_table *rgt;
@@ -1326,7 +1325,7 @@ __gnttab_unmap_common_complete(struct gn
 
 if ( !op->done )
 {
-/* __gntab_unmap_common() didn't do anything - nothing to complete. */
+/* unmap_common() didn't do anything - nothing to complete. */
 return;
 }
 
@@ -1395,7 +1394,7 @@ __gnttab_unmap_common_complete(struct gn
 }
 
 static void
-__gnttab_unmap_grant_ref(
+unmap_grant_ref(
 struct gnttab_unmap_grant_ref *op,
 struct gnttab_unmap_common *common)
 {
@@ -1409,7 +1408,7 @@ __gnttab_unmap_grant_ref(
 common->rd = NULL;
 common->frame = 0;
 
-__gnttab_unmap_common(common);
+unma

[Xen-devel] [PATCH v2 5/7] gnttab: drop struct active_grant_entry's gfn field for release builds

[Jan Beulich]

This shrinks the size from 48 to 40 bytes bytes on 64-bit builds.
Switch to gfn_t at once.

Signed-off-by: Jan Beulich 
---
v2: Re-base. Use inline function instead of macro. Switch to gfn_t.

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -185,7 +185,9 @@ struct active_grant_entry {
 grant_ref_t   trans_gref;
 struct domain *trans_domain;
 unsigned long frame;  /* Frame being granted. */
-unsigned long gfn;/* Guest's idea of the frame being granted. */
+#ifndef NDEBUG
+gfn_t gfn;/* Guest's idea of the frame being granted. */
+#endif
 spinlock_tlock;  /* lock to protect access of this entry.
 see docs/misc/grant-tables.txt for
 locking protocol  */
@@ -195,6 +197,13 @@ struct active_grant_entry {
 #define _active_entry(t, e) \
 ((t)->active[(e)/ACGNT_PER_PAGE][(e)%ACGNT_PER_PAGE])
 
+static inline void act_set_gfn(struct active_grant_entry *act, gfn_t gfn)
+{
+#ifndef NDEBUG
+act->gfn = gfn;
+#endif
+}
+
 DEFINE_PERCPU_RWLOCK_GLOBAL(grant_rwlock);
 
 static inline void gnttab_flush_tlb(const struct domain *d)
@@ -891,7 +900,7 @@ map_grant_ref(
  op->flags & GNTMAP_readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
-act->gfn = gfn;
+act_set_gfn(act, _gfn(gfn));
 act->domid = ld->domain_id;
 act->frame = frame;
 act->start = 0;
@@ -2287,7 +2296,7 @@ acquire_grant_for_copy(
 act->trans_domain = td;
 act->trans_gref = trans_gref;
 act->frame = grant_frame;
-act->gfn = gfn_x(INVALID_GFN);
+act_set_gfn(act, INVALID_GFN);
 /*
  * The actual remote remote grant may or may not be a sub-page,
  * but we always treat it as one because that blocks mappings of
@@ -2313,7 +2322,7 @@ acquire_grant_for_copy(
 rc = get_paged_frame(gfn, &grant_frame, page, readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
-act->gfn = gfn;
+act_set_gfn(act, _gfn(gfn));
 is_sub_page = false;
 trans_page_off = 0;
 trans_length = PAGE_SIZE;
@@ -2324,7 +2333,7 @@ acquire_grant_for_copy(
  readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
-act->gfn = sha2->full_page.frame;
+act_set_gfn(act, _gfn(sha2->full_page.frame));
 is_sub_page = false;
 trans_page_off = 0;
 trans_length = PAGE_SIZE;
@@ -2335,7 +2344,7 @@ acquire_grant_for_copy(
  readonly, rd);
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
-act->gfn = sha2->sub_page.frame;
+act_set_gfn(act, _gfn(sha2->sub_page.frame));
 is_sub_page = true;
 trans_page_off = sha2->sub_page.page_off;
 trans_length = sha2->sub_page.length;
@@ -3500,8 +3509,16 @@ void grant_table_warn_active_grants(stru
 
 nr_active++;
 if ( nr_active <= WARN_GRANT_MAX )
-printk(XENLOG_G_DEBUG "Dom%d has an active grant: GFN: %lx (MFN: 
%lx)\n",
-   d->domain_id, act->gfn, act->frame);
+printk(XENLOG_G_DEBUG "Dom%d has active grant %x ("
+#ifndef NDEBUG
+   "GFN %lx, "
+#endif
+   "MFN: %lx)\n",
+   d->domain_id, ref,
+#ifndef NDEBUG
+   gfn_x(act->gfn),
+#endif
+   act->frame);
 active_entry_release(act);
 }
 




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

[Xen-devel] [PATCH v2 4/7] gnttab: re-arrange struct active_grant_entry

[Jan Beulich]

While benign to 32-bit arches, this shrinks the size from 56 to 48
bytes on 64-bit ones (while still leaving a 16-bit hole).

Take the opportunity and consistently use bool/true/false for all
is_sub_page uses.

Signed-off-by: Jan Beulich 
---
v2: Re-base and extend to clean up is_sub_page uses at once.

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -177,15 +177,15 @@ struct active_grant_entry {
 #define GNTPIN_devr_mask (0xFFU << GNTPIN_devr_shift)
 
 domid_t   domid;  /* Domain being granted access. */
-struct domain *trans_domain;
+unsigned int  start:15; /* For sub-page grants, the start offset
+   in the page.   */
+bool  is_sub_page:1; /* True if this is a sub-page grant. */
+unsigned int  length:16; /* For sub-page grants, the length of the
+grant.*/
 grant_ref_t   trans_gref;
+struct domain *trans_domain;
 unsigned long frame;  /* Frame being granted. */
 unsigned long gfn;/* Guest's idea of the frame being granted. */
-unsigned  is_sub_page:1; /* True if this is a sub-page grant. */
-unsigned  start:15; /* For sub-page grants, the start offset
-   in the page.   */
-unsigned  length:16; /* For sub-page grants, the length of the
-grant.*/
 spinlock_tlock;  /* lock to protect access of this entry.
 see docs/misc/grant-tables.txt for
 locking protocol  */
@@ -896,7 +896,7 @@ map_grant_ref(
 act->frame = frame;
 act->start = 0;
 act->length = PAGE_SIZE;
-act->is_sub_page = 0;
+act->is_sub_page = false;
 act->trans_domain = rd;
 act->trans_gref = op->ref;
 }
@@ -2164,7 +2164,7 @@ acquire_grant_for_copy(
 unsigned long grant_frame;
 uint16_t trans_page_off;
 uint16_t trans_length;
-int is_sub_page;
+bool is_sub_page;
 s16 rc = GNTST_okay;
 
 *page = NULL;
@@ -2293,7 +2293,7 @@ acquire_grant_for_copy(
  * but we always treat it as one because that blocks mappings of
  * transitive grants.
  */
-act->is_sub_page = 1;
+act->is_sub_page = true;
 }
 }
 else if ( !old_pin ||
@@ -2314,7 +2314,7 @@ acquire_grant_for_copy(
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
 act->gfn = gfn;
-is_sub_page = 0;
+is_sub_page = false;
 trans_page_off = 0;
 trans_length = PAGE_SIZE;
 }
@@ -2325,7 +2325,7 @@ acquire_grant_for_copy(
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
 act->gfn = sha2->full_page.frame;
-is_sub_page = 0;
+is_sub_page = false;
 trans_page_off = 0;
 trans_length = PAGE_SIZE;
 }
@@ -2336,7 +2336,7 @@ acquire_grant_for_copy(
 if ( rc != GNTST_okay )
 goto unlock_out_clear;
 act->gfn = sha2->sub_page.frame;
-is_sub_page = 1;
+is_sub_page = true;
 trans_page_off = sha2->sub_page.page_off;
 trans_length = sha2->sub_page.length;
 }




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

[Xen-devel] [PATCH v2 7/7] gnttab: properly handle transfer slots when changing version

[Jan Beulich]

Reported-by: Andrew Cooper 
Signed-off-by: Jan Beulich 
---
v2: New.
---
Or should we put ourselves on the position that reserved entries aren#t
meant to be used for transfers?

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2755,6 +2755,11 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
 {
 switch ( shared_entry_v2(gt, i).hdr.flags & GTF_type_mask )
 {
+case GTF_accept_transfer:
+ if ( !(shared_entry_v2(gt, i).hdr.flags &
+GTF_transfer_completed) )
+ break;
+ /* fall through */
 case GTF_permit_access:
  if ( !(shared_entry_v2(gt, i).full_page.frame >> 32) )
  break;
@@ -2784,13 +2789,17 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
 switch ( flags & GTF_type_mask )
 {
 case GTF_permit_access:
-reserved_entries[i].flags = flags | status_entry(gt, i);
+flags |= status_entry(gt, i);
+/* fall through */
+case GTF_accept_transfer:
+reserved_entries[i].flags = flags;
 reserved_entries[i].domid = shared_entry_v2(gt, i).hdr.domid;
 reserved_entries[i].frame = shared_entry_v2(gt, 
i).full_page.frame;
 break;
+
 default:
 gdprintk(XENLOG_INFO,
- "bad flags %#x in grant %#x when switching version\n",
+ "unstable flags %#x in grant %#x when switching 
version\n",
  flags, i);
 /* fall through */
 case GTF_invalid:
@@ -2819,10 +2828,20 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
 case 2:
 for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
 {
-status_entry(gt, i) =
-reserved_entries[i].flags & (GTF_reading | GTF_writing);
-shared_entry_v2(gt, i).hdr.flags =
-reserved_entries[i].flags & ~(GTF_reading | GTF_writing);
+if ( (reserved_entries[i].flags & GTF_type_mask) !=
+ GTF_accept_transfer )
+{
+status_entry(gt, i) =
+reserved_entries[i].flags & (GTF_reading | 
GTF_writing);
+shared_entry_v2(gt, i).hdr.flags =
+reserved_entries[i].flags & ~(GTF_reading | 
GTF_writing);
+}
+else
+{
+status_entry(gt, i) = 0;
+shared_entry_v2(gt, i).hdr.flags =
+reserved_entries[i].flags;
+}
 shared_entry_v2(gt, i).hdr.domid =
 reserved_entries[i].domid;
 shared_entry_v2(gt, i).full_page.frame =




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

[Xen-devel] [PATCH v2 6/7] gnttab: clean up main switch in do_grant_table_op()

[Jan Beulich]

Add blank lines as necessary and drop unnecessary braces.

Signed-off-by: Jan Beulich 
---
v2: New.

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3153,6 +3153,7 @@ do_grant_table_op(
 {
 XEN_GUEST_HANDLE_PARAM(gnttab_map_grant_ref_t) map =
 guest_handle_cast(uop, gnttab_map_grant_ref_t);
+
 if ( unlikely(!guest_handle_okay(map, count)) )
 goto out;
 rc = gnttab_map_grant_ref(map, count);
@@ -3163,10 +3164,12 @@ do_grant_table_op(
 }
 break;
 }
+
 case GNTTABOP_unmap_grant_ref:
 {
 XEN_GUEST_HANDLE_PARAM(gnttab_unmap_grant_ref_t) unmap =
 guest_handle_cast(uop, gnttab_unmap_grant_ref_t);
+
 if ( unlikely(!guest_handle_okay(unmap, count)) )
 goto out;
 rc = gnttab_unmap_grant_ref(unmap, count);
@@ -3177,10 +3180,12 @@ do_grant_table_op(
 }
 break;
 }
+
 case GNTTABOP_unmap_and_replace:
 {
 XEN_GUEST_HANDLE_PARAM(gnttab_unmap_and_replace_t) unmap =
 guest_handle_cast(uop, gnttab_unmap_and_replace_t);
+
 if ( unlikely(!guest_handle_okay(unmap, count)) )
 goto out;
 rc = -ENOSYS;
@@ -3194,17 +3199,18 @@ do_grant_table_op(
 }
 break;
 }
+
 case GNTTABOP_setup_table:
-{
 rc = gnttab_setup_table(
 guest_handle_cast(uop, gnttab_setup_table_t), count);
 ASSERT(rc <= 0);
 break;
-}
+
 case GNTTABOP_transfer:
 {
 XEN_GUEST_HANDLE_PARAM(gnttab_transfer_t) transfer =
 guest_handle_cast(uop, gnttab_transfer_t);
+
 if ( unlikely(!guest_handle_okay(transfer, count)) )
 goto out;
 rc = gnttab_transfer(transfer, count);
@@ -3215,10 +3221,12 @@ do_grant_table_op(
 }
 break;
 }
+
 case GNTTABOP_copy:
 {
 XEN_GUEST_HANDLE_PARAM(gnttab_copy_t) copy =
 guest_handle_cast(uop, gnttab_copy_t);
+
 if ( unlikely(!guest_handle_okay(copy, count)) )
 goto out;
 rc = gnttab_copy(copy, count);
@@ -3230,33 +3238,31 @@ do_grant_table_op(
 }
 break;
 }
+
 case GNTTABOP_query_size:
-{
 rc = gnttab_query_size(
 guest_handle_cast(uop, gnttab_query_size_t), count);
 ASSERT(rc <= 0);
 break;
-}
+
 case GNTTABOP_set_version:
-{
 rc = gnttab_set_version(guest_handle_cast(uop, gnttab_set_version_t));
 break;
-}
+
 case GNTTABOP_get_status_frames:
-{
 rc = gnttab_get_status_frames(
 guest_handle_cast(uop, gnttab_get_status_frames_t), count);
 break;
-}
+
 case GNTTABOP_get_version:
-{
 rc = gnttab_get_version(guest_handle_cast(uop, gnttab_get_version_t));
 break;
-}
+
 case GNTTABOP_swap_grant_ref:
 {
 XEN_GUEST_HANDLE_PARAM(gnttab_swap_grant_ref_t) swap =
 guest_handle_cast(uop, gnttab_swap_grant_ref_t);
+
 if ( unlikely(!guest_handle_okay(swap, count)) )
 goto out;
 rc = gnttab_swap_grant_ref(swap, count);
@@ -3267,6 +3273,7 @@ do_grant_table_op(
 }
 break;
 }
+
 case GNTTABOP_cache_flush:
 {
 XEN_GUEST_HANDLE_PARAM(gnttab_cache_flush_t) cflush =
@@ -3283,6 +3290,7 @@ do_grant_table_op(
 opaque_out = opaque_in;
 break;
 }
+
 default:
 rc = -ENOSYS;
 break;




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

Re: [Xen-devel] [PATCH 03/11] xen/arm: traps: Re-order the includes alphabetically

[Julien Grall]

Hi Bhupinder,

On 16/08/17 07:44, Bhupinder Thakur wrote:

Hi Julien,

On 11 August 2017 at 23:32, Julien Grall  wrote:

Signed-off-by: Julien Grall 
---
 xen/arch/arm/traps.c | 42 ++
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c07999b518..ca9bef712c 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -16,41 +16,43 @@
  * GNU General Public License for more details.
  */

-#include 
-#include 
-#include 
-#include 
-#include 
+#include 
+#include 
+#include 
 #include 
+#include 
+#include 

The above two files should be before xen/irq.h.


Got my alphabet wrong :/. Thank you for spotting it! I will fix it in 
the next version.





 #include 
 #include 
+#include 
 #include 
-#include 
-#include 
-#include 
-#include 
 #include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
-#include 
-#include 
+
 #include 
 #include 
-#include 
-#include 
-#include 
+
+#include 
 #include 
-#include 
-#include 
+#include 
 #include 
+#include 
+#include 
 #include 
+#include 
+#include 
 #include 
+#include 

The above two files should be before asm/psci.h with asm/mmio.h included first.


Same here :/




+#include 
+#include 

 #include "decode.h"
 #include "vtimer.h"
-#include 
-#include 
-#include 
-#include 

 /* The base of the stack must always be double-word aligned, which means
  * that both the kernel half of struct cpu_user_regs (which is pushed in
--
2.11.0



With these two changes.

Acked-by: Bhupinder Thakur 


Thank you for the review.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 07/11] xen/arm: traps: Export a bunch of helpers to handle emulation

[Julien Grall]

On 16/08/17 10:30, Bhupinder Thakur wrote:

Hi Julien,


Hi Bhupinder,


On 11 August 2017 at 23:32, Julien Grall  wrote:

diff --git a/xen/include/asm-arm/arm64/traps.h 
b/xen/include/asm-arm/arm64/traps.h
new file mode 100644
index 00..5cb45df6bd
--- /dev/null
+++ b/xen/include/asm-arm/arm64/traps.h
@@ -0,0 +1,15 @@
+#ifndef __ASM_ARM32_TRAPS__
+#define __ASM_ARM32_TRAPS__


I think it should be __ASM_ARM64_TRAPS__ here.


Hmmm yes and ...




+
+void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len);
+
+#endif /* __ASM_ARM32_TRAPS__ */


here too. I will fix it in the next version.


+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
+
diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
new file mode 100644
index 00..4e227c4dd2
--- /dev/null
+++ b/xen/include/asm-arm/traps.h
@@ -0,0 +1,35 @@
+#ifndef __ASM_ARM_TRAPS__
+#define __ASM_ARM_TRAPS__
+
+#include 
+
+#if defined(CONFIG_ARM_32)
+# include 
+#elif defined(CONFIG_ARM_64)
+# include 
+#endif
+
+int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr);

a newline can be added here.


I will do that.




+void advance_pc(struct cpu_user_regs *regs, const union hsr hsr);
+
+void inject_undef_exception(struct cpu_user_regs *regs, const union hsr hsr);
+
+void handle_raz_wi(struct cpu_user_regs *regs, int regidx, bool read,
+   const union hsr hsr, int min_el);
+
+void handle_wo_wi(struct cpu_user_regs *regs, int regidx, bool read,
+  const union hsr hsr, int min_el);
+
+void handle_ro_raz(struct cpu_user_regs *regs, int regidx, bool read,
+   const union hsr hsr, int min_el);
+
+#endif /* __ASM_ARM_TRAPS__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
+
--
2.11.0



Cheers,

--
Julien Grall

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

[Xen-devel] [xen-4.9-testing baseline-only test] 71979: regressions - trouble: blocked/broken/fail/pass

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 71979 xen-4.9-testing real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/71979/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 15 guest-saverestore.2 
fail REGR. vs. 71943
 test-amd64-i386-xl-qemut-win10-i386 10 windows-installfail REGR. vs. 71943
 test-amd64-amd64-amd64-pvgrub 17 guest-localmigrate/x10   fail REGR. vs. 71943
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
71943
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-install   fail REGR. vs. 71943

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64   2 hosts-allocate   broken never pass
 build-arm64-pvops 2 hosts-allocate   broken never pass
 build-arm64-xsm   2 hosts-allocate   broken never pass
 build-arm64   3 capture-logs broken never pass
 build-arm64-pvops 3 capture-logs broken never pass
 build-arm64-xsm   3 capture-logs broken never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 10 windows-installfail never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-midway   13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-installfail never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 13 guest-saverestore   fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass

version targeted for testing:
 xen  0e186e33c0487a81c48dccdede206e63db22dd7d
baseline version:
 xen  f4f02f121f271ee0722723e393226687b42e29a1

Last test of basis71943  2017-08-05 19:45:27 Z   10 days
Testing same since71979  2017-08-16 03:50:15 Z0 days1 attempts


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

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  broken  
 build-armhf-xsm   

Re: [Xen-devel] [RFC PATCH v2 14/22] ARM: vGIC: move virtual IRQ configuration from rank to pending_irq

[Julien Grall]

Hi Andre,

On 21/07/17 21:00, Andre Przywara wrote:

The IRQ configuration (level or edge triggered) for a group of IRQs
are still stored in the irq_rank structure.
Introduce a new bit called GIC_IRQ_GUEST_LEVEL in the "status" field,
which holds that information.
Remove the storage from the irq_rank and use the existing wrappers to
store and retrieve the configuration bit for multiple IRQs.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/vgic-v2.c | 21 +++-
 xen/arch/arm/vgic-v3.c | 25 --
 xen/arch/arm/vgic.c| 81 +-
 xen/include/asm-arm/vgic.h |  5 ++-
 4 files changed, 73 insertions(+), 59 deletions(-)

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index a3fd500..0c8a598 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -278,20 +278,12 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, 
mmio_info_t *info,
 goto read_reserved;

 case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
-{
-uint32_t icfgr;
-
 if ( dabt.size != DABT_WORD ) goto bad_width;
-rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
-if ( rank == NULL) goto read_as_zero;
-vgic_lock_rank(v, rank, flags);
-icfgr = rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR, 
DABT_WORD)];
-vgic_unlock_rank(v, rank, flags);

-*r = vreg_reg32_extract(icfgr, info);
+irq = (gicd_reg - GICD_ICFGR) * 4;
+*r = vgic_fetch_irq_config(v, irq);

 return 1;
-}

 case VRANGE32(0xD00, 0xDFC):
 goto read_impl_defined;
@@ -529,13 +521,8 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info,

 case VRANGE32(GICD_ICFGR2, GICD_ICFGRN): /* SPIs */
 if ( dabt.size != DABT_WORD ) goto bad_width;
-rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
-if ( rank == NULL) goto write_ignore;
-vgic_lock_rank(v, rank, flags);
-vreg_reg32_update(&rank->icfg[REG_RANK_INDEX(2, gicd_reg - GICD_ICFGR,
- DABT_WORD)],
-  r, info);
-vgic_unlock_rank(v, rank, flags);
+irq = (gicd_reg - GICD_ICFGR) * 4; /* 2 bit per IRQ */


s/bit/bits/


+vgic_store_irq_config(v, irq, r);
 return 1;

 case VRANGE32(0xD00, 0xDFC):
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index d3356ae..e9e36eb 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -722,20 +722,11 @@ static int __vgic_v3_distr_common_mmio_read(const char 
*name, struct vcpu *v,
 return 1;

 case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
-{
-uint32_t icfgr;
-
 if ( dabt.size != DABT_WORD ) goto bad_width;
-rank = vgic_rank_offset(v, 2, reg - GICD_ICFGR, DABT_WORD);
-if ( rank == NULL ) goto read_as_zero;
-vgic_lock_rank(v, rank, flags);
-icfgr = rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR, DABT_WORD)];
-vgic_unlock_rank(v, rank, flags);
-
-*r = vreg_reg32_extract(icfgr, info);
-
+irq = (reg - GICD_ICFGR) * 4;
+if ( irq >= v->domain->arch.vgic.nr_spis + 32 ) goto read_as_zero;
+*r = vgic_fetch_irq_config(v, irq);
 return 1;
-}

 default:
 printk(XENLOG_G_ERR
@@ -834,13 +825,9 @@ static int __vgic_v3_distr_common_mmio_write(const char 
*name, struct vcpu *v,
 /* ICFGR1 for PPI's, which is implementation defined
if ICFGR1 is programmable or not. We chose to program */
 if ( dabt.size != DABT_WORD ) goto bad_width;
-rank = vgic_rank_offset(v, 2, reg - GICD_ICFGR, DABT_WORD);
-if ( rank == NULL ) goto write_ignore;
-vgic_lock_rank(v, rank, flags);
-vreg_reg32_update(&rank->icfg[REG_RANK_INDEX(2, reg - GICD_ICFGR,
- DABT_WORD)],
-  r, info);
-vgic_unlock_rank(v, rank, flags);
+irq = (reg - GICD_ICFGR) * 4;
+if ( irq >= v->domain->arch.vgic.nr_spis + 32 ) goto write_ignore;
+vgic_store_irq_config(v, irq, r);
 return 1;

 default:
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index ddcd99b..e5a4765 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -268,6 +268,55 @@ void vgic_store_irq_priority(struct vcpu *v, unsigned int 
nrirqs,
 local_irq_restore(flags);
 }

+#define IRQS_PER_CFGR   16
+/**
+ * vgic_fetch_irq_config: assemble the configuration bits for a group of 16 
IRQs
+ * @v: the VCPU for private IRQs, any VCPU of a domain for SPIs
+ * @first_irq: the first IRQ to be queried, must be aligned to 16
+ */
+uint32_t vgic_fetch_irq_config(struct vcpu *v, unsigned int first_irq)
+{
+struct pending_irq *pirqs[IRQS_PER_CFGR];
+unsigned long flags;
+uint32_t ret = 0, i;
+
+local_irq_save(flags);
+vgic_lock_irqs(v, IRQS_PER_CFGR, first_irq, pirqs);
+
+fo

Re: [Xen-devel] [PATCH] x86: Skip check apic_id_limit for Xen

[Paolo Bonzini]

On 16/08/2017 02:22, Lan Tianyu wrote:
> Xen vIOMMU device model will be in Xen hypervisor. Skip vIOMMU
> check for Xen here when vcpu number is more than 255.

I think you still need to do a check for vIOMMU being enabled.

Paolo

> Signed-off-by: Lan Tianyu 
> ---
>  hw/i386/pc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 5943539..fc17885 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1260,7 +1260,7 @@ void pc_machine_done(Notifier *notifier, void *data)
>  fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
>  }
>  
> -if (pcms->apic_id_limit > 255) {
> +if (pcms->apic_id_limit > 255 && !xen_enabled()) {
>  IntelIOMMUState *iommu = INTEL_IOMMU_DEVICE(x86_iommu_get_default());
>  
>  if (!iommu || !iommu->x86_iommu.intr_supported ||
> 


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

[Xen-devel] [PATCH v2 2/4] xen/x86: Drop unnecessary barriers

[Andrew Cooper]

x86's current implementation of wmb() is a compiler barrier.  As a result, the
only change in this patch is to remove an mfence instruction from
cpuidle_disable_deep_cstate().

None of these barriers serve any purpose.  Most aren't aren't synchronising
with any remote cpus, where as the mcetelem barriers are redundant with
spin_unlock(), which already has full read/write barrier semantics.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

v2:
 * s/erronious/unnecessary/
 * Drop more unnecessary barriers
---
 xen/arch/x86/acpi/cpu_idle.c | 2 --
 xen/arch/x86/cpu/mcheck/mce.c| 3 ---
 xen/arch/x86/cpu/mcheck/mctelem.c| 3 ---
 xen/arch/x86/crash.c | 3 ---
 xen/arch/x86/mm/shadow/multi.c   | 1 -
 xen/arch/x86/smpboot.c   | 2 --
 xen/drivers/passthrough/amd/iommu_init.c | 2 --
 7 files changed, 16 deletions(-)

diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index 482b8a7..5879ad6 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -1331,8 +1331,6 @@ void cpuidle_disable_deep_cstate(void)
 max_cstate = 1;
 }
 
-mb();
-
 hpet_disable_legacy_broadcast();
 }
 
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 30525dd..aa6e556 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -86,7 +86,6 @@ static x86_mce_vector_t _machine_check_vector = 
unexpected_machine_check;
 void x86_mce_vector_register(x86_mce_vector_t hdlr)
 {
 _machine_check_vector = hdlr;
-wmb();
 }
 
 /* Call the installed machine check handler for this CPU setup. */
@@ -369,8 +368,6 @@ mcheck_mca_logout(enum mca_source who, struct mca_banks 
*bankmask,
 mcabank_clear(i);
 else if ( who == MCA_MCE_SCAN && need_clear)
 mcabanks_set(i, clear_bank);
-
-wmb();
 }
 
 if (mig && errcnt > 0) {
diff --git a/xen/arch/x86/cpu/mcheck/mctelem.c 
b/xen/arch/x86/cpu/mcheck/mctelem.c
index b144a66..1731514 100644
--- a/xen/arch/x86/cpu/mcheck/mctelem.c
+++ b/xen/arch/x86/cpu/mcheck/mctelem.c
@@ -520,7 +520,6 @@ mctelem_cookie_t 
mctelem_consume_oldest_begin(mctelem_class_t which)
}
 
mctelem_processing_hold(tep);
-   wmb();
spin_unlock(&processing_lock);
return MCTE2COOKIE(tep);
 }
@@ -531,7 +530,6 @@ void mctelem_consume_oldest_end(mctelem_cookie_t cookie)
 
spin_lock(&processing_lock);
mctelem_processing_release(tep);
-   wmb();
spin_unlock(&processing_lock);
 }
 
@@ -547,6 +545,5 @@ void mctelem_ack(mctelem_class_t which, mctelem_cookie_t 
cookie)
spin_lock(&processing_lock);
if (tep == mctctl.mctc_processing_head[target])
mctelem_processing_release(tep);
-   wmb();
spin_unlock(&processing_lock);
 }
diff --git a/xen/arch/x86/crash.c b/xen/arch/x86/crash.c
index 82535c4..8d74258 100644
--- a/xen/arch/x86/crash.c
+++ b/xen/arch/x86/crash.c
@@ -146,9 +146,6 @@ static void nmi_shootdown_cpus(void)
 write_atomic((unsigned long *)__va(__pa(&exception_table[TRAP_nmi])),
  (unsigned long)&do_nmi_crash);
 
-/* Ensure the new callback function is set before sending out the NMI. */
-wmb();
-
 smp_send_nmi_allbutself();
 
 msecs = 1000; /* Wait at most a second for the other cpus to stop */
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index c9c2252..1e3dfaf 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -3112,7 +3112,6 @@ static int sh_page_fault(struct vcpu *v,
  * will make sure no inconsistent mapping being translated into
  * shadow page table. */
 version = atomic_read(&d->arch.paging.shadow.gtable_dirty_version);
-rmb();
 walk_ok = sh_walk_guest_tables(v, va, &gw, error_code);
 
 #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 8d91f6c..5b094b4 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -355,7 +355,6 @@ void start_secondary(void *unused)
 spin_debug_enable();
 set_cpu_sibling_map(cpu);
 notify_cpu_starting(cpu);
-wmb();
 
 /*
  * We need to hold vector_lock so there the set of online cpus
@@ -371,7 +370,6 @@ void start_secondary(void *unused)
 local_irq_enable();
 mtrr_ap_init();
 
-wmb();
 startup_cpu_idle_loop();
 }
 
diff --git a/xen/drivers/passthrough/amd/iommu_init.c 
b/xen/drivers/passthrough/amd/iommu_init.c
index a459e99..d5b6049 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -558,7 +558,6 @@ static void parse_event_log_entry(struct amd_iommu *iommu, 
u32 entry[])
 return;
 }
 udelay(1);
-rmb();
 code = get_field_from_reg_u32(entry[1], IOMMU_EVENT_CODE_MASK,
   IOMMU_EVENT_CODE_SHIFT);
 }
@@ -663,7 

[Xen-devel] [PATCH v2 0/4] x86: Corrections to barrier usage

[Andrew Cooper]

This series brings Xen's barrier ABI in line with Linuxs, so developers stand
a better chance of not getting them wrong.

Andrew Cooper (4):
  x86/mcheck: Minor cleanup to amd_nonfatal
  xen/x86: Drop unnecessary barriers
  xen/x86: Replace remaining mandatory barriers with SMP barriers
  xen/x86: Correct mandatory and SMP barrier definitions

 xen/arch/x86/acpi/cpu_idle.c | 10 --
 xen/arch/x86/cpu/mcheck/amd_nonfatal.c   | 15 +--
 xen/arch/x86/cpu/mcheck/barrier.c| 10 +-
 xen/arch/x86/cpu/mcheck/mce.c|  3 ---
 xen/arch/x86/cpu/mcheck/mctelem.c|  7 ++-
 xen/arch/x86/crash.c |  3 ---
 xen/arch/x86/genapic/x2apic.c|  6 +++---
 xen/arch/x86/hpet.c  |  2 +-
 xen/arch/x86/hvm/ioreq.c |  4 ++--
 xen/arch/x86/irq.c   |  4 ++--
 xen/arch/x86/mm/shadow/multi.c   |  1 -
 xen/arch/x86/smpboot.c   | 14 ++
 xen/arch/x86/time.c  |  8 
 xen/drivers/passthrough/amd/iommu_init.c |  2 --
 xen/include/asm-x86/desc.h   |  8 
 xen/include/asm-x86/system.h | 30 +-
 xen/include/asm-x86/x86_64/system.h  |  3 ---
 17 files changed, 55 insertions(+), 75 deletions(-)

-- 
2.1.4


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

[Xen-devel] [PATCH v2 4/4] xen/x86: Correct mandatory and SMP barrier definitions

[Andrew Cooper]

Barriers are a complicated topic, a source of confusion, and their incorrect
use is a common cause of bugs.  It *really* doesn't help when Xen's API is the
same as Linux, but its ABI different.

Bring the two back in line, so programmers stand a chance of actually getting
their usage correct.

Drop the links in the comment, both of which are now stale.  Instead, refer to
the vendor system manuals.

No functional change.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

v2:
 * Keep mandatory barrier definitions
 * Drop stale documentation links
---
 xen/include/asm-x86/system.h| 28 
 xen/include/asm-x86/x86_64/system.h |  3 ---
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/xen/include/asm-x86/system.h b/xen/include/asm-x86/system.h
index 9cb6fd7..3d21291 100644
--- a/xen/include/asm-x86/system.h
+++ b/xen/include/asm-x86/system.h
@@ -164,23 +164,27 @@ static always_inline unsigned long __xadd(
 ((typeof(*(ptr)))__xadd(ptr, (typeof(*(ptr)))(v), sizeof(*(ptr
 
 /*
+ * Mandatory barriers, for enforced ordering of reads and writes, e.g. for use
+ * with MMIO devices mapped with reduced cacheability.
+ */
+#define mb()asm volatile ("mfence" ::: "memory")
+#define rmb()   asm volatile ("lfence" ::: "memory")
+#define wmb()   asm volatile ("sfence" ::: "memory")
+
+/*
+ * SMP barriers, for ordering of reads and writes between CPUs, most commonly
+ * used with shared memory.
+ *
  * Both Intel and AMD agree that, from a programmer's viewpoint:
  *  Loads cannot be reordered relative to other loads.
  *  Stores cannot be reordered relative to other stores.
- * 
- * Intel64 Architecture Memory Ordering White Paper
- * 
- * 
- * AMD64 Architecture Programmer's Manual, Volume 2: System Programming
- * 
+ *  Loads may be reordered ahead of an unaliasing store.
+ *
+ * Refer to the vendor system programming manuals for further details.
  */
-#define rmb()   barrier()
-#define wmb()   barrier()
-
 #define smp_mb()mb()
-#define smp_rmb()   rmb()
-#define smp_wmb()   wmb()
+#define smp_rmb()   barrier()
+#define smp_wmb()   barrier()
 
 #define set_mb(var, value) do { xchg(&var, value); } while (0)
 #define set_wmb(var, value) do { var = value; smp_wmb(); } while (0)
diff --git a/xen/include/asm-x86/x86_64/system.h 
b/xen/include/asm-x86/x86_64/system.h
index 88beae1..6b56761 100644
--- a/xen/include/asm-x86/x86_64/system.h
+++ b/xen/include/asm-x86/x86_64/system.h
@@ -80,7 +80,4 @@ static always_inline __uint128_t __cmpxchg16b(
 _rc;\
 })
 
-#define mb()\
-asm volatile ( "mfence" : : : "memory" )
-
 #endif /* __X86_64_SYSTEM_H__ */
-- 
2.1.4


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

[Xen-devel] [PATCH v2 3/4] xen/x86: Replace remaining mandatory barriers with SMP barriers

[Andrew Cooper]

There is no functional change.  Xen currently assignes smp_* meaning to
the non-smp_* barriers.

All of these uses are just to deal with shared memory between multiple
processors, so use the smp_*() which are the correct barriers for the purpose.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

v2:
 * Drop more unnecessary barriers, rather than converting them to smp
---
 xen/arch/x86/acpi/cpu_idle.c  |  8 
 xen/arch/x86/cpu/mcheck/barrier.c | 10 +-
 xen/arch/x86/cpu/mcheck/mctelem.c |  4 ++--
 xen/arch/x86/genapic/x2apic.c |  6 +++---
 xen/arch/x86/hpet.c   |  2 +-
 xen/arch/x86/hvm/ioreq.c  |  4 ++--
 xen/arch/x86/irq.c|  4 ++--
 xen/arch/x86/smpboot.c| 12 ++--
 xen/arch/x86/time.c   |  8 
 xen/include/asm-x86/desc.h|  8 
 xen/include/asm-x86/system.h  |  2 +-
 11 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index 5879ad6..dea834c 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -390,9 +390,9 @@ void mwait_idle_with_hints(unsigned int eax, unsigned int 
ecx)
 
 if ( boot_cpu_has(X86_FEATURE_CLFLUSH_MONITOR) )
 {
-mb();
+smp_mb();
 clflush((void *)&mwait_wakeup(cpu));
-mb();
+smp_mb();
 }
 
 __monitor((void *)&mwait_wakeup(cpu), 0, 0);
@@ -755,10 +755,10 @@ void acpi_dead_idle(void)
  * instruction, hence memory fence is necessary to make sure all 
  * load/store visible before flush cache line.
  */
-mb();
+smp_mb();
 clflush(mwait_ptr);
 __monitor(mwait_ptr, 0, 0);
-mb();
+smp_mb();
 __mwait(cx->address, 0);
 }
 }
diff --git a/xen/arch/x86/cpu/mcheck/barrier.c 
b/xen/arch/x86/cpu/mcheck/barrier.c
index 7de8e45..a7e5b19 100644
--- a/xen/arch/x86/cpu/mcheck/barrier.c
+++ b/xen/arch/x86/cpu/mcheck/barrier.c
@@ -12,7 +12,7 @@ void mce_barrier_init(struct mce_softirq_barrier *bar)
 void mce_barrier_dec(struct mce_softirq_barrier *bar)
 {
 atomic_inc(&bar->outgen);
-wmb();
+smp_wmb();
 atomic_dec(&bar->val);
 }
 
@@ -24,12 +24,12 @@ void mce_barrier_enter(struct mce_softirq_barrier *bar, 
bool wait)
 return;
 atomic_inc(&bar->ingen);
 gen = atomic_read(&bar->outgen);
-mb();
+smp_mb();
 atomic_inc(&bar->val);
 while ( atomic_read(&bar->val) != num_online_cpus() &&
 atomic_read(&bar->outgen) == gen )
 {
-mb();
+smp_mb();
 mce_panic_check();
 }
 }
@@ -42,12 +42,12 @@ void mce_barrier_exit(struct mce_softirq_barrier *bar, bool 
wait)
 return;
 atomic_inc(&bar->outgen);
 gen = atomic_read(&bar->ingen);
-mb();
+smp_mb();
 atomic_dec(&bar->val);
 while ( atomic_read(&bar->val) != 0 &&
 atomic_read(&bar->ingen) == gen )
 {
-mb();
+smp_mb();
 mce_panic_check();
 }
 }
diff --git a/xen/arch/x86/cpu/mcheck/mctelem.c 
b/xen/arch/x86/cpu/mcheck/mctelem.c
index 1731514..b071dc8 100644
--- a/xen/arch/x86/cpu/mcheck/mctelem.c
+++ b/xen/arch/x86/cpu/mcheck/mctelem.c
@@ -501,9 +501,9 @@ static void mctelem_append_processing(mctelem_class_t which)
ltep->mcte_prev = *procltp;
*procltp = dangling[target];
}
-   wmb();
+   smp_wmb();
dangling[target] = NULL;
-   wmb();
+   smp_wmb();
 }
 
 mctelem_cookie_t mctelem_consume_oldest_begin(mctelem_class_t which)
diff --git a/xen/arch/x86/genapic/x2apic.c b/xen/arch/x86/genapic/x2apic.c
index 5fffb31..4779b0d 100644
--- a/xen/arch/x86/genapic/x2apic.c
+++ b/xen/arch/x86/genapic/x2apic.c
@@ -106,12 +106,12 @@ static void send_IPI_mask_x2apic_phys(const cpumask_t 
*cpumask, int vector)
  * CPU is seen by notified remote CPUs. The WRMSR contained within
  * apic_icr_write() can otherwise be executed early.
  * 
- * The reason mb() is sufficient here is subtle: the register arguments
+ * The reason smp_mb() is sufficient here is subtle: the register arguments
  * to WRMSR must depend on a memory read executed after the barrier. This
  * is guaranteed by cpu_physical_id(), which reads from a global array (and
  * so cannot be hoisted above the barrier even by a clever compiler).
  */
-mb();
+smp_mb();
 
 local_irq_save(flags);
 
@@ -135,7 +135,7 @@ static void send_IPI_mask_x2apic_cluster(const cpumask_t 
*cpumask, int vector)
 const cpumask_t *cluster_cpus;
 unsigned long flags;
 
-mb(); /* See above for an explanation. */
+smp_mb(); /* See above for an explanation. */
 
 local_irq_save(flags);
 
diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index 8229c63..bc7a851 100644
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -608,7 +608,7 @@ void __ini

[Xen-devel] [PATCH v2 1/4] x86/mcheck: Minor cleanup to amd_nonfatal

[Andrew Cooper]

  * Drop trailing whitespace.
  * Move amd_nonfatal_mcheck_init() into .init.text and drop a trailing return.
  * Drop unnecessary wmb()'s.  Because of Xen's implementation, they are only
compiler barriers anyway, and each wrmsr() is already fully serialising.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

New in v2
---
 xen/arch/x86/cpu/mcheck/amd_nonfatal.c | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/amd_nonfatal.c 
b/xen/arch/x86/cpu/mcheck/amd_nonfatal.c
index c6a9c89..222f539 100644
--- a/xen/arch/x86/cpu/mcheck/amd_nonfatal.c
+++ b/xen/arch/x86/cpu/mcheck/amd_nonfatal.c
@@ -106,7 +106,7 @@ static void mce_amd_checkregs(void *info)
} else {
mctelem_dismiss(mctc);
}
-   
+
} else if (mctc != NULL) {
mctelem_dismiss(mctc);
}
@@ -151,7 +151,7 @@ static void mce_amd_work_fn(void *data)
 
/* HW does not count *all* kinds of correctable errors.
 * Thus it is possible, that the polling routine finds an
-* correctable error even if the HW reports nothing. */ 
+* correctable error even if the HW reports nothing. */
if (counter > 0) {
/* HW reported correctable errors,
 * the polling routine did not find...
@@ -164,8 +164,8 @@ static void mce_amd_work_fn(void *data)
(counter == 1 ? "" : "s"),
(counter == 1 ? "was" : "were"));
}
-   /* subtract 1 to not double count the error 
-* from the polling service routine */ 
+   /* subtract 1 to not double count the error
+* from the polling service routine */
adjust += (counter - 1);
 
/* Restart counter */
@@ -174,7 +174,6 @@ static void mce_amd_work_fn(void *data)
/* Counter enable */
value |= (1ULL << 51);
mca_wrmsr(MSR_IA32_MCx_MISC(4), value);
-   wmb();
}
}
 
@@ -202,7 +201,7 @@ static void mce_amd_work_fn(void *data)
adjust = 0;
 }
 
-void amd_nonfatal_mcheck_init(struct cpuinfo_x86 *c)
+void __init amd_nonfatal_mcheck_init(struct cpuinfo_x86 *c)
 {
if (c->x86_vendor != X86_VENDOR_AMD)
return;
@@ -238,14 +237,10 @@ void amd_nonfatal_mcheck_init(struct cpuinfo_x86 *c)
/* Counter enable */
value |= (1ULL << 51);
wrmsrl(MSR_IA32_MCx_MISC(4), value);
-   /* serialize */
-   wmb();
printk(XENLOG_INFO "MCA: Use hw thresholding to adjust 
polling frequency\n");
}
}
 
init_timer(&mce_timer, mce_amd_work_fn, NULL, 0);
set_timer(&mce_timer, NOW() + period);
-
-   return;
 }
-- 
2.1.4


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

Re: [Xen-devel] [RFC PATCH v2 17/22] ARM: vGIC: introduce vgic_lock_vcpu_irq()

[Julien Grall]

Hi Andre,

On 21/07/17 21:00, Andre Przywara wrote:

Since a VCPU can own multiple IRQs, the natural locking order is to take
a VCPU lock first, then the individual per-IRQ locks.
However there are situations where the target VCPU is not known without
looking into the struct pending_irq first, which usually means we need to
take the IRQ lock first.
To solve this problem, we provide a function called vgic_lock_vcpu_irq(),
which takes a locked struct pending_irq() and returns with *both* the
VCPU and the IRQ lock held.
This is done by looking up the target VCPU, then briefly dropping the
IRQ lock, taking the VCPU lock, then grabbing the per-IRQ lock again.
Before returning there is a check whether something has changed in the
brief period where we didn't hold the IRQ lock, retrying in this (very
rare) case.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/vgic.c | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 1ba0010..0e6dfe5 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -224,6 +224,48 @@ int vcpu_vgic_free(struct vcpu *v)
 return 0;
 }

+/**
+ * vgic_lock_vcpu_irq(): lock both the pending_irq and the corresponding VCPU
+ *
+ * @v: the VCPU (for private IRQs)
+ * @p: pointer to the locked struct pending_irq
+ * @flags: pointer to the IRQ flags used when locking the VCPU
+ *
+ * The function takes a locked IRQ and returns with both the IRQ and the
+ * corresponding VCPU locked. This is non-trivial due to the locking order
+ * being actually the other way round (VCPU first, then IRQ).
+ *
+ * Returns: pointer to the VCPU this IRQ is targeting.
+ */
+struct vcpu *vgic_lock_vcpu_irq(struct vcpu *v, struct pending_irq *p,
+unsigned long *flags)


The prototype for this function is missing.


+{
+struct vcpu *target_vcpu;
+
+ASSERT(spin_is_locked(&p->lock));
+
+target_vcpu = vgic_get_target_vcpu(v, p);
+spin_unlock(&p->lock);
+
+do
+{
+struct vcpu *current_vcpu;
+
+spin_lock_irqsave(&target_vcpu->arch.vgic.lock, *flags);
+spin_lock(&p->lock);
+
+current_vcpu = vgic_get_target_vcpu(v, p);
+
+if ( target_vcpu->vcpu_id == current_vcpu->vcpu_id )
+return target_vcpu;
+
+spin_unlock(&p->lock);
+spin_unlock_irqrestore(&target_vcpu->arch.vgic.lock, *flags);
+
+target_vcpu = current_vcpu;
+} while (1);
+}
+
 struct vcpu *vgic_get_target_vcpu(struct vcpu *v, struct pending_irq *p)
 {
 struct vgic_irq_rank *rank = vgic_rank_irq(v, p->irq);



Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 1/7] gnttab: avoid spurious maptrack handle allocation failures

[Andrew Cooper]

On 16/08/17 11:48, Jan Beulich wrote:
> When no memory is available in the hypervisor, rather than immediately
> failing the request, try to steal a handle from another vCPU.
>
> Reported-by: George Dunlap 
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

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

[Xen-devel] [distros-debian-squeeze test] 71980: tolerable trouble: broken/fail/pass

[Platform Team regression test user]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 build-arm64   2 hosts-allocate   broken like 71954
 build-arm64-pvops 2 hosts-allocate   broken like 71954
 build-arm64-pvops 3 capture-logs broken like 71954
 build-arm64   3 capture-logs broken like 71954
 test-amd64-i386-amd64-squeeze-netboot-pygrub 10 debian-di-install fail like 
71954
 test-amd64-i386-i386-squeeze-netboot-pygrub 10 debian-di-install fail like 
71954
 test-amd64-amd64-i386-squeeze-netboot-pygrub 10 debian-di-install fail like 
71954
 test-amd64-amd64-amd64-squeeze-netboot-pygrub 10 debian-di-install fail like 
71954

baseline version:
 flight   71954

jobs:
 build-amd64  pass
 build-arm64  broken  
 build-armhf  pass
 build-i386   pass
 build-amd64-pvopspass
 build-arm64-pvopsbroken  
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-amd64-squeeze-netboot-pygrubfail
 test-amd64-i386-amd64-squeeze-netboot-pygrub fail
 test-amd64-amd64-i386-squeeze-netboot-pygrub fail
 test-amd64-i386-i386-squeeze-netboot-pygrub  fail



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

Test harness code can be found at
http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary


Push not applicable.


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

Re: [Xen-devel] [PATCH v2 4/7] gnttab: re-arrange struct active_grant_entry

[Andrew Cooper]

On 16/08/17 11:49, Jan Beulich wrote:
> While benign to 32-bit arches, this shrinks the size from 56 to 48
> bytes on 64-bit ones (while still leaving a 16-bit hole).
>
> Take the opportunity and consistently use bool/true/false for all
> is_sub_page uses.
>
> 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 v2 5/7] gnttab: drop struct active_grant_entry's gfn field for release builds

[Andrew Cooper]

On 16/08/17 11:50, Jan Beulich wrote:
> This shrinks the size from 48 to 40 bytes bytes on 64-bit builds.
> Switch to gfn_t at once.
>
> 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 v2 6/7] gnttab: clean up main switch in do_grant_table_op()

[Andrew Cooper]

On 16/08/17 11:50, Jan Beulich wrote:
> Add blank lines as necessary and drop unnecessary braces.
>
> 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 v2 7/7] gnttab: properly handle transfer slots when changing version

[Andrew Cooper]

On 16/08/17 11:51, Jan Beulich wrote:
> Reported-by: Andrew Cooper 
> Signed-off-by: Jan Beulich 
> ---
> v2: New.
> ---
> Or should we put ourselves on the position that reserved entries aren#t
> meant to be used for transfers?

Transfers are only usable by x86 PV (now, following XSA-214), and I
can't see any plausible situation for the toolstack constructing one.

I'd err on the side of fully disallowing them (also because it avoids
adding more conversion code to this function).

Also, it looks like the resulting code would be rather cleaner by
pulling shared_entry_v2(gt, i) out into a variable first.

~Andrew

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

[Xen-devel] [xen-unstable-coverity test] 112663: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 xen  7591ea75f77643342b194031ef5a903564901ba8
baseline version:
 xen  de62402a9c2e403b049aa238b4fa4e2d618e8870

Last test of basis   112617  2017-08-13 09:19:45 Z3 days
Testing same since   112663  2017-08-16 10:48:26 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Boris Ostrovsky 
  Ian Jackson 
  Jan Beulich 
  Juergen Gross 
  Julien Grall 
  Wei Liu 

jobs:
 coverity-amd64   pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

+ branch=xen-unstable-coverity
+ revision=7591ea75f77643342b194031ef5a903564901ba8
+ . ./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-coverity 7591ea75f77643342b194031ef5a903564901ba8
+ branch=xen-unstable-coverity
+ revision=7591ea75f77643342b194031ef5a903564901ba8
+ . ./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-coverity
+ qemuubranch=qemu-upstream-unstable-coverity
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-coverity
+ prevxenbranch=xen-4.9-testing
+ '[' x7591ea75f77643342b194031ef5a903564901ba8 = 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/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.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/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git
++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/xen/git/linux-pvops.git
++ : git://xenbits.xen.org/linux-pvops.git
++ : tested/linux-4.9
++ : tested/linux-arm-xen
++ '[' xgit://xenbits.xen.org/linux-pvops.git = x ']'
++ '[' x = x ']'
++ : git://xenbits.xen.org/linux-pvops.git
++ : tested/linux-arm-xen
++ : git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git
++ : tested/2.6.39.x
++ : daily-cron.xen-unstable-coverity
++ : daily-cron.xen-unstable-cov

[Xen-devel] [PATCH] x86/asm: add .file directives

[Jan Beulich]

Make sure local symbols are correctly associated with their source
files: I've just run across a cpufreq.c#create_bounce_frame stack trace
entry. Since we have multiple entry.S, don't use __FILE__ there to
fully disambiguate things.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/acpi/wakeup_prot.S
+++ b/xen/arch/x86/acpi/wakeup_prot.S
@@ -1,3 +1,4 @@
+.file __FILE__
 .text
 
 #include 
--- a/xen/arch/x86/clear_page.S
+++ b/xen/arch/x86/clear_page.S
@@ -1,3 +1,5 @@
+.file __FILE__
+
 #include 
 
 #define ptr_reg %rdi
--- a/xen/arch/x86/copy_page.S
+++ b/xen/arch/x86/copy_page.S
@@ -1,3 +1,5 @@
+.file __FILE__
+
 #include 
 
 #define src_reg %rsi
--- a/xen/arch/x86/hvm/svm/entry.S
+++ b/xen/arch/x86/hvm/svm/entry.S
@@ -17,6 +17,8 @@
  * this program; If not, see .
  */
 
+.file "svm/entry.S"
+
 #include 
 #include 
 #include 
--- a/xen/arch/x86/hvm/vmx/entry.S
+++ b/xen/arch/x86/hvm/vmx/entry.S
@@ -16,6 +16,8 @@
  * this program; If not, see .
  */
 
+.file "vmx/entry.S"
+
 #include 
 #include 
 #include 
--- a/xen/arch/x86/pv/gpr_switch.S
+++ b/xen/arch/x86/pv/gpr_switch.S
@@ -5,6 +5,8 @@
  * Copyright (c) 2006, Novell, Inc.
  */
 
+.file __FILE__
+
 #include 
 
 ENTRY(host_to_guest_gpr_switch)
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -2,6 +2,8 @@
  * Compatibility hypercall routines.
  */
 
+.file "x86_64/compat/entry.S"
+
 #include 
 #include 
 #include 
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -4,6 +4,8 @@
  * Copyright (c) 2005, K A Fraser
  */
 
+.file "x86_64/entry.S"
+
 #include 
 #include 
 #include 
--- a/xen/arch/x86/x86_64/kexec_reloc.S
+++ b/xen/arch/x86/x86_64/kexec_reloc.S
@@ -10,6 +10,9 @@
  * This source code is licensed under the GNU General Public License,
  * Version 2.  See the file COPYING for more details.
  */
+
+.file __FILE__
+
 #include 
 
 #include 




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

Re: [Xen-devel] [PATCH] x86/asm: add .file directives

[Andrew Cooper]

On 16/08/17 13:13, Jan Beulich wrote:
> Make sure local symbols are correctly associated with their source
> files: I've just run across a cpufreq.c#create_bounce_frame stack trace
> entry. Since we have multiple entry.S, don't use __FILE__ there to
> fully disambiguate things.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

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

[Xen-devel] [PATCH v3] passthrough: give XEN_DOMCTL_test_assign_device more sane semantics

[Jan Beulich]

So far callers of the libxc interface passed in a domain ID which was
then ignored in the hypervisor. Instead, make the hypervisor honor it
(accepting DOMID_INVALID to obtain original behavior), allowing to
query whether a device can be assigned to a particular domain.

Drop XSM's test_assign_{,dt}device hooks as no longer being
individually useful.

Signed-off-by: Jan Beulich 
---
v3: Drop test-assign XSM hooks.
v2: Alter the semantics to check whether the device can be assigned to
the passed in domain.

--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -391,11 +391,15 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
 
 switch ( op->cmd )
 {
-case XEN_DOMCTL_createdomain:
 case XEN_DOMCTL_test_assign_device:
+if ( op->domain == DOMID_INVALID )
+{
+case XEN_DOMCTL_createdomain:
 case XEN_DOMCTL_gdbsx_guestmemio:
-d = NULL;
-break;
+d = NULL;
+break;
+}
+/* fall through */
 default:
 d = rcu_lock_domain_by_id(op->domain);
 if ( !d && op->cmd != XEN_DOMCTL_getdomaininfo )
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -143,12 +143,15 @@ int iommu_do_dt_domctl(struct xen_domctl
 switch ( domctl->cmd )
 {
 case XEN_DOMCTL_assign_device:
+ASSERT(d);
+/* fall through */
+case XEN_DOMCTL_test_assign_device:
 ret = -ENODEV;
 if ( domctl->u.assign_device.dev != XEN_DOMCTL_DEV_DT )
 break;
 
 ret = -EINVAL;
-if ( d->is_dying || domctl->u.assign_device.flags )
+if ( (d && d->is_dying) || domctl->u.assign_device.flags )
 break;
 
 ret = dt_find_node_by_gpath(domctl->u.assign_device.u.dt.path,
@@ -161,6 +164,17 @@ int iommu_do_dt_domctl(struct xen_domctl
 if ( ret )
 break;
 
+if ( domctl->cmd == XEN_DOMCTL_test_assign_device )
+{
+if ( iommu_dt_device_is_assigned(dev) )
+{
+printk(XENLOG_G_ERR "%s already assigned.\n",
+   dt_node_full_name(dev));
+ret = -EINVAL;
+}
+break;
+}
+
 ret = iommu_assign_dt_device(d, dev);
 
 if ( ret )
@@ -194,33 +208,6 @@ int iommu_do_dt_domctl(struct xen_domctl
dt_node_full_name(dev), d->domain_id, ret);
 break;
 
-case XEN_DOMCTL_test_assign_device:
-ret = -ENODEV;
-if ( domctl->u.assign_device.dev != XEN_DOMCTL_DEV_DT )
-break;
-
-ret = -EINVAL;
-if ( domctl->u.assign_device.flags )
-break;
-
-ret = dt_find_node_by_gpath(domctl->u.assign_device.u.dt.path,
-domctl->u.assign_device.u.dt.size,
-&dev);
-if ( ret )
-break;
-
-ret = xsm_test_assign_dtdevice(XSM_HOOK, dt_node_full_name(dev));
-if ( ret )
-break;
-
-if ( iommu_dt_device_is_assigned(dev) )
-{
-printk(XENLOG_G_ERR "%s already assigned.\n",
-   dt_node_full_name(dev));
-ret = -EINVAL;
-}
-break;
-
 default:
 ret = -ENOSYS;
 break;
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1583,35 +1583,10 @@ int iommu_do_pci_domctl(
 }
 break;
 
-case XEN_DOMCTL_test_assign_device:
-ret = -ENODEV;
-if ( domctl->u.assign_device.dev != XEN_DOMCTL_DEV_PCI )
-break;
-
-ret = -EINVAL;
-if ( domctl->u.assign_device.flags )
-break;
-
-machine_sbdf = domctl->u.assign_device.u.pci.machine_sbdf;
-
-ret = xsm_test_assign_device(XSM_HOOK, machine_sbdf);
-if ( ret )
-break;
-
-seg = machine_sbdf >> 16;
-bus = PCI_BUS(machine_sbdf);
-devfn = PCI_DEVFN2(machine_sbdf);
-
-if ( device_assigned(seg, bus, devfn) )
-{
-printk(XENLOG_G_INFO
-   "%04x:%02x:%02x.%u already assigned, or non-existent\n",
-   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
-ret = -EINVAL;
-}
-break;
-
 case XEN_DOMCTL_assign_device:
+ASSERT(d);
+/* fall through */
+case XEN_DOMCTL_test_assign_device:
 /* Don't support self-assignment of devices. */
 if ( d == current->domain )
 {
@@ -1625,7 +1600,9 @@ int iommu_do_pci_domctl(
 
 ret = -EINVAL;
 flags = domctl->u.assign_device.flags;
-if ( d->is_dying || (flags & ~XEN_DOMCTL_DEV_RDM_RELAXED) )
+if ( domctl->cmd == XEN_DOMCTL_assign_device
+ ? d->is_dying || (flags & ~XEN_DOMCTL_DEV_RDM_RELAXED)
+ : flags )
 break;
 
 machine_sbdf = domctl->u.assign_device.u.pci.machine_sbdf;
@@ -1638,8 +1615,20 @@ int iommu_do_pci_domctl(
 bus = PCI_BUS(machine_s

[Xen-devel] [xen-unstable-smoke test] 112665: tolerable trouble: broken/pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 build-arm64-pvops 2 hosts-allocate  broken like 112654
 build-arm64   2 hosts-allocate  broken like 112654
 build-arm64-pvops 3 capture-logsbroken like 112654
 build-arm64   3 capture-logsbroken like 112654
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  4befb4ed85cf5f6784f8c0aaf1d2dba1dbd26ac0
baseline version:
 xen  7591ea75f77643342b194031ef5a903564901ba8

Last test of basis   112654  2017-08-15 17:01:16 Z0 days
Testing same since   112665  2017-08-16 10:48:42 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Yi Sun 

jobs:
 build-amd64  pass
 build-arm64  broken  
 build-armhf  pass
 build-amd64-libvirt  pass
 build-arm64-pvopsbroken  
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  broken  
 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

broken-step build-arm64-pvops hosts-allocate
broken-step build-arm64 hosts-allocate
broken-step build-arm64-pvops capture-logs
broken-step build-arm64 capture-logs

Pushing revision :

+ branch=xen-unstable-smoke
+ revision=4befb4ed85cf5f6784f8c0aaf1d2dba1dbd26ac0
+ . ./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 
4befb4ed85cf5f6784f8c0aaf1d2dba1dbd26ac0
+ branch=xen-unstable-smoke
+ revision=4befb4ed85cf5f6784f8c0aaf1d2dba1dbd26ac0
+ . ./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.9-testing
+ '[' x4befb4ed85cf5f6784f8c0aaf1d2dba1dbd26ac0 = 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/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/x

[Xen-devel] [PATCH] x86/mm: Drop more PV superpage leftovers

[Andrew Cooper]

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
---
 xen/include/asm-x86/x86_64/page.h | 10 --
 1 file changed, 10 deletions(-)

diff --git a/xen/include/asm-x86/x86_64/page.h 
b/xen/include/asm-x86/x86_64/page.h
index 5caf321..c3eab62 100644
--- a/xen/include/asm-x86/x86_64/page.h
+++ b/xen/include/asm-x86/x86_64/page.h
@@ -50,16 +50,6 @@ extern unsigned long xen_virt_end;
 #define pdx_to_virt(pdx) ((void *)(DIRECTMAP_VIRT_START + \
((unsigned long)(pdx) << PAGE_SHIFT)))
 
-static inline unsigned long pfn_to_sdx(unsigned long pfn)
-{
-return pfn_to_pdx(pfn) >> (SUPERPAGE_SHIFT-PAGE_SHIFT);
-}
-
-static inline unsigned long sdx_to_pfn(unsigned long sdx)
-{
-return pdx_to_pfn(sdx << (SUPERPAGE_SHIFT-PAGE_SHIFT));
-}
-
 static inline unsigned long __virt_to_maddr(unsigned long va)
 {
 ASSERT(va >= XEN_VIRT_START && va < DIRECTMAP_VIRT_END);
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v8 08/13] arm/guest_access: Move vgic_access_guest_memory to guest_access.h

[Sergej Proskurin]

On 08/16/2017 12:11 PM, Julien Grall wrote:
>
>
> On 16/08/17 10:58, Sergej Proskurin wrote:
>> Hi Julien,
>>
>>
>> On 08/09/2017 10:20 AM, Sergej Proskurin wrote:
>>> This commit moves the function vgic_access_guest_memory to guestcopy.c
>>> and the header asm/guest_access.h. No functional changes are made.
>>> Please note that the function will be renamed in the following commit.
>>>
>>> Signed-off-by: Sergej Proskurin 
>>> Acked-by: Julien Grall 
>>> ---
>>> Cc: Stefano Stabellini 
>>> Cc: Julien Grall 
>>> ---
>>> v6: We added this patch to our patch series.
>>>
>>> v7: Add Acked-by Julien Grall.
>>
>> [...]
>>
>>> diff --git a/xen/include/asm-arm/guest_access.h
>>> b/xen/include/asm-arm/guest_access.h
>>> index 251e935597..49716501a4 100644
>>> --- a/xen/include/asm-arm/guest_access.h
>>> +++ b/xen/include/asm-arm/guest_access.h
>>> @@ -10,6 +10,9 @@ unsigned long raw_copy_to_guest_flush_dcache(void
>>> *to, const void *from,
>>>  unsigned long raw_copy_from_guest(void *to, const void *from,
>>> unsigned len);
>>>  unsigned long raw_clear_guest(void *to, unsigned len);
>>>
>>> +int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
>>> + uint32_t size, bool_t is_write);
>>> +
>>>  #define __raw_copy_to_guest raw_copy_to_guest
>>>  #define __raw_copy_from_guest raw_copy_from_guest
>>>  #define __raw_clear_guest raw_clear_guest
>>> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
>>> index d4ed23df28..e489d0bf21 100644
>>> --- a/xen/include/asm-arm/vgic.h
>>> +++ b/xen/include/asm-arm/vgic.h
>>> @@ -217,9 +217,6 @@ extern void register_vgic_ops(struct domain *d,
>>> const struct vgic_ops *ops);
>>>  int vgic_v2_init(struct domain *d, int *mmio_count);
>>>  int vgic_v3_init(struct domain *d, int *mmio_count);
>>>
>>> -int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
>>> - uint32_t size, bool_t is_write);
>>> -
>>>  extern int domain_vgic_register(struct domain *d, int *mmio_count);
>>>  extern int vcpu_vgic_free(struct vcpu *v);
>>>  extern bool vgic_to_sgi(struct vcpu *v, register_t sgir,
>>
>> As Stefano and Andrew mentioned in patch 11/13, due to a recent patch in
>> staging, the upper patch failes building due to a missing declaration of
>> struct domain in . This can be easily fixed by adding a
>> forward declaration to struct domain right above
>> vgic_access_guest_memory in  as you will find in the
>> following patch.
>
> Why the forward declaration and not directly including xen/sched.h?

Yeap, that works too :)

Thanks,
~Sergej

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

Re: [Xen-devel] [PATCH] x86/mm: Drop more PV superpage leftovers

[Wei Liu]

On Wed, Aug 16, 2017 at 01:27:06PM +0100, Andrew Cooper wrote:
> Signed-off-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

[Xen-devel] [PATCH] x86/mm: Drop __PAGE_OFFSET

[Andrew Cooper]

It is a vestigial leftover of Xen having inherited Linux's memory management
code in the early days.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Wei Liu 
---
 xen/include/asm-x86/x86_64/page.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/xen/include/asm-x86/x86_64/page.h 
b/xen/include/asm-x86/x86_64/page.h
index c3eab62..1256277 100644
--- a/xen/include/asm-x86/x86_64/page.h
+++ b/xen/include/asm-x86/x86_64/page.h
@@ -19,7 +19,6 @@
 #define SUPERPAGE_ORDER PAGETABLE_ORDER
 #define SUPERPAGE_PAGES (1

[Xen-devel] [PATCH v3 22/52] xen/arch/x86/x86_64/mmconfig-shared.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/x86_64/mmconfig-shared.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
---
V3:
- dont return out of loop (Jan Beulich)
- dont modify option value in parsing function
---
 xen/arch/x86/x86_64/mmconfig-shared.c | 29 +
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/x86_64/mmconfig-shared.c 
b/xen/arch/x86/x86_64/mmconfig-shared.c
index 488470bfeb..5a7118481d 100644
--- a/xen/arch/x86/x86_64/mmconfig-shared.c
+++ b/xen/arch/x86/x86_64/mmconfig-shared.c
@@ -28,22 +28,35 @@
 
 unsigned int pci_probe = PCI_PROBE_CONF1 | PCI_PROBE_MMCONF;
 
-static void __init parse_mmcfg(char *s)
+static int __init parse_mmcfg(const char *s)
 {
-char *ss;
+const char *ss;
+int rc = 0;
 
 do {
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
-if ( !parse_bool(s) )
+switch ( parse_bool(s) ) {
+case 0:
 pci_probe &= ~PCI_PROBE_MMCONF;
-else if ( !strcmp(s, "amd_fam10") || !strcmp(s, "amd-fam10") )
-pci_probe |= PCI_CHECK_ENABLE_AMD_MMCONF;
+break;
+case 1:
+break;
+default:
+if ( !strncmp(s, "amd_fam10", ss - s) ||
+ !strncmp(s, "amd-fam10", ss - s) )
+pci_probe |= PCI_CHECK_ENABLE_AMD_MMCONF;
+else
+rc = -EINVAL;
+break;
+}
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 custom_param("mmcfg", parse_mmcfg);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 28/52] xen/common/sched_credit2.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/sched_credit2.c

to indicate whether the parameter value was parsed successfully.

Cc: George Dunlap 
Cc: Dario Faggioli 
Signed-off-by: Juergen Gross 
Acked-by: Dario Faggioli 
Acked-by: Wei Liu 
---
 xen/common/sched_credit2.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 29c002a63e..9b1db1351f 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -339,7 +339,7 @@ static const char *const opt_runqueue_str[] = {
 };
 static int __read_mostly opt_runqueue = OPT_RUNQUEUE_SOCKET;
 
-static void parse_credit2_runqueue(const char *s)
+static int parse_credit2_runqueue(const char *s)
 {
 unsigned int i;
 
@@ -348,11 +348,13 @@ static void parse_credit2_runqueue(const char *s)
 if ( !strcmp(s, opt_runqueue_str[i]) )
 {
 opt_runqueue = i;
-return;
+return 0;
 }
 }
 
 printk("WARNING, unrecognized value of credit2_runqueue option!\n");
+
+return -EINVAL;
 }
 custom_param("credit2_runqueue", parse_credit2_runqueue);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 21/52] xen/arch/x86/time.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/time.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/arch/x86/time.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index b988b94d2e..eba7aed72d 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1870,7 +1870,7 @@ int hwdom_pit_access(struct ioreq *ioreq)
  * tsc=skewed: Assume TSCs are individually reliable, but skewed across CPUs.
  * tsc=stable:socket: Assume TSCs are reliable across sockets.
  */
-static void __init tsc_parse(const char *s)
+static int __init tsc_parse(const char *s)
 {
 if ( !strcmp(s, "unstable") )
 {
@@ -1882,6 +1882,10 @@ static void __init tsc_parse(const char *s)
 setup_clear_cpu_cap(X86_FEATURE_TSC_RELIABLE);
 else if ( !strcmp(s, "stable:socket") )
 tsc_flags |= TSC_RELIABLE_SOCKET;
+else
+return -EINVAL;
+
+return 0;
 }
 custom_param("tsc", tsc_parse);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 25/52] xen/common/efi/boot.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/efi/boot.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Signed-off-by: Juergen Gross 
---
V3:
- dont return out of loops (Jan Beulich)
- dont modify option value in parsing function
---
 xen/common/efi/boot.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 11bdc7a2a4..01d33004e0 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1306,9 +1306,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
*SystemTable)
 
 static bool __initdata efi_map_uc;
 
-static void __init parse_efi_param(char *s)
+static int __init parse_efi_param(const char *s)
 {
-char *ss;
+const char *ss;
+int rc = 0;
 
 do {
 bool val = strncmp(s, "no-", 3);
@@ -1317,21 +1318,25 @@ static void __init parse_efi_param(char *s)
 s += 3;
 
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
-if ( !strcmp(s, "rs") )
+if ( !strncmp(s, "rs", ss - s) )
 {
 if ( val )
 __set_bit(EFI_RS, &efi_flags);
 else
 __clear_bit(EFI_RS, &efi_flags);
 }
-else if ( !strcmp(s, "attr=uc") )
+else if ( !strncmp(s, "attr=uc", ss - s) )
 efi_map_uc = val;
+else
+rc = -EINVAL;
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 custom_param("efi", parse_efi_param);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 10/52] xen/arch/x86/hvm/vmx/vmcs.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/hvm/vmx/vmcs.c

to indicate whether the parameter value was parsed successfully.

Cc: Jun Nakajima 
Cc: Kevin Tian 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
---
V3:
- dont modify option value in parsing function
---
 xen/arch/x86/hvm/vmx/vmcs.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 7854802cba..02f54e7f84 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -74,9 +74,10 @@ static s8 __read_mostly opt_ept_ad = -1;
  *  pml Enable PML
  *  ad  Use A/D bits
  */
-static void __init parse_ept_param(char *s)
+static int __init parse_ept_param(const char *s)
 {
-char *ss;
+const char *ss;
+int rc = 0;
 
 do {
 bool_t val = !!strncmp(s, "no-", 3);
@@ -85,16 +86,20 @@ static void __init parse_ept_param(char *s)
 s += 3;
 
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
-if ( !strcmp(s, "pml") )
+if ( !strncmp(s, "pml", ss - s) )
 opt_pml_enabled = val;
-else if ( !strcmp(s, "ad") )
+else if ( !strncmp(s, "ad", ss - s) )
 opt_ept_ad = val;
+else
+rc = -EINVAL;
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 custom_param("ept", parse_ept_param);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 01/52] xen/arch/arm/acpi/boot.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/arm/acpi/boot.c

to indicate whether the parameter value was parsed successfully.

Cc: Stefano Stabellini 
Cc: Julien Grall 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Julien Grall 
---
 xen/arch/arm/acpi/boot.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index 889208a0ea..46b4352d01 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -193,16 +193,20 @@ static int __init acpi_parse_fadt(struct 
acpi_table_header *table)
 static bool_t __initdata param_acpi_off;
 static bool_t __initdata param_acpi_force;
 
-static void __init parse_acpi_param(char *arg)
+static int __init parse_acpi_param(const char *arg)
 {
 if ( !arg )
-return;
+return -EINVAL;
 
 /* Interpret the parameter for use within Xen. */
 if ( !parse_bool(arg) )
 param_acpi_off = true;
 else if ( !strcmp(arg, "force") ) /* force ACPI to be enabled */
 param_acpi_force = true;
+else
+return -EINVAL;
+
+return 0;
 }
 custom_param("acpi", parse_acpi_param);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 47/52] xen: add basic support for runtime parameter changing

[Juergen Gross]

Add the needed infrastructure for runtime parameter changing similar
to that used at boot time via cmdline. We are using the same parsing
functions as for cmdline parsing, but with a different array of
parameter definitions.

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Signed-off-by: Juergen Gross 
---
V2:
- added modification of ARM linker script (Wei Liu)

V3:
- moved runtime parameter array in linker scripts (Jan Beulich)
- renamed macros to *_runtime_param() (Jan Beulich)
- added *runtime_only_param() macros (Jan Beulich)
- let *_runtime_param() macros include boot param functionality
  (Jan Beulich)
---
 xen/arch/arm/xen.lds.S |  4 
 xen/arch/x86/xen.lds.S |  4 
 xen/common/kernel.c|  5 +
 xen/include/xen/init.h | 57 --
 xen/include/xen/lib.h  |  1 +
 5 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 2d54f224ec..c9b9546435 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -61,6 +61,10 @@ SECTIONS
*(.lockprofile.data)
__lock_profile_end = .;
 #endif
+   . = ALIGN(POINTER_ALIGN);
+   __param_start = .;
+   *(.data.param)
+   __param_end = .;
   } :text
 
 #if defined(BUILD_ID)
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index ff08bbe42a..6a7bbb8ca1 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -120,6 +120,10 @@ SECTIONS
*(.lockprofile.data)
__lock_profile_end = .;
 #endif
+   . = ALIGN(POINTER_ALIGN);
+   __param_start = .;
+   *(.data.param)
+   __param_end = .;
   } :text
 
 #if defined(BUILD_ID)
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 1b66ad4e55..2ab8ed496d 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -188,6 +188,11 @@ static void __init _cmdline_parse(const char *cmdline)
 parse_params(cmdline, __setup_start, __setup_end);
 }
 
+int runtime_parse(const char *line)
+{
+return parse_params(line, __param_start, __param_end);
+}
+
 /**
  *cmdline_parse -- parses the xen command line.
  * If CONFIG_CMDLINE is set, it would be parsed prior to @cmdline.
diff --git a/xen/include/xen/init.h b/xen/include/xen/init.h
index 234ec25aae..db06c76fdf 100644
--- a/xen/include/xen/init.h
+++ b/xen/include/xen/init.h
@@ -90,11 +90,16 @@ struct kernel_param {
 };
 
 extern const struct kernel_param __setup_start[], __setup_end[];
+extern const struct kernel_param __param_start[], __param_end[];
+
+#define __dataparam   __used_section(".data.param")
+
+#define __param(att)  static const att \
+__attribute__((__aligned__(sizeof(void * struct kernel_param
 
 #define __setup_str static const __initconst \
 __attribute__((__aligned__(1))) char
-#define __kparam static const __initsetup \
-__attribute__((__aligned__(sizeof(void * struct kernel_param
+#define __kparam  __param(__initsetup)
 
 #define custom_param(_name, _var) \
 __setup_str __setup_str_##_var[] = _name; \
@@ -131,6 +136,54 @@ extern const struct kernel_param __setup_start[], 
__setup_end[];
   .len = sizeof(_var), \
   .par.var = &_var }
 
+#define __rtparam __param(__dataparam)
+
+#define custom_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+  { .name = _name, \
+  .type = OPT_CUSTOM, \
+  .par.func = _var }
+#define boolean_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+{ .name = _name, \
+  .type = OPT_BOOL, \
+  .len = sizeof(_var), \
+  .par.var = &_var }
+#define integer_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+{ .name = _name, \
+  .type = OPT_UINT, \
+  .len = sizeof(_var), \
+  .par.var = &_var }
+#define size_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+{ .name = _name, \
+  .type = OPT_SIZE, \
+  .len = sizeof(_var), \
+  .par.var = &_var }
+#define string_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+{ .name = _name, \
+  .type = OPT_STR, \
+  .len = sizeof(_var), \
+  .par.var = &_var }
+
+#define custom_runtime_param(_name, _var) \
+custom_param(_name, _var); \
+custom_runtime_only_param(_name, _var)
+#define boolean_runtime_param(_name, _var) \
+boolean_param(_name, _var); \
+boolean_runtime_only_param(_name, _var)
+#define integer_runtime_param(_name, _var) \
+integer_param(_name, _var); \
+integer_runtime_only_param(_name, _var)
+#define size_runtime_param(_name, _var) \
+size_param(_name, _var); \
+size_runtime_only_param(_name, _var)
+#define string_runtime_param(_name, _var) \
+string_param(_name, _var); \
+string_runtime_only_param(_name, _var)
+
 #endif /* __ASSEMBLY__ *

[Xen-devel] [PATCH v3 44/52] xen/common/kexec.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
some messages issued by parse_low_crashinfo() and
parse_crashinfo_maxaddr() can be removed.

Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
---
 xen/common/kexec.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index fcc68bd4d8..e0f1e3eafa 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -213,7 +213,6 @@ static int __init parse_low_crashinfo(const char *str)
 low_crashinfo_mode = LOW_CRASHINFO_ALL;
 else
 {
-printk("Unknown low_crashinfo parameter '%s'.  Defaulting to min.\n", 
str);
 low_crashinfo_mode = LOW_CRASHINFO_MIN;
 return -EINVAL;
 }
@@ -240,11 +239,7 @@ static int __init parse_crashinfo_maxaddr(const char *str)
 if ( (addr = parse_size_and_unit(str, &q)) )
 crashinfo_maxaddr = addr;
 else
-{
-printk("Unable to parse crashinfo_maxaddr. Defaulting to 
%"PRIpaddr"\n",
-   crashinfo_maxaddr);
 return -EINVAL;
-}
 
 return *q ? -EINVAL : 0;
 }
-- 
2.12.3


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

[Xen-devel] [PATCH v3 29/52] xen/drivers/acpi/tables.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/acpi/tables.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/drivers/acpi/tables.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/acpi/tables.c b/xen/drivers/acpi/tables.c
index dd2031f36a..b890b73901 100644
--- a/xen/drivers/acpi/tables.c
+++ b/xen/drivers/acpi/tables.c
@@ -456,14 +456,15 @@ int __init acpi_table_init(void)
return 0;
 }
 
-static int __init acpi_parse_apic_instance(char *str)
+static int __init acpi_parse_apic_instance(const char *str)
 {
+   const char *q;
 
-   acpi_apic_instance = simple_strtoul(str, NULL, 0);
+   acpi_apic_instance = simple_strtoul(str, &q, 0);
 
printk(KERN_NOTICE PREFIX "Shall use APIC/MADT table %d\n",
   acpi_apic_instance);
 
-   return 0;
+   return *q ? -EINVAL : 0;
 }
 custom_param("acpi_apic_instance", acpi_parse_apic_instance);
-- 
2.12.3


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

[Xen-devel] [PATCH v3 24/52] xen/common/domain.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/domain.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
V3:
- cosmetic changes (Jan Beulich)
---
 xen/common/domain.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index b22aacc57e..acd420a1a5 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -245,12 +245,15 @@ static int late_hwdom_init(struct domain *d)
 
 static unsigned int __read_mostly extra_hwdom_irqs;
 static unsigned int __read_mostly extra_domU_irqs = 32;
-static void __init parse_extra_guest_irqs(const char *s)
+
+static int __init parse_extra_guest_irqs(const char *s)
 {
 if ( isdigit(*s) )
 extra_domU_irqs = simple_strtoul(s, &s, 0);
 if ( *s == ',' && isdigit(*++s) )
 extra_hwdom_irqs = simple_strtoul(s, &s, 0);
+
+return *s ? -EINVAL : 0;
 }
 custom_param("extra_guest_irqs", parse_extra_guest_irqs);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 32/52] xen/drivers/passthrough/amd/iommu_acpi.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/amd/iommu_acpi.c

to indicate whether the parameter value was parsed successfully.

Cc: Suravee Suthikulpanit 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
---
 xen/drivers/passthrough/amd/iommu_acpi.c | 26 +-
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c 
b/xen/drivers/passthrough/amd/iommu_acpi.c
index f4c7206c2a..64d10481d7 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -632,21 +632,23 @@ static u16 __init parse_ivhd_device_extended_range(
 return dev_length;
 }
 
-static void __init parse_ivrs_ioapic(char *str)
+static int __init parse_ivrs_ioapic(const char *str)
 {
 const char *s = str;
 unsigned long id;
 unsigned int seg, bus, dev, func;
 unsigned int idx;
 
-ASSERT(*s == '[');
+if ( *s != '[' )
+return -EINVAL;
+
 id = simple_strtoul(s + 1, &s, 0);
 if ( *s != ']' || *++s != '=' )
-return;
+return -EINVAL;
 
 s = parse_pci(s + 1, &seg, &bus, &dev, &func);
 if ( !s || *s )
-return;
+return -EINVAL;
 
 idx = ioapic_id_to_index(id);
 if ( idx == MAX_IO_APICS )
@@ -655,7 +657,7 @@ static void __init parse_ivrs_ioapic(char *str)
 if ( idx == MAX_IO_APICS )
 {
 printk(XENLOG_ERR "Error: %s: Too many IO APICs.\n", __func__);
-return;
+return -EINVAL;
 }
 }
 
@@ -663,28 +665,34 @@ static void __init parse_ivrs_ioapic(char *str)
 ioapic_sbdf[idx].seg = seg;
 ioapic_sbdf[idx].id = id;
 ioapic_sbdf[idx].cmdline = true;
+
+return 0;
 }
 custom_param("ivrs_ioapic[", parse_ivrs_ioapic);
 
-static void __init parse_ivrs_hpet(char *str)
+static int __init parse_ivrs_hpet(const char *str)
 {
 const char *s = str;
 unsigned long id;
 unsigned int seg, bus, dev, func;
 
-ASSERT(*s == '[');
+if ( *s != '[' )
+return -EINVAL;
+
 id = simple_strtoul(s + 1, &s, 0);
 if ( id != (typeof(hpet_sbdf.id))id || *s != ']' || *++s != '=' )
-return;
+return -EINVAL;
 
 s = parse_pci(s + 1, &seg, &bus, &dev, &func);
 if ( !s || *s )
-return;
+return -EINVAL;
 
 hpet_sbdf.id = id;
 hpet_sbdf.bdf = PCI_BDF(bus, dev, func);
 hpet_sbdf.seg = seg;
 hpet_sbdf.init = HPET_CMDL;
+
+return 0;
 }
 custom_param("ivrs_hpet[", parse_ivrs_hpet);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 02/52] xen/arch/arm/domain_build.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/arm/domain_build.c

to indicate whether the parameter value was parsed successfully.

Cc: Stefano Stabellini 
Cc: Julien Grall 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Julien Grall 
---
 xen/arch/arm/domain_build.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 1bec4fa23d..d6f9585503 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -33,9 +33,11 @@ int dom0_11_mapping = 1;
 
 static u64 __initdata dom0_mem;
 
-static void __init parse_dom0_mem(const char *s)
+static int __init parse_dom0_mem(const char *s)
 {
 dom0_mem = parse_size_and_unit(s, &s);
+
+return *s ? -EINVAL : 0;
 }
 custom_param("dom0_mem", parse_dom0_mem);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 11/52] xen/arch/x86/io_apic.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/io_apic.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/arch/x86/io_apic.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 2838f6bd99..f767c4560c 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -1581,7 +1581,7 @@ static unsigned int startup_level_ioapic_irq(struct 
irq_desc *desc)
 return 0; /* don't check for pending */
 }
 
-static void __init setup_ioapic_ack(char *s)
+static int __init setup_ioapic_ack(const char *s)
 {
 if ( !strcmp(s, "old") )
 {
@@ -1594,7 +1594,12 @@ static void __init setup_ioapic_ack(char *s)
 ioapic_ack_forced = true;
 }
 else
+{
 printk("Unknown ioapic_ack value specified: '%s'\n", s);
+return -EINVAL;
+}
+
+return 0;
 }
 custom_param("ioapic_ack", setup_ioapic_ack);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 17/52] xen/arch/x86/oprofile/nmi_int.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/oprofile/nmi_int.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/arch/x86/oprofile/nmi_int.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/arch/x86/oprofile/nmi_int.c b/xen/arch/x86/oprofile/nmi_int.c
index 126f7a8d9f..84ba108fdb 100644
--- a/xen/arch/x86/oprofile/nmi_int.c
+++ b/xen/arch/x86/oprofile/nmi_int.c
@@ -323,12 +323,15 @@ static int __init p4_init(char ** cpu_type)
 
 
 static int force_arch_perfmon;
+
 static int force_cpu_type(const char *str)
 {
if (!strcmp(str, "arch_perfmon")) {
force_arch_perfmon = 1;
printk(KERN_INFO "oprofile: forcing architectural perfmon\n");
}
+   else
+   return -EINVAL;
 
return 0;
 }
-- 
2.12.3


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

[Xen-devel] [PATCH v3 15/52] xen/arch/x86/nmi.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/nmi.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
V3:
- cosmetic changes (Jan Beulich)
---
 xen/arch/x86/nmi.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index 8914581f66..17013192dd 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -46,35 +46,44 @@ bool __initdata opt_watchdog;
 /* watchdog_force: If true, process unknown NMIs when running the watchdog. */
 bool watchdog_force;
 
-static void __init parse_watchdog(char *s)
+static int __init parse_watchdog(const char *s)
 {
 if ( !*s )
 {
 opt_watchdog = true;
-return;
+return 0;
 }
 
 switch ( parse_bool(s) )
 {
 case 0:
 opt_watchdog = false;
-return;
+return 0;
 case 1:
 opt_watchdog = true;
-return;
+return 0;
 }
 
 if ( !strcmp(s, "force") )
 watchdog_force = opt_watchdog = true;
+else
+return -EINVAL;
+
+return 0;
 }
 custom_param("watchdog", parse_watchdog);
 
 /* opt_watchdog_timeout: Number of seconds to wait before panic. */
 static unsigned int opt_watchdog_timeout = 5;
-static void parse_watchdog_timeout(char * s)
+
+static int parse_watchdog_timeout(const char *s)
 {
-opt_watchdog_timeout = simple_strtoull(s, NULL, 0);
+const char *q;
+
+opt_watchdog_timeout = simple_strtoull(s, &q, 0);
 opt_watchdog = !!opt_watchdog_timeout;
+
+return *q ? -EINVAL : 0;
 }
 custom_param("watchdog_timeout", parse_watchdog_timeout);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 14/52] xen/arch/x86/mm.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/mm.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
V3:
- cosmetic changes (Jan Beulich)
---
 xen/arch/x86/mm.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 5b863c6fa6..5afb0c3e21 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -170,14 +170,20 @@ static uint32_t base_disallow_mask;
  L1_DISALLOW_MASK : (L1_DISALLOW_MASK & ~PAGE_CACHE_ATTRS))
 
 static s8 __read_mostly opt_mmio_relax;
-static void __init parse_mmio_relax(const char *s)
+
+static int __init parse_mmio_relax(const char *s)
 {
 if ( !*s )
 opt_mmio_relax = 1;
 else
 opt_mmio_relax = parse_bool(s);
 if ( opt_mmio_relax < 0 && strcmp(s, "all") )
+{
 opt_mmio_relax = 0;
+return -EINVAL;
+}
+
+return 0;
 }
 custom_param("mmio-relax", parse_mmio_relax);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 38/52] xen/xsm/flask/flask_op.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/xsm/flask/flask_op.c

to indicate whether the parameter value was parsed successfully.

Cc: Daniel De Graaf 
Signed-off-by: Juergen Gross 
Acked-by: Daniel De Graaf 
Acked-by: Wei Liu 
---
 xen/xsm/flask/flask_op.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index 15aa96b09f..03baa28b68 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -26,7 +26,7 @@
 #define _copy_from_guest copy_from_guest
 
 enum flask_bootparam_t __read_mostly flask_bootparam = 
FLASK_BOOTPARAM_ENFORCING;
-static void parse_flask_param(char *s);
+static int parse_flask_param(const char *s);
 custom_param("flask", parse_flask_param);
 
 bool __read_mostly flask_enforcing = true;
@@ -58,7 +58,7 @@ static int flask_security_make_bools(void);
 
 extern int ss_initialized;
 
-static void __init parse_flask_param(char *s)
+static int __init parse_flask_param(const char *s)
 {
 if ( !strcmp(s, "enforcing") )
 flask_bootparam = FLASK_BOOTPARAM_ENFORCING;
@@ -70,6 +70,8 @@ static void __init parse_flask_param(char *s)
 flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
 else
 flask_bootparam = FLASK_BOOTPARAM_INVALID;
+
+return (flask_bootparam == FLASK_BOOTPARAM_INVALID) ? -EINVAL : 0;
 }
 
 static int domain_has_security(struct domain *d, u32 perms)
-- 
2.12.3


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

[Xen-devel] [PATCH v3 30/52] xen/drivers/char/console.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/char/console.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/drivers/char/console.c | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index f0659fba1b..17ad02083d 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -67,7 +67,7 @@ enum con_timestamp_mode
 
 static enum con_timestamp_mode __read_mostly opt_con_timestamp_mode = TSM_NONE;
 
-static void parse_console_timestamps(char *s);
+static int parse_console_timestamps(const char *s);
 custom_param("console_timestamps", parse_console_timestamps);
 
 /* conring_size: allows a large console ring than default (16kB). */
@@ -123,8 +123,8 @@ static int __read_mostly xenlog_guest_upper_thresh =
 static int __read_mostly xenlog_guest_lower_thresh =
 XENLOG_GUEST_LOWER_THRESHOLD;
 
-static void parse_loglvl(char *s);
-static void parse_guest_loglvl(char *s);
+static int parse_loglvl(const char *s);
+static int parse_guest_loglvl(const char *s);
 
 /*
  *  := none|error|warning|info|debug|all
@@ -145,7 +145,7 @@ static atomic_t print_everything = ATOMIC_INIT(0);
 return (lvlnum);\
 }
 
-static int __init __parse_loglvl(char *s, char **ps)
+static int __init __parse_loglvl(const char *s, const char **ps)
 {
 ___parse_loglvl(s, ps, "none",0);
 ___parse_loglvl(s, ps, "error",   1);
@@ -156,23 +156,26 @@ static int __init __parse_loglvl(char *s, char **ps)
 return 2; /* sane fallback */
 }
 
-static void __init _parse_loglvl(char *s, int *lower, int *upper)
+static int __init _parse_loglvl(const char *s, int *lower, int *upper)
 {
 *lower = *upper = __parse_loglvl(s, &s);
 if ( *s == '/' )
 *upper = __parse_loglvl(s+1, &s);
 if ( *upper < *lower )
 *upper = *lower;
+
+return *s ? -EINVAL : 0;
 }
 
-static void __init parse_loglvl(char *s)
+static int __init parse_loglvl(const char *s)
 {
-_parse_loglvl(s, &xenlog_lower_thresh, &xenlog_upper_thresh);
+return _parse_loglvl(s, &xenlog_lower_thresh, &xenlog_upper_thresh);
 }
 
-static void __init parse_guest_loglvl(char *s)
+static int __init parse_guest_loglvl(const char *s)
 {
-_parse_loglvl(s, &xenlog_guest_lower_thresh, &xenlog_guest_upper_thresh);
+return _parse_loglvl(s, &xenlog_guest_lower_thresh,
+ &xenlog_guest_upper_thresh);
 }
 
 static char *loglvl_str(int lvl)
@@ -603,16 +606,16 @@ static int printk_prefix_check(char *p, char **pp)
 ((loglvl < upper_thresh) && printk_ratelimit()));
 } 
 
-static void __init parse_console_timestamps(char *s)
+static int __init parse_console_timestamps(const char *s)
 {
 switch ( parse_bool(s) )
 {
 case 0:
 opt_con_timestamp_mode = TSM_NONE;
-return;
+return 0;
 case 1:
 opt_con_timestamp_mode = TSM_DATE;
-return;
+return 0;
 }
 if ( *s == '\0' || /* Compat for old booleanparam() */
  !strcmp(s, "date") )
@@ -623,6 +626,10 @@ static void __init parse_console_timestamps(char *s)
 opt_con_timestamp_mode = TSM_BOOT;
 else if ( !strcmp(s, "none") )
 opt_con_timestamp_mode = TSM_NONE;
+else
+return -EINVAL;
+
+return 0;
 }
 
 static void printk_start_of_line(const char *prefix)
-- 
2.12.3


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

[Xen-devel] [PATCH v3 42/52] xen/arch/x86/hvm/viridian.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by parse_viridian_version() can be removed.

Cc: Paul Durrant 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Reviewed-by: Paul Durrant 
---
 xen/arch/x86/hvm/viridian.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index 2edf9d0b23..e0546f32fc 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -1104,7 +1104,7 @@ static int __init parse_viridian_version(const char *arg)
 {
 n[i] = simple_strtoul(arg, &e, 0);
 if ( e != t )
-goto fail;
+break;
 }
 
 i++;
@@ -1112,12 +1112,12 @@ static int __init parse_viridian_version(const char 
*arg)
 } while ( *t );
 
 if ( i != 3 )
-goto fail;
+return -EINVAL;
 
 if ( ((typeof(viridian_major))n[0] != n[0]) ||
  ((typeof(viridian_minor))n[1] != n[1]) ||
  ((typeof(viridian_build))n[2] != n[2]) )
-goto fail;
+return -EINVAL;
 
 viridian_major = n[0];
 viridian_minor = n[1];
@@ -1126,10 +1126,6 @@ static int __init parse_viridian_version(const char *arg)
 printk("viridian-version = %#x,%#x,%#x\n",
viridian_major, viridian_minor, viridian_build);
 return 0;
-
- fail:
-printk(XENLOG_WARNING "Invalid viridian-version, using default\n");
-return -EINVAL;
 }
 custom_param("viridian-version", parse_viridian_version);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 34/52] xen/drivers/passthrough/pci.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/pci.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Signed-off-by: Juergen Gross 
---
V3:
- cosmetic changes (Jan Beulich)
- dont modify option value in parsing funtion
---
 xen/drivers/passthrough/pci.c | 30 +++---
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 27bdb7163c..fdc53aef12 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -149,17 +149,18 @@ static struct phantom_dev {
 } phantom_devs[8];
 static unsigned int nr_phantom_devs;
 
-static void __init parse_phantom_dev(char *str) {
+static int __init parse_phantom_dev(const char *str)
+{
 const char *s = str;
 unsigned int seg, bus, slot;
 struct phantom_dev phantom;
 
 if ( !s || !*s || nr_phantom_devs >= ARRAY_SIZE(phantom_devs) )
-return;
+return -EINVAL;
 
 s = parse_pci(s, &seg, &bus, &slot, NULL);
 if ( !s || *s != ',' )
-return;
+return -EINVAL;
 
 phantom.seg = seg;
 phantom.bus = bus;
@@ -170,10 +171,12 @@ static void __init parse_phantom_dev(char *str) {
 case 1: case 2: case 4:
 if ( *s )
 default:
-return;
+return -EINVAL;
 }
 
 phantom_devs[nr_phantom_devs++] = phantom;
+
+return 0;
 }
 custom_param("pci-phantom", parse_phantom_dev);
 
@@ -189,9 +192,10 @@ static u16 __read_mostly bridge_ctl_mask;
  *   perr   don't suppress parity errors (default)
  *   no-perrsuppress parity errors
  */
-static void __init parse_pci_param(char *s)
+static int __init parse_pci_param(const char *s)
 {
-char *ss;
+const char *ss;
+int rc = 0;
 
 do {
 bool_t on = !!strncmp(s, "no-", 3);
@@ -201,19 +205,21 @@ static void __init parse_pci_param(char *s)
 s += 3;
 
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
-if ( !strcmp(s, "serr") )
+if ( !strncmp(s, "serr", ss - s) )
 {
 cmd_mask = PCI_COMMAND_SERR;
 brctl_mask = PCI_BRIDGE_CTL_SERR | PCI_BRIDGE_CTL_DTMR_SERR;
 }
-else if ( !strcmp(s, "perr") )
+else if ( !strncmp(s, "perr", ss - s) )
 {
 cmd_mask = PCI_COMMAND_PARITY;
 brctl_mask = PCI_BRIDGE_CTL_PARITY;
 }
+else
+rc = -EINVAL;
 
 if ( on )
 {
@@ -227,7 +233,9 @@ static void __init parse_pci_param(char *s)
 }
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 custom_param("pci", parse_pci_param);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 19/52] xen/arch/x86/setup.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/setup.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/arch/x86/setup.c | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index db5df6956d..1ae5b90fe9 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -67,7 +67,7 @@ unsigned long __read_mostly cr4_pv32_mask;
 /* "acpi=force":  Override the disable blacklist.   */
 /* "acpi=ht": Limit ACPI just to boot-time to enable HT.*/
 /* "acpi=noirq":  Disables ACPI interrupt routing.  */
-static void parse_acpi_param(char *s);
+static int parse_acpi_param(const char *s);
 custom_param("acpi", parse_acpi_param);
 
 /*  Linux config option: propagated to domain0. */
@@ -102,59 +102,70 @@ unsigned long __read_mostly mmu_cr4_features = 
XEN_MINIMAL_CR4;
 /* smep: Enable/disable Supervisor Mode Execution Protection (default on). */
 #define SMEP_HVM_ONLY (-1)
 static s8 __initdata opt_smep = 1;
-static void __init parse_smep_param(char *s)
+
+static int __init parse_smep_param(const char *s)
 {
 if ( !*s )
 {
 opt_smep = 1;
-return;
+return 0;
 }
 
 switch ( parse_bool(s) )
 {
 case 0:
 opt_smep = 0;
-return;
+return 0;
 case 1:
 opt_smep = 1;
-return;
+return 0;
 }
 
 if ( !strcmp(s, "hvm") )
 opt_smep = SMEP_HVM_ONLY;
+else
+return -EINVAL;
+
+return 0;
 }
 custom_param("smep", parse_smep_param);
 
 /* smap: Enable/disable Supervisor Mode Access Prevention (default on). */
 #define SMAP_HVM_ONLY (-1)
 static s8 __initdata opt_smap = 1;
-static void __init parse_smap_param(char *s)
+
+static int __init parse_smap_param(const char *s)
 {
 if ( !*s )
 {
 opt_smap = 1;
-return;
+return 0;
 }
 
 switch ( parse_bool(s) )
 {
 case 0:
 opt_smap = 0;
-return;
+return 0;
 case 1:
 opt_smap = 1;
-return;
+return 0;
 }
 
 if ( !strcmp(s, "hvm") )
 opt_smap = SMAP_HVM_ONLY;
+else
+return -EINVAL;
+
+return 0;
 }
 custom_param("smap", parse_smap_param);
 
 bool __read_mostly acpi_disabled;
 bool __initdata acpi_force;
 static char __initdata acpi_param[10] = "";
-static void __init parse_acpi_param(char *s)
+
+static int __init parse_acpi_param(const char *s)
 {
 /* Save the parameter so it can be propagated to domain0. */
 safe_strcpy(acpi_param, s);
@@ -180,6 +191,10 @@ static void __init parse_acpi_param(char *s)
 {
 acpi_noirq_set();
 }
+else
+return -EINVAL;
+
+return 0;
 }
 
 static const module_t *__initdata initial_images;
-- 
2.12.3


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

[Xen-devel] [PATCH v3 36/52] xen/drivers/passthrough/vtd/quirks.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/vtd/quirks.c

to indicate whether the parameter value was parsed successfully.

Cc: Kevin Tian 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
---
 xen/drivers/passthrough/vtd/quirks.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/quirks.c 
b/xen/drivers/passthrough/vtd/quirks.c
index 5bbbd96d51..4e61ac9ddb 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -247,9 +247,10 @@ void vtd_ops_postamble_quirk(struct iommu* iommu)
 }
 }
 
-static void __init parse_snb_timeout(const char *s)
+static int __init parse_snb_timeout(const char *s)
 {
 int t;
+const char *q = NULL;
 
 t = parse_bool(s);
 if ( t < 0 )
@@ -259,13 +260,13 @@ static void __init parse_snb_timeout(const char *s)
 else if ( strcmp(s, "cap") == 0 )
 t = SNB_IGD_TIMEOUT;
 else
-t = strtoul(s, NULL, 0);
+t = strtoul(s, &q, 0);
 }
 else
 t = t ? SNB_IGD_TIMEOUT_LEGACY : 0;
 snb_igd_timeout = MILLISECS(t);
 
-return;
+return (q && *q) ? -EINVAL : 0;
 }
 custom_param("snb_igd_quirk", parse_snb_timeout);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 16/52] xen/arch/x86/numa.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/numa.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/arch/x86/numa.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index d45196fafc..3311b3b080 100644
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -18,7 +18,7 @@
 #include 
 #include 
 
-static int numa_setup(char *s);
+static int numa_setup(const char *s);
 custom_param("numa", numa_setup);
 
 #ifndef Dprintk
@@ -299,14 +299,14 @@ void numa_set_node(int cpu, nodeid_t node)
 }
 
 /* [numa=off] */
-static __init int numa_setup(char *opt) 
-{ 
+static __init int numa_setup(const char *opt)
+{
 if ( !strncmp(opt,"off",3) )
 numa_off = true;
-if ( !strncmp(opt,"on",2) )
+else if ( !strncmp(opt,"on",2) )
 numa_off = false;
 #ifdef CONFIG_NUMA_EMU
-if ( !strncmp(opt, "fake=", 5) )
+else if ( !strncmp(opt, "fake=", 5) )
 {
 numa_off = false;
 numa_fake = simple_strtoul(opt+5,NULL,0);
@@ -315,14 +315,16 @@ static __init int numa_setup(char *opt)
 }
 #endif
 #ifdef CONFIG_ACPI_NUMA
-if ( !strncmp(opt,"noacpi",6) )
+else if ( !strncmp(opt,"noacpi",6) )
 {
 numa_off = false;
 acpi_numa = -1;
 }
 #endif
+else
+return -EINVAL;
 
-return 1;
+return 0;
 } 
 
 /*
-- 
2.12.3


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

[Xen-devel] [PATCH v3 40/52] xen/arch/x86/apic.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by apic_set_verbosity() can be removed.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Jan Beulich 
---
 xen/arch/x86/apic.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 325b48074a..2638414e08 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -800,11 +800,8 @@ static int __init apic_set_verbosity(const char *str)
 apic_verbosity = APIC_DEBUG;
 else if (strcmp("verbose", str) == 0)
 apic_verbosity = APIC_VERBOSE;
-else {
-printk(KERN_WARNING "APIC Verbosity level %s not recognised"
-   " use apic_verbosity=verbose or apic_verbosity=debug", str);
+else
 return -EINVAL;
-}
 
 return 0;
 }
-- 
2.12.3


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

[Xen-devel] [PATCH v3 13/52] xen/arch/x86/microcode.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/microcode.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/arch/x86/microcode.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 7558202efa..77c1efc97f 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -73,15 +73,19 @@ void __init microcode_set_module(unsigned int idx)
  * If the EFI has forced which of the multiboot payloads is to be used,
  * no parsing will be attempted.
  */
-static void __init parse_ucode(char *s)
+static int __init parse_ucode(const char *s)
 {
+const char *q = NULL;
+
 if ( ucode_mod_forced ) /* Forced by EFI */
-   return;
+   return 0;
 
 if ( !strncmp(s, "scan", 4) )
 ucode_scan = 1;
 else
-ucode_mod_idx = simple_strtol(s, NULL, 0);
+ucode_mod_idx = simple_strtol(s, &q, 0);
+
+return (q && *q) ? -EINVAL : 0;
 }
 custom_param("ucode", parse_ucode);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 46/52] xen: carve out a generic parsing function from _cmdline_parse()

[Juergen Gross]

In order to support generic parameter parsing carve out the parser from
_cmdline_parse(). As this generic function might be called after boot
remove the __init annotations from all called sub-functions.

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Signed-off-by: Juergen Gross 
Reviewed-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/common/kernel.c | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index c629ffa11c..1b66ad4e55 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -23,8 +23,7 @@ enum system_state system_state = SYS_STATE_early_boot;
 xen_commandline_t saved_cmdline;
 static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
 
-static int __init assign_integer_param(
-const struct kernel_param *param, uint64_t val)
+static int assign_integer_param(const struct kernel_param *param, uint64_t val)
 {
 switch ( param->len )
 {
@@ -53,12 +52,13 @@ static int __init assign_integer_param(
 return 0;
 }
 
-static void __init _cmdline_parse(const char *cmdline)
+static int parse_params(const char *cmdline, const struct kernel_param *start,
+const struct kernel_param *end)
 {
 char opt[128], *optval, *optkey, *q;
 const char *p = cmdline, *s, *key;
 const struct kernel_param *param;
-int bool_assert, rctmp, rc;
+int bool_assert, rctmp, rc, final_rc = 0;
 bool found;
 
 for ( ; ; )
@@ -100,7 +100,7 @@ static void __init _cmdline_parse(const char *cmdline)
 
 rc = 0;
 found = false;
-for ( param = __setup_start; param < __setup_end; param++ )
+for ( param = start; param < end; param++ )
 {
 if ( strcmp(param->name, optkey) )
 {
@@ -169,10 +169,23 @@ static void __init _cmdline_parse(const char *cmdline)
 }
 
 if ( rc )
+{
 printk("parameter \"%s\" has invalid value \"%s\"!\n", key, 
optval);
+final_rc = rc;
+}
 if ( !found )
+{
 printk("parameter \"%s\" unknown!\n", key);
+final_rc = -EINVAL;
+}
 }
+
+return final_rc;
+}
+
+static void __init _cmdline_parse(const char *cmdline)
+{
+parse_params(cmdline, __setup_start, __setup_end);
 }
 
 /**
@@ -197,7 +210,7 @@ void __init cmdline_parse(const char *cmdline)
 #endif
 }
 
-int __init parse_bool(const char *s)
+int parse_bool(const char *s)
 {
 if ( !strcmp("no", s) ||
  !strcmp("off", s) ||
-- 
2.12.3


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

[Xen-devel] [PATCH v3 35/52] xen/drivers/passthrough/vtd/dmar.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/vtd/dmar.c

to indicate whether the parameter value was parsed successfully.

Cc: Kevin Tian 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
---
 xen/drivers/passthrough/vtd/dmar.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/vtd/dmar.c 
b/xen/drivers/passthrough/vtd/dmar.c
index 82040ddc05..dd122da730 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -1090,7 +1090,7 @@ int intel_iommu_get_reserved_device_memory(iommu_grdm_t 
*func, void *ctxt)
  * If a segment is specified for other than the first device, and it does not
  * match the one specified for the first one, an error will be reported.
  */
-static void __init parse_rmrr_param(const char *str)
+static int __init parse_rmrr_param(const char *str)
 {
 const char *s = str, *cur, *stmp;
 unsigned int seg, bus, dev, func, dev_count;
@@ -1143,5 +1143,7 @@ static void __init parse_rmrr_param(const char *str)
 nr_rmrr++;
 
 } while ( *s++ == ';' && nr_rmrr < MAX_USER_RMRR );
+
+return *s ? -EINVAL : 0;
 }
 custom_param("rmrr", parse_rmrr_param);
-- 
2.12.3


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

[Xen-devel] [PATCH v3 12/52] xen/arch/x86/irq.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/irq.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/arch/x86/irq.c | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 57e6c18970..77ca181985 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -26,7 +26,7 @@
 #include 
 #include 
 
-static void parse_irq_vector_map_param(char *s);
+static int parse_irq_vector_map_param(const char *s);
 
 /* opt_noirqbalance: If true, software IRQ balancing/affinity is disabled. */
 bool __read_mostly opt_noirqbalance;
@@ -60,24 +60,29 @@ static struct timer irq_ratelimit_timer;
 static unsigned int __read_mostly irq_ratelimit_threshold = 1;
 integer_param("irq_ratelimit", irq_ratelimit_threshold);
 
-static void __init parse_irq_vector_map_param(char *s)
+static int __init parse_irq_vector_map_param(const char *s)
 {
-char *ss;
+const char *ss;
+int rc = 0;
 
 do {
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
-if ( !strcmp(s, "none"))
+if ( !strncmp(s, "none", ss - s))
 opt_irq_vector_map=OPT_IRQ_VECTOR_MAP_NONE;
-else if ( !strcmp(s, "global"))
+else if ( !strncmp(s, "global", ss - s))
 opt_irq_vector_map=OPT_IRQ_VECTOR_MAP_GLOBAL;
-else if ( !strcmp(s, "per-device"))
+else if ( !strncmp(s, "per-device", ss - s))
 opt_irq_vector_map=OPT_IRQ_VECTOR_MAP_PERDEV;
+else
+rc = -EINVAL;
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 
 /* Must be called when irq disabled */
-- 
2.12.3


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

[Xen-devel] [PATCH v3 00/52] Support for modifying parameters at runtime

[Juergen Gross]

Currently parameters of the hypervisor (e.g. console log level) can be
set via boot command line. Instead of having to reboot the system in
case another setting is desired, being able to modify many of those
parameters at runtime would be the better option.

This patch series addresses this by adding a new xl command
"xl set-parameters" which takes a string similar to the boot command
line as parameter and passes this string to the hypervisor which will
then use the same parsing infrastructure as for the command line in
order to apply the parameter settings.

As error checks for invalid parameters or parameter values have been
very sparse if present at all in the hypervisor, a major part of this
patch series addresses this problem first: all custom parameter parsing
functions are being changed to return success or an error. The main
parsing function tests for generic parameter value errors (like e.g.
overflow) or invalid parameters and issues a message in case an error
has been detected. Most error messages in the custom parsing functions
are removed then.

While not strictly required for runtime parameter modification I
believe an improved parameter validation is a win with or without the
runtime parameter modification support.

* Patches 1-38 are modifying the custom parameter parsing functions to
  return success or error
* Patch 39 adds generic parameter error tests in the core parameter
  parsing functions
* Patches 40-45 remove custom parsing function error messages
* Patches 46-51 add the runtime parameter modification support
* Patch 52 adds support for runtime modification of some console related
  parameters 

Changes in V3:
- patches 7, 14, 15, 24, 27, 34: cosmetic changes (Jan Beulich)
- patches 7, 9, 10, 18, 22, 25, 31, 33, 34: dont modify option value in
parsing function
- patches 8, 22, 25: dont return out of loop (Jan Beulich)
- patch 9: fix error in parsing routine
- patch 18: let parse_psr_bool() return bool value (Jan Beulich)
- patch 18: return error in case no string matches (Jan Beulich)
- patch 20: dont stop loop at first invalid character (Jan Beulich)
- patch 31: remove prototype of cpufreq_cmdline_parse() from cpufreq.h
and make it static
- patch 39: use function pointer in struct kernel_param (Jan Beulich)
- patch 39: better range check in assign_integer_param() (Jan Beulich)
- patch 39: dont assign int values in case of overflow (Jan Beulich)
- patch 39: allow multiple handlers for a parameter (Jan Beulich)
- patch 47: moved runtime parameter array in linker scripts (Jan Beulich)
- patch 47: renamed macros to *_runtime_param() (Jan Beulich)
- patch 47: added *runtime_only_param() macros (Jan Beulich)
- patch 47: let *_runtime_param() macros include boot param functionality
(Jan Beulich)
- patch 48: check pad[] to be zero (Jan Beulich)
- patch 48: return E2BIG in case of parameters too long (Jan Beulich)
- patch 48: move max. parameter size define to sysctl.c (Jan Beulich)
- patch 49: zero padding fields in sysctl.u.set_parameter
- patch 50: use LOGEV() for error message
- patch 52: *_runtime_param() now includes boot parameter setting
(Jan Beulich)

Changes in V2:
- patch 39 (xen: check parameter validity when parsing command line):
  replaced literal 8 by BITS_PER_BYTE (Wei Liu)
  added test for empty string to parse_bool()
- patch 47 (xen: add basic support for runtime parameter changing):
  added modification of ARM linker script (Wei Liu)
- patch 48 (xen: add hypercall for setting parameters at runtime):
  corrected XSM test (Daniel De Graaf)
- patch 50 (libxl: add libxl_set_parameters() function):
  corrected coding style (Wei Liu)


Juergen Gross (52):
  xen/arch/arm/acpi/boot.c: let custom parameter parsing routines return
errno
  xen/arch/arm/domain_build.c: let custom parameter parsing routines
return errno
  xen/arch/arm/traps.c: let custom parameter parsing routines return
errno
  xen/arch/x86/apic.c: let custom parameter parsing routines return
errno
  xen/arch/x86/cpu/mcheck/mce.c: let custom parameter parsing routines
return errno
  xen/arch/x86/cpu/vpmu.c: let custom parameter parsing routines return
errno
  xen/arch/x86/dom0_build.c: let custom parameter parsing routines
return errno
  xen/arch/x86/genapic/probe.c: let custom parameter parsing routines
return errno
  xen/arch/x86/hvm/viridian.c: let custom parameter parsing routines
return errno
  xen/arch/x86/hvm/vmx/vmcs.c: let custom parameter parsing routines
return errno
  xen/arch/x86/io_apic.c: let custom parameter parsing routines return
errno
  xen/arch/x86/irq.c: let custom parameter parsing routines return errno
  xen/arch/x86/microcode.c: let custom parameter parsing routines return
errno
  xen/arch/x86/mm.c: let custom parameter parsing routines return errno
  xen/arch/x86/nmi.c: let custom parameter parsing routines return errno
  xen/arch/x86/numa.c: let custom parameter parsing routines return
errno
  xen/arch/x86/oprofile/nmi_int

[Xen-devel] [PATCH v3 04/52] xen/arch/x86/apic.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/apic.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/arch/x86/apic.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 851a6cc6cb..325b48074a 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -785,23 +785,28 @@ int lapic_resume(void)
  * Original code written by Keir Fraser.
  */
 
-static void __init lapic_disable(char *str)
+static int __init lapic_disable(const char *str)
 {
 enable_local_apic = -1;
 setup_clear_cpu_cap(X86_FEATURE_APIC);
+return 0;
 }
 custom_param("nolapic", lapic_disable);
 boolean_param("lapic", enable_local_apic);
 
-static void __init apic_set_verbosity(char *str)
+static int __init apic_set_verbosity(const char *str)
 {
 if (strcmp("debug", str) == 0)
 apic_verbosity = APIC_DEBUG;
 else if (strcmp("verbose", str) == 0)
 apic_verbosity = APIC_VERBOSE;
-else
+else {
 printk(KERN_WARNING "APIC Verbosity level %s not recognised"
" use apic_verbosity=verbose or apic_verbosity=debug", str);
+return -EINVAL;
+}
+
+return 0;
 }
 custom_param("apic_verbosity", apic_set_verbosity);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 18/52] xen/arch/x86/psr.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/psr.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
---
V3:
- let parse_psr_bool() return bool value (Jan Beulich)
- return error in case no string matches (Jan Beulich)
- dont modify option value in parsing function
---
 xen/arch/x86/psr.c | 57 ++
 1 file changed, 36 insertions(+), 21 deletions(-)

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index c2036cbed4..fe4f59b651 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -418,50 +418,65 @@ static const struct feat_props l2_cat_props = {
 .write_msr = l2_cat_write_msr,
 };
 
-static void __init parse_psr_bool(char *s, char *value, char *feature,
-  unsigned int mask)
+static bool __init parse_psr_bool(const char *s, const char *value,
+  const char *feature, unsigned int mask)
 {
-if ( !strcmp(s, feature) )
+if ( !strncmp(s, feature, value - s) )
 {
-if ( !value )
+if ( !*value )
 opt_psr |= mask;
 else
 {
-int val_int = parse_bool(value);
+int val_int = parse_bool(value + 1);
 
 if ( val_int == 0 )
 opt_psr &= ~mask;
 else if ( val_int == 1 )
 opt_psr |= mask;
+else
+return false;
 }
+return true;
 }
+return false;
 }
 
-static void __init parse_psr_param(char *s)
+static int __init parse_psr_param(const char *s)
 {
-char *ss, *val_str;
+const char *ss, *val_str;
+const char *q;
+int rc = 0;
 
 do {
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
 val_str = strchr(s, ':');
-if ( val_str )
-*val_str++ = '\0';
-
-parse_psr_bool(s, val_str, "cmt", PSR_CMT);
-parse_psr_bool(s, val_str, "cat", PSR_CAT);
-parse_psr_bool(s, val_str, "cdp", PSR_CDP);
+if ( !val_str )
+val_str = strchr(s, '\0');
 
-if ( val_str && !strcmp(s, "rmid_max") )
-opt_rmid_max = simple_strtoul(val_str, NULL, 0);
-
-if ( val_str && !strcmp(s, "cos_max") )
-opt_cos_max = simple_strtoul(val_str, NULL, 0);
+if ( *val_str && !strncmp(s, "rmid_max", val_str - s) )
+{
+opt_rmid_max = simple_strtoul(val_str + 1, &q, 0);
+if ( *q && *q != ',' )
+rc = -EINVAL;
+}
+else if ( *val_str && !strncmp(s, "cos_max", val_str - s) )
+{
+opt_cos_max = simple_strtoul(val_str + 1, &q, 0);
+if ( *q && *q != ',' )
+rc = -EINVAL;
+}
+else if ( !parse_psr_bool(s, val_str, "cmt", PSR_CMT) &&
+  !parse_psr_bool(s, val_str, "cat", PSR_CAT) &&
+  !parse_psr_bool(s, val_str, "cdp", PSR_CDP) )
+rc = -EINVAL;
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 custom_param("psr", parse_psr_param);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 51/52] xl: add new xl command set-parameters

[Juergen Gross]

Add a new xl command "set-parameters" to set hypervisor parameters at
runtime similar to boot time parameters via command line.

Cc: Ian Jackson 
Cc: Wei Liu 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
---
 docs/man/xl.pod.1.in   |  5 +
 tools/xl/xl.h  |  1 +
 tools/xl/xl_cmdtable.c |  5 +
 tools/xl/xl_misc.c | 20 
 4 files changed, 31 insertions(+)

diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
index 16c83066fe..3d5f2f7359 100644
--- a/docs/man/xl.pod.1.in
+++ b/docs/man/xl.pod.1.in
@@ -822,6 +822,11 @@ Pass the VNC password to vncviewer via stdin.
 Send debug I to Xen. It is the same as pressing the Xen
 "conswitch" (Ctrl-A by default) three times and then pressing "keys".
 
+=item B I
+
+Set hypervisor parameters as specified in I. This allows for some
+boot parameters of the hypervisor to be modified in the running systems.
+
 =item B [I]
 
 Reads the Xen message buffer, similar to dmesg on a Linux system.  The
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index aa95b77146..5d3d2a4835 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -154,6 +154,7 @@ int main_rename(int argc, char **argv);
 int main_trigger(int argc, char **argv);
 int main_sysrq(int argc, char **argv);
 int main_debug_keys(int argc, char **argv);
+int main_set_parameters(int argc, char **argv);
 int main_dmesg(int argc, char **argv);
 int main_top(int argc, char **argv);
 int main_networkattach(int argc, char **argv);
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 2c71a9f776..ba0159df67 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -309,6 +309,11 @@ struct cmd_spec cmd_table[] = {
   "Send debug keys to Xen",
   "",
 },
+{ "set-parameters",
+  &main_set_parameters, 0, 1,
+  "Set hypervisor parameters",
+  "",
+},
 { "dmesg",
   &main_dmesg, 0, 0,
   "Read and/or clear dmesg buffer",
diff --git a/tools/xl/xl_misc.c b/tools/xl/xl_misc.c
index 9c6227af23..dcf940a6d4 100644
--- a/tools/xl/xl_misc.c
+++ b/tools/xl/xl_misc.c
@@ -155,6 +155,26 @@ int main_debug_keys(int argc, char **argv)
 return EXIT_SUCCESS;
 }
 
+int main_set_parameters(int argc, char **argv)
+{
+int opt;
+char *params;
+
+SWITCH_FOREACH_OPT(opt, "", NULL, "set-parameters", 1) {
+/* No options */
+}
+
+params = argv[optind];
+
+if (libxl_set_parameters(ctx, params)) {
+fprintf(stderr, "cannot set parameters: %s\n", params);
+fprintf(stderr, "Use \"xl dmesg\" to look for possible reason.\n");
+return EXIT_FAILURE;
+}
+
+return EXIT_SUCCESS;
+}
+
 int main_devd(int argc, char **argv)
 {
 int ret = 0, opt = 0, daemonize = 1;
-- 
2.12.3


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

[Xen-devel] [PATCH v3 06/52] xen/arch/x86/cpu/vpmu.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/cpu/vpmu.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/arch/x86/cpu/vpmu.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 90954ca884..e0c87a68d4 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -53,7 +53,7 @@ CHECK_pmu_params;
 static unsigned int __read_mostly opt_vpmu_enabled;
 unsigned int __read_mostly vpmu_mode = XENPMU_MODE_OFF;
 unsigned int __read_mostly vpmu_features = 0;
-static void parse_vpmu_params(char *s);
+static int parse_vpmu_params(const char *s);
 custom_param("vpmu", parse_vpmu_params);
 
 static DEFINE_SPINLOCK(vpmu_lock);
@@ -61,7 +61,7 @@ static unsigned vpmu_count;
 
 static DEFINE_PER_CPU(struct vcpu *, last_vcpu);
 
-static int parse_vpmu_param(char *s, unsigned int len)
+static int parse_vpmu_param(const char *s, unsigned int len)
 {
 if ( !*s || !len )
 return 0;
@@ -76,9 +76,9 @@ static int parse_vpmu_param(char *s, unsigned int len)
 return 0;
 }
 
-static void __init parse_vpmu_params(char *s)
+static int __init parse_vpmu_params(const char *s)
 {
-char *sep, *p = s;
+const char *sep, *p = s;
 
 switch ( parse_bool(s) )
 {
@@ -104,10 +104,11 @@ static void __init parse_vpmu_params(char *s)
 opt_vpmu_enabled = 1;
 break;
 }
-return;
+return 0;
 
  error:
 printk("VPMU: unknown flags: %s - vpmu disabled!\n", s);
+return -EINVAL;
 }
 
 void vpmu_lvtpc_update(uint32_t val)
-- 
2.12.3


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

[Xen-devel] [PATCH v3 05/52] xen/arch/x86/cpu/mcheck/mce.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/cpu/mcheck/mce.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/arch/x86/cpu/mcheck/mce.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 30525dd78b..8015dff8c4 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -62,13 +62,18 @@ struct mca_banks *mca_allbanks;
 #endif
 
 int mce_verbosity;
-static void __init mce_set_verbosity(char *str)
+static int __init mce_set_verbosity(const char *str)
 {
 if (strcmp("verbose", str) == 0)
 mce_verbosity = MCE_VERBOSE;
 else
+{
 printk(KERN_DEBUG "Machine Check verbosity level %s not recognised"
"use mce_verbosity=verbose", str);
+return -EINVAL;
+}
+
+return 0;
 }
 custom_param("mce_verbosity", mce_set_verbosity);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 43/52] xen/arch/x86/io_apic.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by setup_ioapic_ack() can be removed.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Jan Beulich 
---
 xen/arch/x86/io_apic.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index f767c4560c..f959090ca0 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -1594,10 +1594,7 @@ static int __init setup_ioapic_ack(const char *s)
 ioapic_ack_forced = true;
 }
 else
-{
-printk("Unknown ioapic_ack value specified: '%s'\n", s);
 return -EINVAL;
-}
 
 return 0;
 }
-- 
2.12.3


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

[Xen-devel] [PATCH v3 48/52] xen: add hypercall for setting parameters at runtime

[Juergen Gross]

Add a sysctl hypercall to support setting parameters similar to
command line parameters, but at runtime. The parameters to set are
specified as a string, just like the boot parameters.

Cc: Daniel De Graaf 
Cc: Ian Jackson 
Cc: Wei Liu 
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Signed-off-by: Juergen Gross 
---
V2:
- corrected XSM test (Daniel De Graaf)

V3:
- check pad[] to be zero (Jan Beulich)
- return E2BIG in case of parameters too long (Jan Beulich)
- move max. parameter size define to sysctl.c (Jan Beulich)
---
 tools/flask/policy/modules/dom0.te  |  2 +-
 xen/common/sysctl.c | 36 
 xen/include/public/sysctl.h | 17 +
 xen/xsm/flask/hooks.c   |  3 +++
 xen/xsm/flask/policy/access_vectors |  2 ++
 5 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/tools/flask/policy/modules/dom0.te 
b/tools/flask/policy/modules/dom0.te
index d0a4d91ac0..338caaf41e 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -16,7 +16,7 @@ allow dom0_t xen_t:xen {
 allow dom0_t xen_t:xen2 {
resource_op psr_cmt_op psr_cat_op pmu_ctrl get_symbol
get_cpu_levelling_caps get_cpu_featureset livepatch_op
-   gcov_op
+   gcov_op set_parameter
 };
 
 # Allow dom0 to use all XENVER_ subops that have checks.
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index ae58a0f650..d08580f492 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -467,6 +467,42 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
u_sysctl)
 copyback = 1;
 break;
 
+case XEN_SYSCTL_set_parameter:
+{
+#define XEN_SET_PARAMETER_MAX_SIZE 1023
+char *params;
+
+if ( op->u.set_parameter.pad[0] || op->u.set_parameter.pad[1] ||
+ op->u.set_parameter.pad[2] )
+{
+ret = -EINVAL;
+break;
+}
+if ( op->u.set_parameter.size > XEN_SET_PARAMETER_MAX_SIZE )
+{
+ret = -E2BIG;
+break;
+}
+params = xmalloc_bytes(op->u.set_parameter.size + 1);
+if ( !params )
+{
+ret = -ENOMEM;
+break;
+}
+if ( __copy_from_guest(params, op->u.set_parameter.params,
+   op->u.set_parameter.size) )
+ret = -EFAULT;
+else
+{
+params[op->u.set_parameter.size] = 0;
+ret = runtime_parse(params);
+}
+
+xfree(params);
+
+break;
+}
+
 default:
 ret = arch_do_sysctl(op, u_sysctl);
 copyback = 0;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 9e51af61e1..29ef08efb1 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1096,6 +1096,21 @@ struct xen_sysctl_livepatch_op {
 typedef struct xen_sysctl_livepatch_op xen_sysctl_livepatch_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_livepatch_op_t);
 
+/*
+ * XEN_SYSCTL_set_parameter
+ *
+ * Change hypervisor parameters at runtime.
+ * The input string is parsed similar to the boot parameters.
+ */
+
+struct xen_sysctl_set_parameter {
+XEN_GUEST_HANDLE_64(char) params;   /* IN: pointer to parameters. */
+uint16_t size;  /* IN: size of parameters. */
+uint16_t pad[3];/* IN: MUST be zero. */
+};
+typedef struct xen_sysctl_set_parameter xen_sysctl_set_parameter_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_set_parameter_t);
+
 struct xen_sysctl {
 uint32_t cmd;
 #define XEN_SYSCTL_readconsole1
@@ -1124,6 +1139,7 @@ struct xen_sysctl {
 #define XEN_SYSCTL_get_cpu_levelling_caps25
 #define XEN_SYSCTL_get_cpu_featureset26
 #define XEN_SYSCTL_livepatch_op  27
+#define XEN_SYSCTL_set_parameter 28
 uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
 union {
 struct xen_sysctl_readconsole   readconsole;
@@ -1152,6 +1168,7 @@ struct xen_sysctl {
 struct xen_sysctl_cpu_levelling_caps cpu_levelling_caps;
 struct xen_sysctl_cpu_featuresetcpu_featureset;
 struct xen_sysctl_livepatch_op  livepatch;
+struct xen_sysctl_set_parameter set_parameter;
 uint8_t pad[128];
 } u;
 };
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 91146275bb..ad6e458822 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -827,6 +827,9 @@ static int flask_sysctl(int cmd)
 case XEN_SYSCTL_gcov_op:
 return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
 XEN2__GCOV_OP, NULL);
+case XEN_SYSCTL_set_parameter:
+return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
+XEN2__SET_PARAMETER, NULL);
 
 default:

[Xen-devel] [PATCH v3 31/52] xen/drivers/cpufreq/cpufreq.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/cpufreq/cpufreq.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Signed-off-by: Juergen Gross 
---
V3:
- dont modify option value in handling function
- remove prototype of cpufreq_cmdline_parse() from cpufreq.h and make
  it static
---
 xen/drivers/cpufreq/cpufreq.c  | 37 +
 xen/include/acpi/cpufreq/cpufreq.h |  2 --
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
index fd82ef5dce..216ad32350 100644
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -62,37 +62,41 @@ LIST_HEAD_READ_MOSTLY(cpufreq_governor_list);
 /* set xen as default cpufreq */
 enum cpufreq_controller cpufreq_controller = FREQCTL_xen;
 
-static void __init setup_cpufreq_option(char *str)
+static int __init cpufreq_cmdline_parse(const char *s);
+
+static int __init setup_cpufreq_option(const char *str)
 {
-char *arg = strpbrk(str, ",:");
+const char *arg = strpbrk(str, ",:");
 int choice;
 
-if ( arg )
-*arg++ = '\0';
+if ( !arg )
+arg = strchr(str, '\0');
 choice = parse_bool(str);
 
-if ( choice < 0 && !strcmp(str, "dom0-kernel") )
+if ( choice < 0 && !strncmp(str, "dom0-kernel", arg - str) )
 {
 xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
 cpufreq_controller = FREQCTL_dom0_kernel;
 opt_dom0_vcpus_pin = 1;
-return;
+return 0;
 }
 
-if ( choice == 0 || !strcmp(str, "none") )
+if ( choice == 0 || !strncmp(str, "none", arg - str) )
 {
 xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
 cpufreq_controller = FREQCTL_none;
-return;
+return 0;
 }
 
-if ( choice > 0 || !strcmp(str, "xen") )
+if ( choice > 0 || !strncmp(str, "xen", arg - str) )
 {
 xen_processor_pmbits |= XEN_PROCESSOR_PM_PX;
 cpufreq_controller = FREQCTL_xen;
-if ( arg && *arg )
-cpufreq_cmdline_parse(arg);
+if ( *arg && *(arg + 1) )
+return cpufreq_cmdline_parse(arg + 1);
 }
+
+return (choice < 0) ? -EINVAL : 0;
 }
 custom_param("cpufreq", setup_cpufreq_option);
 
@@ -571,7 +575,7 @@ static int __init cpufreq_handle_common_option(const char 
*name, const char *val
 return 0;
 }
 
-void __init cpufreq_cmdline_parse(char *str)
+static int __init cpufreq_cmdline_parse(const char *s)
 {
 static struct cpufreq_governor *__initdata cpufreq_governors[] =
 {
@@ -581,8 +585,12 @@ void __init cpufreq_cmdline_parse(char *str)
 &cpufreq_gov_performance,
 &cpufreq_gov_powersave
 };
+static char buf[128];
+char *str = buf;
 unsigned int gov_index = 0;
+int rc = 0;
 
+strlcpy(buf, s, sizeof(buf));
 do {
 char *val, *end = strchr(str, ',');
 unsigned int i;
@@ -611,11 +619,16 @@ void __init cpufreq_cmdline_parse(char *str)
 if (str && !cpufreq_handle_common_option(str, val) &&
 (!cpufreq_governors[gov_index]->handle_option ||
  !cpufreq_governors[gov_index]->handle_option(str, val)))
+{
 printk(XENLOG_WARNING "cpufreq/%s: option '%s' not recognized\n",
cpufreq_governors[gov_index]->name, str);
+rc = -EINVAL;
+}
 
 str = end;
 } while (str);
+
+return rc;
 }
 
 static int cpu_callback(
diff --git a/xen/include/acpi/cpufreq/cpufreq.h 
b/xen/include/acpi/cpufreq/cpufreq.h
index 48ad1d0004..a5cd7d08a1 100644
--- a/xen/include/acpi/cpufreq/cpufreq.h
+++ b/xen/include/acpi/cpufreq/cpufreq.h
@@ -79,8 +79,6 @@ DECLARE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_policy);
 extern int __cpufreq_set_policy(struct cpufreq_policy *data,
 struct cpufreq_policy *policy);
 
-void cpufreq_cmdline_parse(char *);
-
 #define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
 #define CPUFREQ_SHARED_TYPE_HW   (1) /* HW does needed coordination */
 #define CPUFREQ_SHARED_TYPE_ALL  (2) /* All dependent CPUs should set freq */
-- 
2.12.3


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

[Xen-devel] [PATCH v3 37/52] xen/drivers/video/vesa.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/video/vesa.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/drivers/video/vesa.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c
index 09d344c021..c92497e0bc 100644
--- a/xen/drivers/video/vesa.c
+++ b/xen/drivers/video/vesa.c
@@ -29,12 +29,14 @@ static unsigned int vram_remap;
 integer_param("vesa-map", vram_remap);
 
 static int font_height;
-static void __init parse_font_height(const char *s)
+static int __init parse_font_height(const char *s)
 {
 if ( simple_strtoul(s, &s, 10) == 8 && (*s++ == 'x') )
 font_height = simple_strtoul(s, &s, 10);
 if ( *s != '\0' )
 font_height = 0;
+
+return 0;
 }
 custom_param("font", parse_font_height);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 23/52] xen/common/core_parking.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/core_parking.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
 xen/common/core_parking.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/common/core_parking.c b/xen/common/core_parking.c
index de269e06c2..803c6c7911 100644
--- a/xen/common/core_parking.c
+++ b/xen/common/core_parking.c
@@ -41,14 +41,16 @@ static enum core_parking_controller {
 PERFORMANCE_FIRST
 } core_parking_controller = POWER_FIRST;
 
-static void __init setup_core_parking_option(char *str)
+static int __init setup_core_parking_option(const char *str)
 {
 if ( !strcmp(str, "power") )
 core_parking_controller = POWER_FIRST;
 else if ( !strcmp(str, "performance") )
 core_parking_controller = PERFORMANCE_FIRST;
 else
-return;
+return -EINVAL;
+
+return 0;
 }
 custom_param("core_parking", setup_core_parking_option);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 27/52] xen/common/memory.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/memory.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Jan Beulich 
---
V3:
- cosmetic changes (Jan Beulich)
---
 xen/common/memory.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index b2066db07e..26da6050f6 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -55,7 +55,8 @@ static unsigned int __read_mostly hwdom_max_order = 
CONFIG_HWDOM_MAX_ORDER;
 #ifdef HAS_PASSTHROUGH
 static unsigned int __read_mostly ptdom_max_order = CONFIG_PTDOM_MAX_ORDER;
 #endif
-static void __init parse_max_order(const char *s)
+
+static int __init parse_max_order(const char *s)
 {
 if ( *s != ',' )
 domu_max_order = simple_strtoul(s, &s, 0);
@@ -67,6 +68,8 @@ static void __init parse_max_order(const char *s)
 if ( *s == ',' && *++s != ',' )
 ptdom_max_order = simple_strtoul(s, &s, 0);
 #endif
+
+return *s ? -EINVAL : 0;
 }
 custom_param("memop-max-order", parse_max_order);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 08/52] xen/arch/x86/genapic/probe.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/genapic/probe.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
---
V3:
- dont return out of loop (Jan Beulich)
---
 xen/arch/x86/genapic/probe.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/genapic/probe.c b/xen/arch/x86/genapic/probe.c
index 9a147ff64a..af3745aa21 100644
--- a/xen/arch/x86/genapic/probe.c
+++ b/xen/arch/x86/genapic/probe.c
@@ -44,12 +44,17 @@ void __init generic_bigsmp_probe(void)
}
 }
 
-static void __init genapic_apic_force(char *str)
+static int __init genapic_apic_force(const char *str)
 {
-   int i;
+   int i, rc = -EINVAL;
+
for (i = 0; apic_probe[i]; i++)
-   if (!strcmp(apic_probe[i]->name, str))
+   if (!strcmp(apic_probe[i]->name, str)) {
genapic = apic_probe[i];
+   rc = 0;
+   }
+
+   return rc;
 }
 custom_param("apic", genapic_apic_force);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 20/52] xen/arch/x86/shutdown.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/shutdown.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
---
V3:
- dont stop loop at first invalid character (Jan Beulich)
---
 xen/arch/x86/shutdown.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index f63b8a668f..862384a8eb 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -51,8 +51,11 @@ static int reboot_mode;
  * efiUse the EFI reboot (if running under EFI)
  */
 static enum reboot_type reboot_type = BOOT_INVALID;
-static void __init set_reboot_type(char *str)
+
+static int __init set_reboot_type(const char *str)
 {
+int rc = 0;
+
 for ( ; ; )
 {
 switch ( *str )
@@ -74,6 +77,8 @@ static void __init set_reboot_type(char *str)
 case 't':
 reboot_type = *str;
 break;
+default:
+rc = -EINVAL;
 }
 if ( (str = strchr(str, ',')) == NULL )
 break;
@@ -82,6 +87,8 @@ static void __init set_reboot_type(char *str)
 
 if ( reboot_type == BOOT_EFI && !efi_enabled(EFI_RS) )
 reboot_type = BOOT_INVALID;
+
+return rc;
 }
 custom_param("reboot", set_reboot_type);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 03/52] xen/arch/arm/traps.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/arm/traps.c

to indicate whether the parameter value was parsed successfully.

Cc: Stefano Stabellini 
Cc: Julien Grall 
Signed-off-by: Juergen Gross 
Acked-by: Wei Liu 
Acked-by: Julien Grall 
---
 xen/arch/arm/traps.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c07999b518..f94ca1af02 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -108,12 +108,14 @@ static enum {
NATIVE,
 } vwfi;
 
-static void __init parse_vwfi(const char *s)
+static int __init parse_vwfi(const char *s)
 {
if ( !strcmp(s, "native") )
vwfi = NATIVE;
else
vwfi = TRAP;
+
+   return 0;
 }
 custom_param("vwfi", parse_vwfi);
 
@@ -130,7 +132,7 @@ static enum {
 SERRORS_PANIC,
 } serrors_op;
 
-static void __init parse_serrors_behavior(const char *str)
+static int __init parse_serrors_behavior(const char *str)
 {
 if ( !strcmp(str, "forward") )
 serrors_op = SERRORS_FORWARD;
@@ -139,7 +141,7 @@ static void __init parse_serrors_behavior(const char *str)
 else
 serrors_op = SERRORS_DIVERSE;
 
-return;
+return 0;
 }
 custom_param("serrors", parse_serrors_behavior);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 52/52] xen: make some console related parameters settable at runtime

[Juergen Gross]

Support modifying conswitch, console_timestamps, loglvl and
guest_loglvl at runtime.

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Signed-off-by: Juergen Gross 
---
V3:
- *_runtime_param() now includes boot parameter setting (Jan Beulich)
---
 docs/misc/xen-command-line.markdown |  8 
 xen/drivers/char/console.c  | 18 +-
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index 4002eab08b..9797c8db2d 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -391,6 +391,8 @@ makes sense on its own.
 
 > Default: `none`
 
+> Can be modified at runtime
+
 Specify which timestamp format Xen should use for each console line.
 
 * `none`: No timestamps
@@ -417,6 +419,8 @@ into the console ring buffer.
 
 > Default: `conswitch=a`
 
+> Can be modified at runtime
+
 Specify which character should be used to switch serial input between
 Xen and dom0.  The required sequence is CTRL- three
 times.
@@ -898,6 +902,8 @@ maximum number of maptrack frames domain.
 
 > Default: `guest_loglvl=none/warning`
 
+> Can be modified at runtime
+
 Set the logging level for Xen guests.  Any log message with equal more
 more importance will be printed.
 
@@ -1164,6 +1170,8 @@ if left disabled by the BIOS.
 
 > Default: `loglvl=warning`
 
+> Can be modified at runtime
+
 Set the logging level for Xen.  Any log message with equal more more
 importance will be printed.
 
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 17ad02083d..068e2f617c 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -40,7 +40,7 @@ string_param("console", opt_console);
 /* Char 2: If this character is 'x', then do not auto-switch to DOM0 when it */
 /* boots. Any other value, or omitting the char, enables auto-switch */
 static unsigned char __read_mostly opt_conswitch[3] = "a";
-string_param("conswitch", opt_conswitch);
+string_runtime_param("conswitch", opt_conswitch);
 
 /* sync_console: force synchronous console output (useful for debugging). */
 static bool_t __initdata opt_sync_console;
@@ -68,7 +68,7 @@ enum con_timestamp_mode
 static enum con_timestamp_mode __read_mostly opt_con_timestamp_mode = TSM_NONE;
 
 static int parse_console_timestamps(const char *s);
-custom_param("console_timestamps", parse_console_timestamps);
+custom_runtime_param("console_timestamps", parse_console_timestamps);
 
 /* conring_size: allows a large console ring than default (16kB). */
 static uint32_t __initdata opt_conring_size;
@@ -134,8 +134,8 @@ static int parse_guest_loglvl(const char *s);
  * Similar definitions for guest_loglvl, but applies to guest tracing.
  * Defaults: loglvl=warning ; guest_loglvl=none/warning
  */
-custom_param("loglvl", parse_loglvl);
-custom_param("guest_loglvl", parse_guest_loglvl);
+custom_runtime_param("loglvl", parse_loglvl);
+custom_runtime_param("guest_loglvl", parse_guest_loglvl);
 
 static atomic_t print_everything = ATOMIC_INIT(0);
 
@@ -145,7 +145,7 @@ static atomic_t print_everything = ATOMIC_INIT(0);
 return (lvlnum);\
 }
 
-static int __init __parse_loglvl(const char *s, const char **ps)
+static int __parse_loglvl(const char *s, const char **ps)
 {
 ___parse_loglvl(s, ps, "none",0);
 ___parse_loglvl(s, ps, "error",   1);
@@ -156,7 +156,7 @@ static int __init __parse_loglvl(const char *s, const char 
**ps)
 return 2; /* sane fallback */
 }
 
-static int __init _parse_loglvl(const char *s, int *lower, int *upper)
+static int _parse_loglvl(const char *s, int *lower, int *upper)
 {
 *lower = *upper = __parse_loglvl(s, &s);
 if ( *s == '/' )
@@ -167,12 +167,12 @@ static int __init _parse_loglvl(const char *s, int 
*lower, int *upper)
 return *s ? -EINVAL : 0;
 }
 
-static int __init parse_loglvl(const char *s)
+static int parse_loglvl(const char *s)
 {
 return _parse_loglvl(s, &xenlog_lower_thresh, &xenlog_upper_thresh);
 }
 
-static int __init parse_guest_loglvl(const char *s)
+static int parse_guest_loglvl(const char *s)
 {
 return _parse_loglvl(s, &xenlog_guest_lower_thresh,
  &xenlog_guest_upper_thresh);
@@ -606,7 +606,7 @@ static int printk_prefix_check(char *p, char **pp)
 ((loglvl < upper_thresh) && printk_ratelimit()));
 } 
 
-static int __init parse_console_timestamps(const char *s)
+static int parse_console_timestamps(const char *s)
 {
 switch ( parse_bool(s) )
 {
-- 
2.12.3


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

[Xen-devel] [PATCH v3 09/52] xen/arch/x86/hvm/viridian.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/hvm/viridian.c

to indicate whether the parameter value was parsed successfully.

Fix an error in the parsing function: up to now it would overwrite the
stack in case more than 3 values are specified.

Cc: Paul Durrant 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
---
V3:
- dont modify option value in parsing function
- fix error in parsing routine
---
 xen/arch/x86/hvm/viridian.c | 28 ++--
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index aa9b87c0ab..2edf9d0b23 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -1083,7 +1083,7 @@ static int viridian_load_vcpu_ctxt(struct domain *d, 
hvm_domain_context_t *h)
 HVM_REGISTER_SAVE_RESTORE(VIRIDIAN_VCPU, viridian_save_vcpu_ctxt,
   viridian_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
 
-static void __init parse_viridian_version(char *arg)
+static int __init parse_viridian_version(const char *arg)
 {
 const char *t;
 unsigned int n[3];
@@ -1093,17 +1093,24 @@ static void __init parse_viridian_version(char *arg)
 n[1] = viridian_minor;
 n[2] = viridian_build;
 
-while ( (t = strsep(&arg, ",")) != NULL )
-{
+do {
 const char *e;
 
-if ( *t == '\0' )
-continue;
+t = strchr(arg, ',');
+if ( !t )
+t = strchr(arg, '\0');
+
+if ( *arg && *arg != ',' && i < 3 )
+{
+n[i] = simple_strtoul(arg, &e, 0);
+if ( e != t )
+goto fail;
+}
+
+i++;
+arg = t + 1;
+} while ( *t );
 
-n[i++] = simple_strtoul(t, &e, 0);
-if ( *e != '\0' )
-goto fail;
-}
 if ( i != 3 )
 goto fail;
 
@@ -1118,10 +1125,11 @@ static void __init parse_viridian_version(char *arg)
 
 printk("viridian-version = %#x,%#x,%#x\n",
viridian_major, viridian_minor, viridian_build);
-return;
+return 0;
 
  fail:
 printk(XENLOG_WARNING "Invalid viridian-version, using default\n");
+return -EINVAL;
 }
 custom_param("viridian-version", parse_viridian_version);
 
-- 
2.12.3


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

[Xen-devel] [PATCH v3 39/52] xen: check parameter validity when parsing command line

[Juergen Gross]

Where possible check validity of parameters in _cmdline_parse() and
issue a warning message in case of an error detected.

In order to make sure a custom parameter parsing function really
returns a value (error or success), don't use a void pointer for
storing the function address, but a proper typed function pointer.

Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Signed-off-by: Juergen Gross 
---
V2:
- replaced literal 8 by BITS_PER_BYTE (Wei Liu)
- added test for empty string to parse_bool()

V3:
- use function pointer in struct kernel_param (Jan Beulich)
- better range check in assign_integer_param() (Jan Beulich)
- dont assign int values in case of overflow (Jan Beulich)
- allow multiple handlers for a parameter (Jan Beulich)
---
 xen/common/kernel.c | 67 +
 xen/include/xen/init.h  | 30 +-
 xen/include/xen/types.h |  3 +++
 3 files changed, 78 insertions(+), 22 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index ce7cb8adb5..c629ffa11c 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -23,34 +23,43 @@ enum system_state system_state = SYS_STATE_early_boot;
 xen_commandline_t saved_cmdline;
 static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
 
-static void __init assign_integer_param(
+static int __init assign_integer_param(
 const struct kernel_param *param, uint64_t val)
 {
 switch ( param->len )
 {
 case sizeof(uint8_t):
-*(uint8_t *)param->var = val;
+if ( val > UINT8_MAX && val < (uint64_t)INT8_MIN )
+return -EOVERFLOW;
+*(uint8_t *)param->par.var = val;
 break;
 case sizeof(uint16_t):
-*(uint16_t *)param->var = val;
+if ( val > UINT16_MAX && val < (uint64_t)INT16_MIN )
+return -EOVERFLOW;
+*(uint16_t *)param->par.var = val;
 break;
 case sizeof(uint32_t):
-*(uint32_t *)param->var = val;
+if ( val > UINT32_MAX && val < (uint64_t)INT32_MIN )
+return -EOVERFLOW;
+*(uint32_t *)param->par.var = val;
 break;
 case sizeof(uint64_t):
-*(uint64_t *)param->var = val;
+*(uint64_t *)param->par.var = val;
 break;
 default:
 BUG();
 }
+
+return 0;
 }
 
 static void __init _cmdline_parse(const char *cmdline)
 {
 char opt[128], *optval, *optkey, *q;
-const char *p = cmdline;
+const char *p = cmdline, *s, *key;
 const struct kernel_param *param;
-int bool_assert;
+int bool_assert, rctmp, rc;
+bool found;
 
 for ( ; ; )
 {
@@ -84,10 +93,13 @@ static void __init _cmdline_parse(const char *cmdline)
 }
 
 /* Boolean parameters can be inverted with 'no-' prefix. */
+key = optkey;
 bool_assert = !!strncmp("no-", optkey, 3);
 if ( !bool_assert )
 optkey += 3;
 
+rc = 0;
+found = false;
 for ( param = __setup_start; param < __setup_end; param++ )
 {
 if ( strcmp(param->name, optkey) )
@@ -96,34 +108,48 @@ static void __init _cmdline_parse(const char *cmdline)
  strlen(param->name) == q + 1 - opt &&
  !strncmp(param->name, opt, q + 1 - opt) )
 {
+found = true;
 optval[-1] = '=';
-((void (*)(const char *))param->var)(q);
+rctmp = param->par.func(q);
 optval[-1] = '\0';
+if ( !rc )
+rc = rctmp;
 }
 continue;
 }
 
+rctmp = 0;
+found = true;
 switch ( param->type )
 {
 case OPT_STR:
-strlcpy(param->var, optval, param->len);
+strlcpy(param->par.var, optval, param->len);
 break;
 case OPT_UINT:
-assign_integer_param(
+rctmp = assign_integer_param(
 param,
-simple_strtoll(optval, NULL, 0));
+simple_strtoll(optval, &s, 0));
+if ( *s )
+rctmp = -EINVAL;
 break;
 case OPT_BOOL:
-if ( !parse_bool(optval) )
+rctmp = parse_bool(optval);
+if ( rctmp < 0 )
+break;
+if ( !rctmp )
 bool_assert = !bool_assert;
+rctmp = 0;
 assign_integer_param(param, bool_assert);
 break;
 case OPT_SIZE:
-assign_integer_param(
+rctmp = assign_integer_param(
 param,
-parse_size_and_unit(optval, NULL));
+parse_size_and_unit(optval, &s));
+

[Xen-devel] [PATCH v3 41/52] xen/arch/x86/cpu/mcheck/mce.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by mce_set_verbosity() can be removed.

Cc: Jan Beulich 
Cc: Andrew Cooper 
Signed-off-by: Juergen Gross 
Acked-by: Jan Beulich 
---
 xen/arch/x86/cpu/mcheck/mce.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 8015dff8c4..7affe2591e 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -67,11 +67,7 @@ static int __init mce_set_verbosity(const char *str)
 if (strcmp("verbose", str) == 0)
 mce_verbosity = MCE_VERBOSE;
 else
-{
-printk(KERN_DEBUG "Machine Check verbosity level %s not recognised"
-   "use mce_verbosity=verbose", str);
 return -EINVAL;
-}
 
 return 0;
 }
-- 
2.12.3


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