Re: [Xen-devel] [PATCH 1/2] tools: remove systemd xenstore socket definitions

[Juergen Gross]

On 29/06/16 15:44, Juergen Gross wrote:
> On 29/06/16 15:31, Ross Lagerwall wrote:
>> On 06/29/2016 02:00 PM, Juergen Gross wrote:
>>> On 29/06/16 14:52, Andrew Cooper wrote:
 On 29/06/16 13:44, Juergen Gross wrote:
> @@ -2068,13 +1964,6 @@ int main(int argc, char *argv[])
>  /* Tell the kernel we're up and running. */
>  xenbus_notify_running();
>
> -#if defined(XEN_SYSTEMD_ENABLED)
> -if (systemd) {
> -sd_notify(1, "READY=1");
> -fprintf(stderr, SD_NOTICE "xenstored is ready\n");
> -}
> -#endif

 Getting rid of the socket configuration for systemd is ok, but we should
 keep the sd_notify() calls for when the daemon is started by systemd.

 Socket activiation and sd_notify() are orthogonal, and sd_notify() is
 still required if we don't want systemd to treat xenstored as a legacy
 unix daemon.
>>>
>>> So what is the downside of xenstored being treated as a legacy daemon?
>>> This question is especially interesting for the case of patch 2 being
>>> considered: xenstored is no longer started by systemd, but by a wrapper
>>> script which might decide to start the xenstore domain instead.
>>>
>>> Another problem: today xenstored decides whether to call sd_notify()
>>> by testing the xenstore sockets being specified via systemd. This will
>>> no longer work. So how to do it now?
>>>
>>>
>>
>> One problem with the patch as it currently is implemented is that the
>> service type is not correct for when xenstored is a daemon. This makes
>> it difficult to manage with systemd and difficult for other services to
>> depend on it in a sensible fashion. The end result is subtle races and
>> occasional failures.
> 
> Could you please educate me what's wrong? I'm no systemd expert.

Ross, you had some concerns I'm still not sure I understand.

Could you please answer my questions?

>> How about:
>> * Maintain the existing service for xenstored
>> * Have a separate service (with different a service type) for starting
>> the xenstore domain
>> * Switch between the two services
> 
> How could I specify e.g. in xendomains.service the dependency to
> xenstore?
> 
>> Personally I think it is better and more uniform for the administrator
>> to enable and disable services in the normal fashion, but if you want to
> 
> The two services would be mutually exclusive. Can I tell systemd this
> is the case?
> 
>> make it configurable with /etc/sysconfig/xencommons, then you can add
>> something like this to xenstored.service:
>>
>> ExecStartPre=/bin/grep -q XENSTORETYPE=daemon /etc/sysconfig/xencommons
>>
>> and to xenstore-domain.service:
>>
>> ExecStartPre=/bin/grep -q XENSTORETYPE=domain /etc/sysconfig/xencommons
> 
> That's no good idea. Someone commenting out the old line and adding the
> other option would end in both variants to be tried. This would have to
> be a little bit more sophisticated. :-)

Juergen


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

[Xen-devel] [distros-debian-squeeze test] 66561: tolerable trouble: blocked/broken

[Platform Team regression test user]

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 build-armhf-pvops 3 host-install(3)  broken like 66523
 build-armhf   3 host-install(3)  broken like 66523
 build-i3863 host-install(3)  broken like 66523
 build-i386-pvops  3 host-install(3)  broken like 66523
 build-amd64   3 host-install(3)  broken like 66523
 build-amd64-pvops 3 host-install(3)  broken like 66523

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

baseline version:
 flight   66523

jobs:
 build-amd64  broken  
 build-armhf  broken  
 build-i386   broken  
 build-amd64-pvopsbroken  
 build-armhf-pvopsbroken  
 build-i386-pvops broken  
 test-amd64-amd64-amd64-squeeze-netboot-pygrubblocked 
 test-amd64-i386-amd64-squeeze-netboot-pygrub blocked 
 test-amd64-amd64-i386-squeeze-netboot-pygrub blocked 
 test-amd64-i386-i386-squeeze-netboot-pygrub  blocked 



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

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

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


Push not applicable.


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

Re: [Xen-devel] [PATCH] xen/apic: Update the comment for apic_id_mask

[Wei, Jiangang]

On Thu, 2016-07-07 at 11:37 -0400, Boris Ostrovsky wrote:
> On 07/07/2016 11:25 AM, Konrad Rzeszutek Wilk wrote:
> > On Thu, Jul 07, 2016 at 11:28:18AM +0800, Wei Jiangang wrote:
> >> verify_local_APIC() had been removed by
> >> commit 4399c03c6780 ("x86/apic: Remove verify_local_APIC()"),
> >> so apic_id_mask isn't used by it.
> 
> Is anyone actually using this field? It looks like 4399c03c6780 removed
> the only user.

Indeed, the field is useless.
Maybe we can remove this field from the struct apic .
what's your opinion?

Thanks,
wei
> 
> -boris
> 
> 
> > CC-ing the proper maintainers.
> >> Signed-off-by: Wei Jiangang 
> >> ---
> >>  arch/x86/xen/apic.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/x86/xen/apic.c b/arch/x86/xen/apic.c
> >> index db52a7fafcc2..9cbb1f48381b 100644
> >> --- a/arch/x86/xen/apic.c
> >> +++ b/arch/x86/xen/apic.c
> >> @@ -177,7 +177,7 @@ static struct apic xen_pv_apic = {
> >>  
> >>.get_apic_id= xen_get_apic_id,
> >>.set_apic_id= xen_set_apic_id, /* Can be NULL on 
> >> 32-bit. */
> >> -  .apic_id_mask   = 0xFF << 24, /* Used by 
> >> verify_local_APIC. Match with what xen_get_apic_id does. */
> >> +  .apic_id_mask   = 0xFF << 24, /* Match with what 
> >> xen_get_apic_id does. */
> >>  
> >>.cpu_mask_to_apicid_and = flat_cpu_mask_to_apicid_and,
> >>  
> >> -- 
> >> 1.9.3
> >>
> >>
> >>
> >>
> >> ___
> >> Xen-devel mailing list
> >> Xen-devel@lists.xen.org
> >> https://lists.xen.org/xen-devel
> 
> 
> 
> 



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

Re: [Xen-devel] [PATCH v2 03/17] libxl/arm: Add a configuration option for ARM DomU ACPI

[Shannon Zhao]

On 2016/7/12 19:33, Wei Liu wrote:
> On Tue, Jul 12, 2016 at 10:22:39AM +0100, Julien Grall wrote:
> [...]
 Yeah, we can deprecate that field. But we need to take care to not break
 users of the old field.
>>> Ok, what name would you suggest?
>>
>> I would suggest b_info->u.acpi
>>
> 
> b_info->acpi would be more appropriate.
> 
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index ef614be..a57823d 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -494,11 +494,16 @@ libxl_domain_build_info = Struct("domain_build_info",[
>  # Note that the partial device tree should avoid to use the phandle
>  # 65000 which is reserved by the toolstack.
>  ("device_tree",  string),
> +("acpi", libxl_defbool),
>  ("u", KeyedUnion(None, libxl_domain_type, "type",
>  [("hvm", Struct(None, [("firmware", string),
> ("bios", libxl_bios_type),
> ("pae",  libxl_defbool),
> ("apic", libxl_defbool),
> +   # The following acpi field is
> +   # deprecated. Please use the unified
> +   # acpi field above which works for 
> both
> +   # x86 and ARM.
> ("acpi", libxl_defbool),
> ("acpi_s3",  libxl_defbool),
> ("acpi_s4",  libxl_defbool),
> 
> 
> And then:
> 
> 1. modify xl to set the new field.
> 2. modify libxl to handle compatibility: user of the old field should
>continue to work.
> 
I found that the default value of acpi is true on x86. But we decided
before it's should be false by default on ARM. How to deal with this?
Julien, Stefano, can we make acpi true by default?

Thanks,
-- 
Shannon


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

Re: [Xen-devel] How to find out how much cpu time each domain used?

[Dario Faggioli]

On Mon, 2016-07-04 at 20:09 +, Amin Fallahi wrote:
> Dear all
> 
> I am modifying credit scheduler and I want to give credit to each
> vcpu based on cpu time which its domain has consumed.
> Suppose:
> credit_fair =((credit_total*sdom->weight)+(weight_total-
> 1))/weight_total
> 
Mind describing what your actual end goal is, and spending a little
more word describing how you were thinking to achieve it?

I see the formula, but I don't understand:
 - what is it that you are trying to improve/achieve? Better fairness, 
   I guess?
 - how do you plan to use such formula, i.e., where in the algorithm 
   you'd put it? Credits are already been burned basing on how much a 
   vcpu executes, is this about how much credits a vcpu is given at 
   the beginning/reset? How would it integrate with credits_per_tslice 
   and friends?

> I want to multiply something to this formula according to the domain
> cpu usage. Thus I need to somehow find out cpu time for each domain.
>
Cpu usage in terms of what? Time? Percentage? Over what time interval?

It is certainly possible to figure out how long a vcpu executed on a
pcpu on a given time interval, but depending from the characteristics
of such time interval, the way to actually do that varies.

Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v7 03/14] xen: Use a typesafe to define INVALID_MFN

[Tim Deegan]

At 18:04 +0100 on 12 Jul (1468346680), George Dunlap wrote:
> On Tue, Jul 12, 2016 at 2:59 PM, Julien Grall  wrote:
> > Also take the opportunity to convert arch/x86/debug.c to the typesafe
> > mfn and use proper printf format for MFN/GFN when the code around is
> > modified.
> >
> > Signed-off-by: Julien Grall 
> > Reviewed-by: Andrew Cooper 
> > Acked-by: Stefano Stabellini 
> 
> What an excellent idea!
> 
> mm/ bits:
> 
> Reviewed-by: George Dunlap 
> 
> That R-B also includes the shadow code, so if Tim hasn't / doesn't
> expressed any objections, I think it doesn't necessarily have to wait
> for his ack (although I'll ping him out-of-band to check).

FAOD, this whole series Acked-by: Tim Deegan 

Cheers,

Tim.

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

Re: [Xen-devel] [PATCH v2] vmx/monitor: CPUID events

[Wei Liu]

On Tue, Jul 12, 2016 at 12:13:18PM -0600, Tamas K Lengyel wrote:
> This patch implements sending notification to a monitor subscriber when an
> x86/vmx guest executes the CPUID instruction.
> 
> Signed-off-by: Tamas K Lengyel 
> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 
> Cc: Razvan Cojocaru 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Jun Nakajima 
> Cc: Kevin Tian 
> 
> v2: add comment describing rc values in vmx
> fix typo in xen-access
> ---
>  tools/libxc/include/xenctrl.h   |  1 +
>  tools/libxc/xc_monitor.c| 13 +

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v4] xen/arm: Add a clock property

[Dirk Behme]

On 13.07.2016 00:26, Michael Turquette wrote:

Quoting Dirk Behme (2016-07-12 00:46:45)

Clocks described by this property are reserved for use by Xen, and the OS
must not alter their state any way, such as disabling or gating a clock,
or modifying its rate. Ensuring this may impose constraints on parent
clocks or other resources used by the clock tree.


Note that clk_prepare_enable will not prevent the rate from changing
(clk_set_rate) or a parent from changing (clk_set_parent). The only way
to do this currently would be to set the following flags on the effected
clocks:

CLK_SET_RATE_GATE
CLK_SET_PARENT_GATE




Regarding setting flags, I think we already talked about that. I think 
the conclusion was that in our case its not possible to manipulate the 
flags in the OS as this isn't intended to be done in cases like ours. 
Therefore no API is exported for this.


I.e. if we need to set these flags, we have to do that in Xen where we 
add the clocks to the hypervisor node in the device tree. And not in the 
kernel patch discussed here.


Best regards

Dirk



And then enable the clocks. All calls to clk_set_parent and clk_set_rate
with those clocks in the path will fail, so long as they are prepared
and enabled. This implementation detail is specific to Linux and
definitely should not go into the binding.



This property is used to proxy clocks for devices Xen has taken ownership
of, such as UARTs, for which the associated clock controller(s) remain
under the control of Dom0.


I'm still not completely sure about this type of layering and whether or
not it is sane. If you want Xen to manage the clock controller, AND you
want Linux guests or dom0 to consume those clocks and manipulate them in
other drivers, then this solution won't work.

Regards,
Mike



Up to now, the workaround for this has been to use the Linux kernel
command line parameter 'clk_ignore_unused'. See Xen bug

http://bugs.xenproject.org/xen/bug/45

too.

Signed-off-by: Dirk Behme 
---
Changes in v4: Switch to the xen.txt description proposed by Mark:
   https://www.spinics.net/lists/arm-kernel/msg516158.html

Changes in v3: Use the xen.txt description proposed by Michael. Thanks!

Changes in v2: Drop the Linux implementation details like clk_disable_unused
   in xen.txt.

 Documentation/devicetree/bindings/arm/xen.txt | 12 +++
 arch/arm/xen/enlighten.c  | 47 +++
 2 files changed, 59 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/xen.txt 
b/Documentation/devicetree/bindings/arm/xen.txt
index c9b9321..437e50b 100644
--- a/Documentation/devicetree/bindings/arm/xen.txt
+++ b/Documentation/devicetree/bindings/arm/xen.txt
@@ -17,6 +17,18 @@ the following properties:
   A GIC node is also required.
   This property is unnecessary when booting Dom0 using ACPI.

+Optional properties:
+
+- clocks: a list of phandle + clock-specifier pairs
+  Clocks described by this property are reserved for use by Xen, and the
+  OS must not alter their state any way, such as disabling or gating a
+  clock, or modifying its rate. Ensuring this may impose constraints on
+  parent clocks or other resources used by the clock tree.
+
+  Note: this property is used to proxy clocks for devices Xen has taken
+  ownership of, such as UARTs, for which the associated clock
+  controller(s) remain under the control of Dom0.
+
 To support UEFI on Xen ARM virtual platforms, Xen populates the FDT "uefi" node
 under /hypervisor with following parameters:

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 47acb36..5c546d0 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -444,6 +445,52 @@ static int __init xen_pm_init(void)
 }
 late_initcall(xen_pm_init);

+/*
+ * Check if we want to register some clocks, that they
+ * are not freed because unused by clk_disable_unused().
+ * E.g. the serial console clock.
+ */
+static int __init xen_arm_register_clks(void)
+{
+   struct clk *clk;
+   struct device_node *xen_node;
+   unsigned int i, count;
+   int ret = 0;
+
+   xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
+   if (!xen_node) {
+   pr_err("Xen support was detected before, but it has 
disappeared\n");
+   return -EINVAL;
+   }
+
+   count = of_clk_get_parent_count(xen_node);
+   if (!count)
+   goto out;
+
+   for (i = 0; i < count; i++) {
+   clk = of_clk_get(xen_node, i);
+   if (IS_ERR(clk)) {
+   pr_err("Xen failed to register clock %i. Error: %li\n",
+  i, PTR_ERR(clk));
+   ret = PTR_ERR(clk);
+   goto out;
+   }
+
+   ret = clk_prepare_enable(clk);
+   if (ret < 0) {
+   pr_err

[Xen-devel] [PATCH v6 00/46] dma-mapping: Use unsigned long for dma_attrs

[Krzysztof Kozlowski]

Hi,

The fifth version of this patchset was merged by Andrew Morton
few days ago.  It was rebased on v4.7-rc5 so it missed some ongoing
changes.

This is just rebase on next-20160713.


For easier testing the patchset is available here:
repo:   https://github.com/krzk/linux
branch: for-next/dma-attrs-const-v6


Changes since v5

1. New patches:
   1/46:  [media] mtk-vcodec: Remove unused dma_attrs
   44/46: remoteproc: qcom: Use unsigned long for dma_attrs
2. 19/46: rebased on next, some more changes inside
3. Added accumulated acks: Marek Szyprowski, Richard Kuo,
   Konrad Rzeszutek Wilk, Daniel Vetter and Joerg Roedel.


Changes since v4

1. Collect some acks. Still need more.
2. Minor fixes pointed by Robin Murphy.
3. Applied changes from Bart Van Assche's comment.
4. More tests and builds (using https://www.kernel.org/pub/tools/crosstool/).


Changes since v3

1. Collect some acks.
2. Drop wrong patch 1/45 ("powerpc: dma-mapping: Don't hard-code
   the value of DMA_ATTR_WEAK_ORDERING").
3. Minor fix pointed out by Michael Ellerman.


Changes since v2

1. Follow Christoph Hellwig's comments (don't use BIT add
   documentation, remove dma_get_attr).


Rationale
=
The dma-mapping core and the implementations do not change the
DMA attributes passed by pointer.  Thus the pointer can point to const
data.  However the attributes do not have to be a bitfield. Instead
unsigned long will do fine:

1. This is just simpler.  Both in terms of reading the code and setting
   attributes.  Instead of initializing local attributes on the stack
   and passing pointer to it to dma_set_attr(), just set the bits.

2. It brings safeness and checking for const correctness because the
   attributes are passed by value.


Best regards,
Krzysztof


Krzysztof Kozlowski (46):
  [media] mtk-vcodec: Remove unused dma_attrs
  dma-mapping: Use unsigned long for dma_attrs
  alpha: dma-mapping: Use unsigned long for dma_attrs
  arc: dma-mapping: Use unsigned long for dma_attrs
  ARM: dma-mapping: Use unsigned long for dma_attrs
  arm64: dma-mapping: Use unsigned long for dma_attrs
  avr32: dma-mapping: Use unsigned long for dma_attrs
  blackfin: dma-mapping: Use unsigned long for dma_attrs
  c6x: dma-mapping: Use unsigned long for dma_attrs
  cris: dma-mapping: Use unsigned long for dma_attrs
  frv: dma-mapping: Use unsigned long for dma_attrs
  drm/exynos: dma-mapping: Use unsigned long for dma_attrs
  drm/mediatek: dma-mapping: Use unsigned long for dma_attrs
  drm/msm: dma-mapping: Use unsigned long for dma_attrs
  drm/nouveau: dma-mapping: Use unsigned long for dma_attrs
  drm/rockship: dma-mapping: Use unsigned long for dma_attrs
  infiniband: dma-mapping: Use unsigned long for dma_attrs
  iommu: dma-mapping: Use unsigned long for dma_attrs
  [media] dma-mapping: Use unsigned long for dma_attrs
  xen: dma-mapping: Use unsigned long for dma_attrs
  swiotlb: dma-mapping: Use unsigned long for dma_attrs
  powerpc: dma-mapping: Use unsigned long for dma_attrs
  video: dma-mapping: Use unsigned long for dma_attrs
  x86: dma-mapping: Use unsigned long for dma_attrs
  iommu: intel: dma-mapping: Use unsigned long for dma_attrs
  h8300: dma-mapping: Use unsigned long for dma_attrs
  hexagon: dma-mapping: Use unsigned long for dma_attrs
  ia64: dma-mapping: Use unsigned long for dma_attrs
  m68k: dma-mapping: Use unsigned long for dma_attrs
  metag: dma-mapping: Use unsigned long for dma_attrs
  microblaze: dma-mapping: Use unsigned long for dma_attrs
  mips: dma-mapping: Use unsigned long for dma_attrs
  mn10300: dma-mapping: Use unsigned long for dma_attrs
  nios2: dma-mapping: Use unsigned long for dma_attrs
  openrisc: dma-mapping: Use unsigned long for dma_attrs
  parisc: dma-mapping: Use unsigned long for dma_attrs
  misc: mic: dma-mapping: Use unsigned long for dma_attrs
  s390: dma-mapping: Use unsigned long for dma_attrs
  sh: dma-mapping: Use unsigned long for dma_attrs
  sparc: dma-mapping: Use unsigned long for dma_attrs
  tile: dma-mapping: Use unsigned long for dma_attrs
  unicore32: dma-mapping: Use unsigned long for dma_attrs
  xtensa: dma-mapping: Use unsigned long for dma_attrs
  remoteproc: qcom: Use unsigned long for dma_attrs
  dma-mapping: Remove dma_get_attr
  dma-mapping: Document the DMA attributes next to the declaration

 Documentation/DMA-API.txt  |  33 +++---
 Documentation/DMA-attributes.txt   |   2 +-
 arch/alpha/include/asm/dma-mapping.h   |   2 -
 arch/alpha/kernel/pci-noop.c   |   2 +-
 arch/alpha/kernel/pci_iommu.c  |  12 +-
 arch/arc/mm/dma.c  |  12 +-
 arch/arm/common/dmabounce.c|   4 +-
 arch/arm/include/asm/dma-mapping.h |  13 +--
 arch/arm/include/asm/xen/page-coherent.h   |  16 +--
 arch/arm/mm/dma-mapping.c 

[Xen-devel] [PATCH v6 05/46] ARM: dma-mapping: Use unsigned long for dma_attrs

[Krzysztof Kozlowski]

Split out subsystem specific changes for easier reviews. This will be
squashed with main commit.

Signed-off-by: Krzysztof Kozlowski 
---
 arch/arm/common/dmabounce.c  |  4 +-
 arch/arm/include/asm/dma-mapping.h   | 13 +++--
 arch/arm/include/asm/xen/page-coherent.h | 16 +++
 arch/arm/mm/dma-mapping.c| 81 
 arch/arm/xen/mm.c|  4 +-
 5 files changed, 56 insertions(+), 62 deletions(-)

diff --git a/arch/arm/common/dmabounce.c b/arch/arm/common/dmabounce.c
index 1143c4d5c567..301281645d08 100644
--- a/arch/arm/common/dmabounce.c
+++ b/arch/arm/common/dmabounce.c
@@ -310,7 +310,7 @@ static inline void unmap_single(struct device *dev, struct 
safe_buffer *buf,
  */
 static dma_addr_t dmabounce_map_page(struct device *dev, struct page *page,
unsigned long offset, size_t size, enum dma_data_direction dir,
-   struct dma_attrs *attrs)
+   unsigned long attrs)
 {
dma_addr_t dma_addr;
int ret;
@@ -344,7 +344,7 @@ static dma_addr_t dmabounce_map_page(struct device *dev, 
struct page *page,
  * should be)
  */
 static void dmabounce_unmap_page(struct device *dev, dma_addr_t dma_addr, 
size_t size,
-   enum dma_data_direction dir, struct dma_attrs *attrs)
+   enum dma_data_direction dir, unsigned long attrs)
 {
struct safe_buffer *buf;
 
diff --git a/arch/arm/include/asm/dma-mapping.h 
b/arch/arm/include/asm/dma-mapping.h
index a83570f10124..d009f7911ffc 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -5,7 +5,6 @@
 
 #include 
 #include 
-#include 
 #include 
 
 #include 
@@ -174,7 +173,7 @@ static inline void dma_mark_clean(void *addr, size_t size) 
{ }
  * to be the device-viewed address.
  */
 extern void *arm_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
-  gfp_t gfp, struct dma_attrs *attrs);
+  gfp_t gfp, unsigned long attrs);
 
 /**
  * arm_dma_free - free memory allocated by arm_dma_alloc
@@ -191,7 +190,7 @@ extern void *arm_dma_alloc(struct device *dev, size_t size, 
dma_addr_t *handle,
  * during and after this call executing are illegal.
  */
 extern void arm_dma_free(struct device *dev, size_t size, void *cpu_addr,
-dma_addr_t handle, struct dma_attrs *attrs);
+dma_addr_t handle, unsigned long attrs);
 
 /**
  * arm_dma_mmap - map a coherent DMA allocation into user space
@@ -208,7 +207,7 @@ extern void arm_dma_free(struct device *dev, size_t size, 
void *cpu_addr,
  */
 extern int arm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
void *cpu_addr, dma_addr_t dma_addr, size_t size,
-   struct dma_attrs *attrs);
+   unsigned long attrs);
 
 /*
  * This can be called during early boot to increase the size of the atomic
@@ -262,16 +261,16 @@ extern void dmabounce_unregister_dev(struct device *);
  * The scatter list versions of the above methods.
  */
 extern int arm_dma_map_sg(struct device *, struct scatterlist *, int,
-   enum dma_data_direction, struct dma_attrs *attrs);
+   enum dma_data_direction, unsigned long attrs);
 extern void arm_dma_unmap_sg(struct device *, struct scatterlist *, int,
-   enum dma_data_direction, struct dma_attrs *attrs);
+   enum dma_data_direction, unsigned long attrs);
 extern void arm_dma_sync_sg_for_cpu(struct device *, struct scatterlist *, int,
enum dma_data_direction);
 extern void arm_dma_sync_sg_for_device(struct device *, struct scatterlist *, 
int,
enum dma_data_direction);
 extern int arm_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
void *cpu_addr, dma_addr_t dma_addr, size_t size,
-   struct dma_attrs *attrs);
+   unsigned long attrs);
 
 #endif /* __KERNEL__ */
 #endif
diff --git a/arch/arm/include/asm/xen/page-coherent.h 
b/arch/arm/include/asm/xen/page-coherent.h
index 9408a994cc91..95ce6ac3a971 100644
--- a/arch/arm/include/asm/xen/page-coherent.h
+++ b/arch/arm/include/asm/xen/page-coherent.h
@@ -2,15 +2,14 @@
 #define _ASM_ARM_XEN_PAGE_COHERENT_H
 
 #include 
-#include 
 #include 
 
 void __xen_dma_map_page(struct device *hwdev, struct page *page,
 dma_addr_t dev_addr, unsigned long offset, size_t size,
-enum dma_data_direction dir, struct dma_attrs *attrs);
+enum dma_data_direction dir, unsigned long attrs);
 void __xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
size_t size, enum dma_data_direction dir,
-   struct dma_attrs *attrs);
+   unsigned long attrs);
 void __xen_dma_sync_single_for_cpu(struct device *hwdev,
dma_addr_t handle, size_t size, enum dma_data_direction dir);
 
@@ -18,22 +17,20 @@ void __xen_dma_sync_single_

[Xen-devel] [PATCH v6 20/46] xen: dma-mapping: Use unsigned long for dma_attrs

[Krzysztof Kozlowski]

Split out subsystem specific changes for easier reviews. This will be
squashed with main commit.

Signed-off-by: Krzysztof Kozlowski 
[for xen]
Acked-by: David Vrabel 
[for xen swiotlb]
Acked-by: Konrad Rzeszutek Wilk 
---
 drivers/xen/swiotlb-xen.c | 14 +++---
 include/xen/swiotlb-xen.h | 12 ++--
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 7399782c0998..87e6035c9e81 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -294,7 +294,7 @@ error:
 void *
 xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
   dma_addr_t *dma_handle, gfp_t flags,
-  struct dma_attrs *attrs)
+  unsigned long attrs)
 {
void *ret;
int order = get_order(size);
@@ -346,7 +346,7 @@ EXPORT_SYMBOL_GPL(xen_swiotlb_alloc_coherent);
 
 void
 xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
- dma_addr_t dev_addr, struct dma_attrs *attrs)
+ dma_addr_t dev_addr, unsigned long attrs)
 {
int order = get_order(size);
phys_addr_t phys;
@@ -378,7 +378,7 @@ EXPORT_SYMBOL_GPL(xen_swiotlb_free_coherent);
 dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
unsigned long offset, size_t size,
enum dma_data_direction dir,
-   struct dma_attrs *attrs)
+   unsigned long attrs)
 {
phys_addr_t map, phys = page_to_phys(page) + offset;
dma_addr_t dev_addr = xen_phys_to_bus(phys);
@@ -434,7 +434,7 @@ EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);
  */
 static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,
 size_t size, enum dma_data_direction dir,
-struct dma_attrs *attrs)
+unsigned long attrs)
 {
phys_addr_t paddr = xen_bus_to_phys(dev_addr);
 
@@ -462,7 +462,7 @@ static void xen_unmap_single(struct device *hwdev, 
dma_addr_t dev_addr,
 
 void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
size_t size, enum dma_data_direction dir,
-   struct dma_attrs *attrs)
+   unsigned long attrs)
 {
xen_unmap_single(hwdev, dev_addr, size, dir, attrs);
 }
@@ -538,7 +538,7 @@ EXPORT_SYMBOL_GPL(xen_swiotlb_sync_single_for_device);
 int
 xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
 int nelems, enum dma_data_direction dir,
-struct dma_attrs *attrs)
+unsigned long attrs)
 {
struct scatterlist *sg;
int i;
@@ -599,7 +599,7 @@ EXPORT_SYMBOL_GPL(xen_swiotlb_map_sg_attrs);
 void
 xen_swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
   int nelems, enum dma_data_direction dir,
-  struct dma_attrs *attrs)
+  unsigned long attrs)
 {
struct scatterlist *sg;
int i;
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index 8b2eb93ae8ba..7c35e279d1e3 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -9,30 +9,30 @@ extern int xen_swiotlb_init(int verbose, bool early);
 extern void
 *xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
dma_addr_t *dma_handle, gfp_t flags,
-   struct dma_attrs *attrs);
+   unsigned long attrs);
 
 extern void
 xen_swiotlb_free_coherent(struct device *hwdev, size_t size,
  void *vaddr, dma_addr_t dma_handle,
- struct dma_attrs *attrs);
+ unsigned long attrs);
 
 extern dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
   unsigned long offset, size_t size,
   enum dma_data_direction dir,
-  struct dma_attrs *attrs);
+  unsigned long attrs);
 
 extern void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
   size_t size, enum dma_data_direction dir,
-  struct dma_attrs *attrs);
+  unsigned long attrs);
 extern int
 xen_swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
 int nelems, enum dma_data_direction dir,
-struct dma_attrs *attrs);
+unsigned long attrs);
 
 extern void
 xen_swiotlb_unmap_sg_attrs(struct device *hwdev, struct scatterlist *sgl,
   int nelems, enum dma_data_direction dir,
-

[Xen-devel] [PATCH v6 45/46] dma-mapping: Remove dma_get_attr

[Krzysztof Kozlowski]

After switching DMA attributes to unsigned long it is easier to just
compare the bits.

Signed-off-by: Krzysztof Kozlowski 
[for avr32]
Acked-by: Hans-Christian Noren Egtvedt 
[for arc]
Acked-by: Vineet Gupta 
[for arm64 and dma-iommu]
Acked-by: Robin Murphy 
---
 Documentation/DMA-API.txt  |  4 +--
 arch/arc/mm/dma.c  |  4 +--
 arch/arm/mm/dma-mapping.c  | 36 --
 arch/arm/xen/mm.c  |  4 +--
 arch/arm64/mm/dma-mapping.c| 10 +++
 arch/avr32/mm/dma-coherent.c   |  4 +--
 arch/ia64/sn/pci/pci_dma.c | 10 ++-
 arch/metag/kernel/dma.c|  2 +-
 arch/mips/mm/dma-default.c |  6 ++---
 arch/openrisc/kernel/dma.c |  4 +--
 arch/parisc/kernel/pci-dma.c   |  2 +-
 arch/powerpc/platforms/cell/iommu.c| 12 -
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c|  2 +-
 drivers/iommu/dma-iommu.c  |  2 +-
 drivers/media/v4l2-core/videobuf2-dma-contig.c |  2 +-
 include/linux/dma-mapping.h| 10 ---
 16 files changed, 47 insertions(+), 67 deletions(-)

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 24f9688bb98a..1d26eeb6b5f6 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -422,9 +422,7 @@ void whizco_dma_map_sg_attrs(struct device *dev, dma_addr_t 
dma_addr,
 unsigned long attrs)
 {

-   int foo =  dma_get_attr(DMA_ATTR_FOO, attrs);
-   
-   if (foo)
+   if (attrs & DMA_ATTR_FOO)
/* twizzle the frobnozzle */

 
diff --git a/arch/arc/mm/dma.c b/arch/arc/mm/dma.c
index 3d1f467d1792..74bbe68dce9d 100644
--- a/arch/arc/mm/dma.c
+++ b/arch/arc/mm/dma.c
@@ -46,7 +46,7 @@ static void *arc_dma_alloc(struct device *dev, size_t size,
 *   (vs. always going to memory - thus are faster)
 */
if ((is_isa_arcv2() && ioc_exists) ||
-   dma_get_attr(DMA_ATTR_NON_CONSISTENT, attrs))
+   (attrs & DMA_ATTR_NON_CONSISTENT))
need_coh = 0;
 
/*
@@ -95,7 +95,7 @@ static void arc_dma_free(struct device *dev, size_t size, 
void *vaddr,
struct page *page = virt_to_page(dma_handle);
int is_non_coh = 1;
 
-   is_non_coh = dma_get_attr(DMA_ATTR_NON_CONSISTENT, attrs) ||
+   is_non_coh = (attrs & DMA_ATTR_NON_CONSISTENT) ||
(is_isa_arcv2() && ioc_exists);
 
if (PageHighMem(page) || !is_non_coh)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index ebb3fde99043..43e03b5293d0 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -126,7 +126,7 @@ static dma_addr_t arm_dma_map_page(struct device *dev, 
struct page *page,
 unsigned long offset, size_t size, enum dma_data_direction dir,
 unsigned long attrs)
 {
-   if (!dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
+   if ((attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
__dma_page_cpu_to_dev(page, offset, size, dir);
return pfn_to_dma(dev, page_to_pfn(page)) + offset;
 }
@@ -155,7 +155,7 @@ static dma_addr_t arm_coherent_dma_map_page(struct device 
*dev, struct page *pag
 static void arm_dma_unmap_page(struct device *dev, dma_addr_t handle,
size_t size, enum dma_data_direction dir, unsigned long attrs)
 {
-   if (!dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
+   if ((attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
__dma_page_dev_to_cpu(pfn_to_page(dma_to_pfn(dev, handle)),
  handle & ~PAGE_MASK, size, dir);
 }
@@ -622,9 +622,9 @@ static void __free_from_contiguous(struct device *dev, 
struct page *page,
 
 static inline pgprot_t __get_dma_pgprot(unsigned long attrs, pgprot_t prot)
 {
-   prot = dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs) ?
-   pgprot_writecombine(prot) :
-   pgprot_dmacoherent(prot);
+   prot = (attrs & DMA_ATTR_WRITE_COMBINE) ?
+   pgprot_writecombine(prot) :
+   pgprot_dmacoherent(prot);
return prot;
 }
 
@@ -744,7 +744,7 @@ static void *__dma_alloc(struct device *dev, size_t size, 
dma_addr_t *handle,
.gfp = gfp,
.prot = prot,
.caller = caller,
-   .want_vaddr = !dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs),
+   .want_vaddr = ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) == 0),
};
 
 #ifdef CONFIG_DMA_API_DEBUG
@@ -887,7 +887,7 @@ static void __arm_dma_free(struct device *dev, size_t size, 
void *cpu_addr,
.size = PAGE_ALIGN(size),
.cpu_addr = cpu_addr,
.page = page,
-   .want_vaddr = !dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs),
+ 

[Xen-devel] [PATCH v6 24/46] x86: dma-mapping: Use unsigned long for dma_attrs

[Krzysztof Kozlowski]

Split out subsystem specific changes for easier reviews. This will be
squashed with main commit.

Signed-off-by: Krzysztof Kozlowski 
---
 arch/x86/include/asm/dma-mapping.h   |  5 ++---
 arch/x86/include/asm/swiotlb.h   |  4 ++--
 arch/x86/include/asm/xen/page-coherent.h |  9 -
 arch/x86/kernel/amd_gart_64.c| 20 ++--
 arch/x86/kernel/pci-calgary_64.c | 14 +++---
 arch/x86/kernel/pci-dma.c|  4 ++--
 arch/x86/kernel/pci-nommu.c  |  4 ++--
 arch/x86/kernel/pci-swiotlb.c|  4 ++--
 arch/x86/pci/sta2x11-fixup.c |  2 +-
 arch/x86/pci/vmd.c   | 16 
 10 files changed, 40 insertions(+), 42 deletions(-)

diff --git a/arch/x86/include/asm/dma-mapping.h 
b/arch/x86/include/asm/dma-mapping.h
index 3a27b93e6261..44461626830e 100644
--- a/arch/x86/include/asm/dma-mapping.h
+++ b/arch/x86/include/asm/dma-mapping.h
@@ -9,7 +9,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -48,11 +47,11 @@ extern int dma_supported(struct device *hwdev, u64 mask);
 
 extern void *dma_generic_alloc_coherent(struct device *dev, size_t size,
dma_addr_t *dma_addr, gfp_t flag,
-   struct dma_attrs *attrs);
+   unsigned long attrs);
 
 extern void dma_generic_free_coherent(struct device *dev, size_t size,
  void *vaddr, dma_addr_t dma_addr,
- struct dma_attrs *attrs);
+ unsigned long attrs);
 
 #ifdef CONFIG_X86_DMA_REMAP /* Platform code defines bridge-specific code */
 extern bool dma_capable(struct device *dev, dma_addr_t addr, size_t size);
diff --git a/arch/x86/include/asm/swiotlb.h b/arch/x86/include/asm/swiotlb.h
index ab05d73e2bb7..d2f69b9ff732 100644
--- a/arch/x86/include/asm/swiotlb.h
+++ b/arch/x86/include/asm/swiotlb.h
@@ -31,9 +31,9 @@ static inline void dma_mark_clean(void *addr, size_t size) {}
 
 extern void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
dma_addr_t *dma_handle, gfp_t flags,
-   struct dma_attrs *attrs);
+   unsigned long attrs);
 extern void x86_swiotlb_free_coherent(struct device *dev, size_t size,
void *vaddr, dma_addr_t dma_addr,
-   struct dma_attrs *attrs);
+   unsigned long attrs);
 
 #endif /* _ASM_X86_SWIOTLB_H */
diff --git a/arch/x86/include/asm/xen/page-coherent.h 
b/arch/x86/include/asm/xen/page-coherent.h
index acd844c017d3..f02f025ff988 100644
--- a/arch/x86/include/asm/xen/page-coherent.h
+++ b/arch/x86/include/asm/xen/page-coherent.h
@@ -2,12 +2,11 @@
 #define _ASM_X86_XEN_PAGE_COHERENT_H
 
 #include 
-#include 
 #include 
 
 static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t size,
dma_addr_t *dma_handle, gfp_t flags,
-   struct dma_attrs *attrs)
+   unsigned long attrs)
 {
void *vstart = (void*)__get_free_pages(flags, get_order(size));
*dma_handle = virt_to_phys(vstart);
@@ -16,18 +15,18 @@ static inline void *xen_alloc_coherent_pages(struct device 
*hwdev, size_t size,
 
 static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
void *cpu_addr, dma_addr_t dma_handle,
-   struct dma_attrs *attrs)
+   unsigned long attrs)
 {
free_pages((unsigned long) cpu_addr, get_order(size));
 }
 
 static inline void xen_dma_map_page(struct device *hwdev, struct page *page,
 dma_addr_t dev_addr, unsigned long offset, size_t size,
-enum dma_data_direction dir, struct dma_attrs *attrs) { }
+enum dma_data_direction dir, unsigned long attrs) { }
 
 static inline void xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
size_t size, enum dma_data_direction dir,
-   struct dma_attrs *attrs) { }
+   unsigned long attrs) { }
 
 static inline void xen_dma_sync_single_for_cpu(struct device *hwdev,
dma_addr_t handle, size_t size, enum dma_data_direction dir) { }
diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c
index 8e3842fc8bea..4aff288e37a4 100644
--- a/arch/x86/kernel/amd_gart_64.c
+++ b/arch/x86/kernel/amd_gart_64.c
@@ -242,7 +242,7 @@ static dma_addr_t dma_map_area(struct device *dev, 
dma_addr_t phys_mem,
 static dma_addr_t gart_map_page(struct device *dev, struct page *page,
unsigned long offset, size_t size,
enum dma_data_direction dir,
-   struct dma_attrs *attrs)
+   unsigned long attrs)
 {

Re: [Xen-devel] [PATCH] credi2-ratelimit: Implement rate limit for credit2 scheduler

[Dario Faggioli]

On Tue, 2016-07-12 at 17:16 +0100, George Dunlap wrote:
> On Wed, Jul 6, 2016 at 6:33 PM, Makkar anshul.mak...@citrix.com
>  wrote:
> > 
> > --- a/xen/common/sched_credit2.c
> > +++ b/xen/common/sched_credit2.c
> > @@ -171,6 +171,11 @@ integer_param("sched_credit2_migrate_resist",
> > opt_migrate_resist);
> >  #define c2r(_ops, _cpu) (CSCHED2_PRIV(_ops)->runq_map[(_cpu)])
> >  /* CPU to runqueue struct macro */
> >  #define RQD(_ops, _cpu) (&CSCHED2_PRIV(_ops)->rqd[c2r(_ops,
> > _cpu)])
> > +/* Find the max of time slice */
> > +#define MAX_TSLICE(t1, t2)  \
> > +   ({ typeof (t1) _t1 = (t1); \
> > +  typeof (t1) _t2 = (t2); \
> > +  _t1 > _t2 ? _t1 < 0 ? 0 : _t1 : _t2 < 0 ? 0
> > : _t2; })
> Hiding the zero-checking behind this seems a bit strange to me.  I
> think if the algorithm is properly laid out, we probably don't need
> this at all (see my proposed alternate below).
> 
I think it's more "overflow-avoidance" than "zero-checking", and as I
said, that's probably better done by means of subtraction(s).

In any case, I agree that, if we don't need it, that's even better. :-)

> > @@ -1709,17 +1767,22 @@ csched2_runtime(const struct scheduler
> > *ops, int cpu, struct csched2_vcpu *snext
> >   * at a different rate. */
> >  time = c2t(rqd, rt_credit, snext);
> > 
> > -/* Check limits */
> > -if ( time < CSCHED2_MIN_TIMER )
> > -{
> > -time = CSCHED2_MIN_TIMER;
> > -SCHED_STAT_CRANK(runtime_min_timer);
> > -}
> > -else if ( time > CSCHED2_MAX_TIMER )
> > +/* Check limits.
> > + * time > ratelimit --> time > MIN.
> > + */
> > +if ( time > CSCHED2_MAX_TIMER )
> >  {
> > +
> >  time = CSCHED2_MAX_TIMER;
> >  SCHED_STAT_CRANK(runtime_max_timer);
> >  }
> > +else
> > +{
> > +time = MAX_TSLICE(MAX_TSLICE(CSCHED2_MIN_TIMER,
> > +  MICROSECS(prv-
> > >ratelimit_us)), time);
> > +SCHED_STAT_CRANK(time);
> > +}
> > +
> >  }
> This is all really confusing.  What about something like this (also
> attached)?  The basic idea is to calculate min_time, then go through
> the normal algorithm, then clip it once at the end.
> 
Yes, this matches my idea and my comment, and I think the code provided
by George makes sense. Only one thing:

> @@ -1675,9 +1711,19 @@ csched2_runtime(const struct scheduler *ops,
> int cpu, struct csched2_vcpu *snext
>   * 1) Run until snext's credit will be 0
>   * 2) But if someone is waiting, run until snext's credit is
> equal
>   * to his
> - * 3) But never run longer than MAX_TIMER or shorter than
> MIN_TIMER.
> + * 3) But never run longer than MAX_TIMER or shorter than
> MIN_TIMER
> + * or your the ratelimit time.
>   */
> 
> +/* Calculate mintime */
> +min_time = CSCHED2_MIN_TIMER;
> +if ( prv->ratelimit_us ) {
> +s_time_t ratelimit_min = snext->vcpu-
> >runstate.state_entry_time +
> +MICROSECS(prv->ratelimit_us) - now;
>
Here snext can indeed be someone which was running already (e.g., we're
choosing current again), in which case runstate.state_entry-time-now
would indeed tell us how long it's actually been running, and the
formula (coupled with the if below) is correct.

But it also can be someone which is runnable (e.g., we're choosing
someone from the runqueue and preempting current), in which case
runstate.state_entry_time tells when it became runnable, and
state_entry_time-now is how long it's been runnable, which is not what
we want here.

In think, in such a case, we want ratelimit_min to just be equal to
prv->ratelimit_us. So, maybe, something like this:

 /* Caluclate mintime */
 min_time = CSCHED2_MIN_TIMER;
 if ( prv->ratelimit_us )
 {
     s_time_t ratelimit_min = prv->ratelimit_us;
     if ( snext->vcpu->is_running )     // XXX or is it better snext == 
curr_on_cpu(cpu)
         ratelimit_min = snext->vcpu->runstate.state_entry_time +
 MICROSECS(prv->ratelimit_us) - now;
 if ( ratelimit_min > min_time )
         min_time = ratelimit_min;
 }

> > @@ -2353,6 +2431,8 @@ csched2_init(struct scheduler *ops)
> >  prv->runq_map[i] = -1;
> >  prv->rqd[i].id = -1;
> >  }
> > +/* initialize ratelimit */
> > +prv->ratelimit_us = 1000 * CSCHED2_MIN_TIMER;
> Is there any reason this isn't using sched_ratelimit_us (the
> hypervisor command-line option, which the credit scheduler is using)?
> 
Yeah, I guess just using that is the best thing to start with.

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
_

[Xen-devel] [PATCH] xl: add option to leave domain paused afer migration

[Roger Pau Monne]

This is useful for debugging domains that crash on resume from migration.

Signed-off-by: Roger Pau Monné 
---
Cc: ian.jack...@eu.citrix.com
Cc: wei.l...@citrix.com
---
 tools/libxl/xl_cmdimpl.c  | 29 +++--
 tools/libxl/xl_cmdtable.c |  3 ++-
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index d8530f0..fd80442 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4742,7 +4742,7 @@ static void migrate_domain(uint32_t domid, const char 
*rune, int debug,
 exit(EXIT_FAILURE);
 }
 
-static void migrate_receive(int debug, int daemonize, int monitor,
+static void migrate_receive(int debug, int daemonize, int monitor, int pause,
 int send_fd, int recv_fd,
 libxl_checkpointed_stream checkpointed,
 char *colo_proxy_script)
@@ -4850,8 +4850,10 @@ static void migrate_receive(int debug, int daemonize, 
int monitor,
 if (rc) goto perhaps_destroy_notify_rc;
 }
 
-rc = libxl_domain_unpause(ctx, domid);
-if (rc) goto perhaps_destroy_notify_rc;
+if (!pause) {
+rc = libxl_domain_unpause(ctx, domid);
+if (rc) goto perhaps_destroy_notify_rc;
+}
 
 fprintf(stderr, "migration target: Domain started successsfully.\n");
 rc = 0;
@@ -4965,7 +4967,7 @@ int main_restore(int argc, char **argv)
 
 int main_migrate_receive(int argc, char **argv)
 {
-int debug = 0, daemonize = 1, monitor = 1;
+int debug = 0, daemonize = 1, monitor = 1, pause = 0;
 libxl_checkpointed_stream checkpointed = LIBXL_CHECKPOINTED_STREAM_NONE;
 int opt;
 char *script = NULL;
@@ -4976,7 +4978,7 @@ int main_migrate_receive(int argc, char **argv)
 COMMON_LONG_OPTS
 };
 
-SWITCH_FOREACH_OPT(opt, "Fedr", opts, "migrate-receive", 0) {
+SWITCH_FOREACH_OPT(opt, "Fedrp", opts, "migrate-receive", 0) {
 case 'F':
 daemonize = 0;
 break;
@@ -4996,13 +4998,16 @@ int main_migrate_receive(int argc, char **argv)
 case 0x200:
 script = optarg;
 break;
+case 'p':
+pause = 1;
+break;
 }
 
 if (argc-optind != 0) {
 help("migrate-receive");
 return EXIT_FAILURE;
 }
-migrate_receive(debug, daemonize, monitor,
+migrate_receive(debug, daemonize, monitor, pause,
 STDOUT_FILENO, STDIN_FILENO,
 checkpointed, script);
 
@@ -5048,14 +5053,14 @@ int main_migrate(int argc, char **argv)
 const char *ssh_command = "ssh";
 char *rune = NULL;
 char *host;
-int opt, daemonize = 1, monitor = 1, debug = 0;
+int opt, daemonize = 1, monitor = 1, debug = 0, pause = 0;
 static struct option opts[] = {
 {"debug", 0, 0, 0x100},
 {"live", 0, 0, 0x200},
 COMMON_LONG_OPTS
 };
 
-SWITCH_FOREACH_OPT(opt, "FC:s:e", opts, "migrate", 2) {
+SWITCH_FOREACH_OPT(opt, "FC:s:ep", opts, "migrate", 2) {
 case 'C':
 config_filename = optarg;
 break;
@@ -5069,6 +5074,9 @@ int main_migrate(int argc, char **argv)
 daemonize = 0;
 monitor = 0;
 break;
+case 'p':
+pause = 1;
+break;
 case 0x100: /* --debug */
 debug = 1;
 break;
@@ -5096,12 +5104,13 @@ int main_migrate(int argc, char **argv)
 } else {
 verbose_len = (minmsglevel_default - minmsglevel) + 2;
 }
-xasprintf(&rune, "exec %s %s xl%s%.*s migrate-receive%s%s",
+xasprintf(&rune, "exec %s %s xl%s%.*s migrate-receive%s%s%s",
   ssh_command, host,
   pass_tty_arg ? " -t" : "",
   verbose_len, verbose_buf,
   daemonize ? "" : " -e",
-  debug ? " -d" : "");
+  debug ? " -d" : "",
+  pause ? " -p" : "");
 }
 
 migrate_domain(domid, rune, debug, config_filename);
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index bf69ffb..85c1e0f 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -164,7 +164,8 @@ struct cmd_spec cmd_table[] = {
   "migrate-receive [-d -e]\n"
   "-e  Do not wait in the background (on ) for the 
death\n"
   "of the domain.\n"
-  "--debug Print huge (!) amount of debug during the migration 
process."
+  "--debug Print huge (!) amount of debug during the migration 
process.\n"
+  "-p  Do not unpause domain after migrating it."
 },
 { "restore",
   &main_restore, 0, 1,
-- 
2.7.4 (Apple Git-66)


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

Re: [Xen-devel] [PATCH v3 1/2] Interface for grant copy operation in libs.

[Wei Liu]

On Fri, Jul 08, 2016 at 02:18:46PM +0100, Wei Liu wrote:
> To unblock Paulina on her series, I would be ok with the cast provided
> there is compile-time check to ensure the user-space structure is
> identical to the ioctl structure.
> 
> That would involve:
> 1. Introducing BUILD_BUG_ON, offsetof, alignof to libs/ if they are not
>already available.

I just checked all these.

BUILD_BUG_ON is not there. A patch for it is trivial. I will do that
soon.

offsetof can be found in stddef.h.

alignof is C11 (we require C99), but there is __alignof__ gcc extension,
which clang also supports. We've been using that for quite a long time,
so I don't think we need to do anything about it.

Wei.

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

[Xen-devel] [PATCH] Add optional ACPI device for Windows Continuum

[Owen Smith]

Windows 10 supports a specific ACPI device for handling the
switch between tablet mode and desktop mode. The meer existance
of this device is the mimimum to allow tablet/desktop mode to
be switched.
Tablet mode referes to the "undocked" state where all applications
are forced full screen and additional touch screen elements are
added, such as touch keyboard, larger icons and menus, and touch
gestures for ease of use.

Signed-off-by: Owen Smith 
---
 tools/firmware/hvmloader/acpi/Makefile  |  4 ++--
 tools/firmware/hvmloader/acpi/build.c   | 11 ++
 tools/firmware/hvmloader/acpi/ssdt_conv.asl | 31 +
 tools/libxl/libxl_types.idl |  1 +
 tools/libxl/xl_cmdimpl.c|  1 +
 5 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 tools/firmware/hvmloader/acpi/ssdt_conv.asl

diff --git a/tools/firmware/hvmloader/acpi/Makefile 
b/tools/firmware/hvmloader/acpi/Makefile
index d3e882a..d75c7af 100644
--- a/tools/firmware/hvmloader/acpi/Makefile
+++ b/tools/firmware/hvmloader/acpi/Makefile
@@ -25,7 +25,7 @@ CFLAGS += $(CFLAGS_xeninclude)
 vpath iasl $(PATH)
 all: acpi.a
 
-ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h: %.h: %.asl iasl
+ssdt_s3.h ssdt_s4.h ssdt_conv.h ssdt_pm.h ssdt_tpm.h: %.h: %.asl iasl
iasl -vs -p $* -tc $<
sed -e 's/AmlCode/$*/g' $*.hex >$@
rm -f $*.hex $*.aml
@@ -56,7 +56,7 @@ iasl:
@echo 
@exit 1
 
-build.o: ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h
+build.o: ssdt_s3.h ssdt_s4.h ssdt_conv.h ssdt_pm.h ssdt_tpm.h
 
 acpi.a: $(OBJS)
$(AR) rc $@ $(OBJS)
diff --git a/tools/firmware/hvmloader/acpi/build.c 
b/tools/firmware/hvmloader/acpi/build.c
index 1f7103e..6485ac8 100644
--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -18,6 +18,7 @@
 #include "acpi2_0.h"
 #include "ssdt_s3.h"
 #include "ssdt_s4.h"
+#include "ssdt_conv.h"
 #include "ssdt_tpm.h"
 #include "ssdt_pm.h"
 #include "../config.h"
@@ -398,6 +399,16 @@ static int construct_secondary_tables(unsigned long 
*table_ptrs,
 printf("S4 disabled\n");
 }
 
+if ( !strncmp(xenstore_read("platform/acpi_conv", "1"), "1", 1) )
+{
+ssdt = mem_alloc(sizeof(ssdt_conv), 16);
+if (!ssdt) return -1;
+memcpy(ssdt, ssdt_conv, sizeof(ssdt_conv));
+table_ptrs[nr_tables++] = (unsigned long)ssdt;
+} else {
+printf("Conv disabled\n");
+}
+
 /* TPM TCPA and SSDT. */
 tis_hdr = (uint16_t *)0xFED40F00;
 if ( (tis_hdr[0] == tis_signature[0]) &&
diff --git a/tools/firmware/hvmloader/acpi/ssdt_conv.asl 
b/tools/firmware/hvmloader/acpi/ssdt_conv.asl
new file mode 100644
index 000..6e20340
--- /dev/null
+++ b/tools/firmware/hvmloader/acpi/ssdt_conv.asl
@@ -0,0 +1,31 @@
+/*
+ * ssdt_conv.asl
+ *
+ * Copyright (c) 2015  Citrix Systems, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; If not, see .
+ */
+
+DefinitionBlock ("SSDT_CONV.aml", "SSDT", 2, "Xen", "HVM", 0)
+{
+Device(CONV)
+{
+Method(_HID, 0x0, NotSerialized)
+{
+Return("ID9001")
+}
+Name(_CID, "PNP0C60")
+}
+}
+
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index ef614be..01c7c61 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -502,6 +502,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
("acpi", libxl_defbool),
("acpi_s3",  libxl_defbool),
("acpi_s4",  libxl_defbool),
+   ("acpi_conv",libxl_defbool),
("nx",   libxl_defbool),
("viridian", libxl_defbool),
("viridian_enable",  libxl_bitmap),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index d1fcfa4..2db76bf 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1574,6 +1574,7 @@ static void parse_config_data(const char *config_source,
 xlu_cfg_get_defbool(config, "acpi", &b_info->u.hvm.acpi, 0);
 xlu_cfg_get_defbool(config, "acpi_s3", &b_info->u.hvm.acpi_s3, 0);
 xlu_cfg_get_defbool(config, "acpi_s4", &b_info->u.hvm.a

Re: [Xen-devel] [PATCH v2 03/17] libxl/arm: Add a configuration option for ARM DomU ACPI

[Julien Grall]

Hi Shannon,

On 13/07/2016 08:54, Shannon Zhao wrote:

On 2016/7/12 19:33, Wei Liu wrote:

On Tue, Jul 12, 2016 at 10:22:39AM +0100, Julien Grall wrote:
[...]

Yeah, we can deprecate that field. But we need to take care to not break
users of the old field.

Ok, what name would you suggest?


I would suggest b_info->u.acpi



b_info->acpi would be more appropriate.

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index ef614be..a57823d 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -494,11 +494,16 @@ libxl_domain_build_info = Struct("domain_build_info",[
 # Note that the partial device tree should avoid to use the phandle
 # 65000 which is reserved by the toolstack.
 ("device_tree",  string),
+("acpi", libxl_defbool),
 ("u", KeyedUnion(None, libxl_domain_type, "type",
 [("hvm", Struct(None, [("firmware", string),
("bios", libxl_bios_type),
("pae",  libxl_defbool),
("apic", libxl_defbool),
+   # The following acpi field is
+   # deprecated. Please use the unified
+   # acpi field above which works for both
+   # x86 and ARM.
("acpi", libxl_defbool),
("acpi_s3",  libxl_defbool),
("acpi_s4",  libxl_defbool),


And then:

1. modify xl to set the new field.
2. modify libxl to handle compatibility: user of the old field should
   continue to work.


I found that the default value of acpi is true on x86. But we decided
before it's should be false by default on ARM. How to deal with this?
Julien, Stefano, can we make acpi true by default?


I already said that I am not in favor of making ACPI true by default at 
least for ARM 32-bit guest.


ARM 32-bit guest will not use ACPI, if we decide to enable it by 
default, we will require the user to install iasl for nothing.


IHMO, ACPI should be disabled by default for any ARM guests. Libxl can 
take this decision easily.


Regards,

--
Julien Grall

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

[Xen-devel] [PATCH] libs/gnttab: introduce BUILD_BUG_ON

[Wei Liu]

The implementation is taken from libxc.

Signed-off-by: Wei Liu 
---
Cc: Ian Jackson 
Cc: Paulina Szubarczyk 

I could have put it in a header file accessible to all libraries under
libs but this construct is only relevant to xengnttab library at the
moment so it's put under gnttab/private.h. It can be easily moved to
a common place when other libraries under libs require it.

This patch is necessary to unblock Paulina on her gnttab copy work.
---
 tools/libs/gnttab/private.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
index d286c86..da487f2 100644
--- a/tools/libs/gnttab/private.h
+++ b/tools/libs/gnttab/private.h
@@ -4,6 +4,13 @@
 #include 
 #include 
 
+/* Force a compilation error if condition is true */
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
+#define BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
+#else
+#define BUILD_BUG_ON(p) ((void)sizeof(struct { int:-!!(p); }))
+#endif
+
 struct xengntdev_handle {
 xentoollog_logger *logger, *logger_tofree;
 int fd;
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v2 03/17] libxl/arm: Add a configuration option for ARM DomU ACPI

[Shannon Zhao]

On 2016/7/13 17:20, Julien Grall wrote:
> On 13/07/2016 08:54, Shannon Zhao wrote:
>> On 2016/7/12 19:33, Wei Liu wrote:
>>> On Tue, Jul 12, 2016 at 10:22:39AM +0100, Julien Grall wrote:
>>> [...]
>> Yeah, we can deprecate that field. But we need to take care to not
>> break
>> users of the old field.
> Ok, what name would you suggest?

 I would suggest b_info->u.acpi

>>>
>>> b_info->acpi would be more appropriate.
>>>
>>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>>> index ef614be..a57823d 100644
>>> --- a/tools/libxl/libxl_types.idl
>>> +++ b/tools/libxl/libxl_types.idl
>>> @@ -494,11 +494,16 @@ libxl_domain_build_info =
>>> Struct("domain_build_info",[
>>>  # Note that the partial device tree should avoid to use the phandle
>>>  # 65000 which is reserved by the toolstack.
>>>  ("device_tree",  string),
>>> +("acpi", libxl_defbool),
>>>  ("u", KeyedUnion(None, libxl_domain_type, "type",
>>>  [("hvm", Struct(None, [("firmware", string),
>>> ("bios",
>>> libxl_bios_type),
>>> ("pae", 
>>> libxl_defbool),
>>> ("apic",
>>> libxl_defbool),
>>> +   # The following acpi field is
>>> +   # deprecated. Please use the
>>> unified
>>> +   # acpi field above which
>>> works for both
>>> +   # x86 and ARM.
>>> ("acpi",
>>> libxl_defbool),
>>> ("acpi_s3", 
>>> libxl_defbool),
>>> ("acpi_s4", 
>>> libxl_defbool),
>>>
>>>
>>> And then:
>>>
>>> 1. modify xl to set the new field.
>>> 2. modify libxl to handle compatibility: user of the old field should
>>>continue to work.
>>>
>> I found that the default value of acpi is true on x86. But we decided
>> before it's should be false by default on ARM. How to deal with this?
>> Julien, Stefano, can we make acpi true by default?
> 
> I already said that I am not in favor of making ACPI true by default at
> least for ARM 32-bit guest.
> 
> ARM 32-bit guest will not use ACPI, if we decide to enable it by
> default, we will require the user to install iasl for nothing.
> 
> IHMO, ACPI should be disabled by default for any ARM guests. Libxl can
> take this decision easily.
I know but here we want to unify the acpi option for x86 and ARM while
on x86 it's true by default. What I want to ask is that how to
distinguish x86 and ARM in libxl__domain_build_info_setdefault(), so we
can assign acpi with different default value for x86 and ARM.

Thanks,
-- 
Shannon


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

Re: [Xen-devel] [PATCH v2 03/17] libxl/arm: Add a configuration option for ARM DomU ACPI

[Julien Grall]

On 13/07/2016 10:48, Shannon Zhao wrote:



On 2016/7/13 17:20, Julien Grall wrote:

On 13/07/2016 08:54, Shannon Zhao wrote:

On 2016/7/12 19:33, Wei Liu wrote:

On Tue, Jul 12, 2016 at 10:22:39AM +0100, Julien Grall wrote:
[...]

Yeah, we can deprecate that field. But we need to take care to not
break
users of the old field.

Ok, what name would you suggest?


I would suggest b_info->u.acpi



b_info->acpi would be more appropriate.

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index ef614be..a57823d 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -494,11 +494,16 @@ libxl_domain_build_info =
Struct("domain_build_info",[
 # Note that the partial device tree should avoid to use the phandle
 # 65000 which is reserved by the toolstack.
 ("device_tree",  string),
+("acpi", libxl_defbool),
 ("u", KeyedUnion(None, libxl_domain_type, "type",
 [("hvm", Struct(None, [("firmware", string),
("bios",
libxl_bios_type),
("pae",
libxl_defbool),
("apic",
libxl_defbool),
+   # The following acpi field is
+   # deprecated. Please use the
unified
+   # acpi field above which
works for both
+   # x86 and ARM.
("acpi",
libxl_defbool),
("acpi_s3",
libxl_defbool),
("acpi_s4",
libxl_defbool),


And then:

1. modify xl to set the new field.
2. modify libxl to handle compatibility: user of the old field should
   continue to work.


I found that the default value of acpi is true on x86. But we decided
before it's should be false by default on ARM. How to deal with this?
Julien, Stefano, can we make acpi true by default?


I already said that I am not in favor of making ACPI true by default at
least for ARM 32-bit guest.

ARM 32-bit guest will not use ACPI, if we decide to enable it by
default, we will require the user to install iasl for nothing.

IHMO, ACPI should be disabled by default for any ARM guests. Libxl can
take this decision easily.

I know but here we want to unify the acpi option for x86 and ARM while
on x86 it's true by default. What I want to ask is that how to
distinguish x86 and ARM in libxl__domain_build_info_setdefault(), so we
can assign acpi with different default value for x86 and ARM.


By using #ifdef in the code?

Regards,

--
Julien Grall

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

Re: [Xen-devel] [DRAFT 1] XenSock protocol design document

[Stefano Stabellini]

On Mon, 11 Jul 2016, Joao Martins wrote:
> On 07/08/2016 12:23 PM, Stefano Stabellini wrote:
> > Hi all,
> > 
> Hey!
> 
> [...]
> 
> > 
> > ## Design
> > 
> > ### Xenstore
> > 
> > The frontend and the backend connect to each other exchanging information 
> > via
> > xenstore. The toolstack creates front and back nodes with state
> > XenbusStateInitialising. There can only be one XenSock frontend per domain.
> > 
> >  Frontend XenBus Nodes
> > 
> > port
> >  Values: 
> > 
> >  The identifier of the Xen event channel used to signal activity
> >  in the ring buffer.
> > 
> > ring-ref
> >  Values: 
> > 
> >  The Xen grant reference granting permission for the backend to map
> >  the sole page in a single page sized ring buffer.
> 
> Would it make sense to export minimum, default and maximum size of the socket 
> over
> xenstore entries? It normally follows a convention depending on the type of 
> socket
> (and OS) you have, or then through settables on socket options.

It makes sense, Juergen suggested something similar. I am thinking of
passing the maximum order of the data ring.

 
> > ### Commands Ring
> > 
> > The shared ring is used by the frontend to forward socket API calls to the
> > backend. I'll refer to this ring as **commands ring** to distinguish it from
> > other rings which will be created later in the lifecycle of the protocol 
> > (data
> > rings). The ring format is defined using the familiar `DEFINE_RING_TYPES` 
> > macro
> > (`xen/include/public/io/ring.h`). Frontend requests are allocated on the 
> > ring
> > using the `RING_GET_REQUEST` macro.
> > 
> > The format is defined as follows:
> > 
> > #define XENSOCK_DATARING_ORDER 6
> > #define XENSOCK_DATARING_PAGES (1 << XENSOCK_DATARING_ORDER)
> > #define XENSOCK_DATARING_SIZE (XENSOCK_DATARING_PAGES << PAGE_SHIFT)
> > 
> > #define XENSOCK_CONNECT0
> > #define XENSOCK_RELEASE3
> > #define XENSOCK_BIND   4
> > #define XENSOCK_LISTEN 5
> > #define XENSOCK_ACCEPT 6
> > #define XENSOCK_POLL   7
> > 
> > struct xen_xensock_request {
> > uint32_t id; /* private to guest, echoed in response */
> > uint32_t cmd;/* command to execute */
> > uint64_t sockid; /* id of the socket */
> > union {
> > struct xen_xensock_connect {
> > uint8_t addr[28];
> > uint32_t len;
> > uint32_t flags;
> > grant_ref_t ref[XENSOCK_DATARING_PAGES];
> > uint32_t evtchn;
> > } connect;
> > struct xen_xensock_bind {
> > uint8_t addr[28]; /* ipv6 ready */
> > uint32_t len;
> > } bind;
> > struct xen_xensock_accept {
> > uint64_t sockid;
> > grant_ref_t ref[XENSOCK_DATARING_PAGES];
> > uint32_t evtchn;
> > } accept;
> > } u;
> > };
> > 
> > The first three fields are common for every command. Their binary layout
> > is:
> > 
> > 0   4   8   12  16
> > +---+---+---+---+
> > |  id   |  cmd  | sockid|
> > +---+---+---+---+
> > 
> > - **id** is generated by the frontend and identifies one specific request
> > - **cmd** is the command requested by the frontend:
> > - `XENSOCK_CONNECT`: 0
> > - `XENSOCK_RELEASE`: 3
> > - `XENSOCK_BIND`:4
> > - `XENSOCK_LISTEN`:  5
> > - `XENSOCK_ACCEPT`:  6
> > - `XENSOCK_POLL`:7
> > - **sockid** is generated by the frontend and identifies the socket to 
> > connect,
> >   bind, etc. A new sockid is required on `XENSOCK_CONNECT` and 
> > `XENSOCK_BIND`
> >   commands. A new sockid is also required on `XENSOCK_ACCEPT`, for the new
> >   socket.
> >   
> Interesting - Have you consider setsockopt and getsockopt to be part of this? 
> There
> are some common options (as in POSIX defined) and then some more exotic 
> flavors Linux
> or FreeBSD specific. Say SO_REUSEPORT used on nginx that is good for load 
> balancing
> across a set of workers or Linux SO_BUSY_POLL for low latency sockets. Though 
> not
> sure how sensible it is to start exposing all of these socket options but to 
> limit to
> a specific subset? Or maybe doesn't make sense for your case - see further 
> suggestion
> regarding data ring part.

I have considered it, but I thought that they might be better suited for
a v2 version of the spec. This protocol needs to be extensible and
adding two new operations such as setsockopt and getsockopt should be
the simplest thing to do. Old backends should return ENOTSUPP. I'll
mention this explicitly in the next draft.


> > All three fields are echoed back by the backend.
> > 
> > As for the other Xen ring based protocols, after writing a request to the 
> > ring,
> > the frontend calls `RING_PUSH_REQUESTS_AND_CHECK_NOTIFY` and

Re: [Xen-devel] [PATCH] credi2-ratelimit: Implement rate limit for credit2 scheduler

[George Dunlap]

On 13/07/16 09:53, Dario Faggioli wrote:
>> @@ -1675,9 +1711,19 @@ csched2_runtime(const struct scheduler *ops,
>> int cpu, struct csched2_vcpu *snext
>>   * 1) Run until snext's credit will be 0
>>   * 2) But if someone is waiting, run until snext's credit is
>> equal
>>   * to his
>> - * 3) But never run longer than MAX_TIMER or shorter than
>> MIN_TIMER.
>> + * 3) But never run longer than MAX_TIMER or shorter than
>> MIN_TIMER
>> + * or your the ratelimit time.
>>   */
>>
>> +/* Calculate mintime */
>> +min_time = CSCHED2_MIN_TIMER;
>> +if ( prv->ratelimit_us ) {
>> +s_time_t ratelimit_min = snext->vcpu-
>>> runstate.state_entry_time +
>> +MICROSECS(prv->ratelimit_us) - now;
>>
> Here snext can indeed be someone which was running already (e.g., we're
> choosing current again), in which case runstate.state_entry-time-now
> would indeed tell us how long it's actually been running, and the
> formula (coupled with the if below) is correct.
> 
> But it also can be someone which is runnable (e.g., we're choosing
> someone from the runqueue and preempting current), in which case
> runstate.state_entry_time tells when it became runnable, and
> state_entry_time-now is how long it's been runnable, which is not what
> we want here.
> 
> In think, in such a case, we want ratelimit_min to just be equal to
> prv->ratelimit_us. So, maybe, something like this:
> 
>  /* Caluclate mintime */
>  min_time = CSCHED2_MIN_TIMER;
>  if ( prv->ratelimit_us )
>  {
>  s_time_t ratelimit_min = prv->ratelimit_us;
>  if ( snext->vcpu->is_running ) // XXX or is it better snext == 
> curr_on_cpu(cpu)
>  ratelimit_min = snext->vcpu->runstate.state_entry_time +
>  MICROSECS(prv->ratelimit_us) - now;
>  if ( ratelimit_min > min_time )
>  min_time = ratelimit_min;
>  }

+1

 -George

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

[Xen-devel] [PATCH 0/2] adjustments

[Corneliu ZUZU]

Corneliu ZUZU (2):
  asm/atomic.h: common prototyping (add xen/atomic.h)
  fix: make atomic_read() param const

 xen/include/asm-arm/arm32/atomic.h |  11 ---
 xen/include/asm-arm/arm64/atomic.h |  11 ---
 xen/include/asm-arm/atomic.h   |  91 +++---
 xen/include/asm-x86/atomic.h   |  51 +---
 xen/include/xen/atomic.h   | 155 +
 5 files changed, 257 insertions(+), 62 deletions(-)
 create mode 100644 xen/include/xen/atomic.h

-- 
2.5.0


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

[Xen-devel] [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)

[Corneliu ZUZU]

Following Andrew Cooper's suggestion, create a common-side  to
establish, among others, prototypes of atomic functions called from common-code.
Done to avoid introducing inconsistencies between arch-side 
headers when we make subtle changes to one of them.

Some arm-side macros had to be turned into inline functions in the process.

Also includes a minor adjustment asm-x86/atomic.h: reorder atomic_inc_and_test()
to follow after atomic_inc().

Signed-off-by: Corneliu ZUZU 
---
 xen/include/asm-arm/arm32/atomic.h |  11 ---
 xen/include/asm-arm/arm64/atomic.h |  11 ---
 xen/include/asm-arm/atomic.h   |  89 ++---
 xen/include/asm-x86/atomic.h   |  49 +---
 xen/include/xen/atomic.h   | 155 +
 5 files changed, 255 insertions(+), 60 deletions(-)
 create mode 100644 xen/include/xen/atomic.h

diff --git a/xen/include/asm-arm/arm32/atomic.h 
b/xen/include/asm-arm/arm32/atomic.h
index 7ec712f..be08ff1 100644
--- a/xen/include/asm-arm/arm32/atomic.h
+++ b/xen/include/asm-arm/arm32/atomic.h
@@ -149,17 +149,6 @@ static inline int __atomic_add_unless(atomic_t *v, int a, 
int u)
 
 #define atomic_xchg(v, new) (xchg(&((v)->counter), new))
 
-#define atomic_inc(v)  atomic_add(1, v)
-#define atomic_dec(v)  atomic_sub(1, v)
-
-#define atomic_inc_and_test(v) (atomic_add_return(1, v) == 0)
-#define atomic_dec_and_test(v) (atomic_sub_return(1, v) == 0)
-#define atomic_inc_return(v)(atomic_add_return(1, v))
-#define atomic_dec_return(v)(atomic_sub_return(1, v))
-#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0)
-
-#define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0)
-
 #endif /* __ARCH_ARM_ARM32_ATOMIC__ */
 /*
  * Local variables:
diff --git a/xen/include/asm-arm/arm64/atomic.h 
b/xen/include/asm-arm/arm64/atomic.h
index b49219e..80d07bf 100644
--- a/xen/include/asm-arm/arm64/atomic.h
+++ b/xen/include/asm-arm/arm64/atomic.h
@@ -125,17 +125,6 @@ static inline int __atomic_add_unless(atomic_t *v, int a, 
int u)
return c;
 }
 
-#define atomic_inc(v)  atomic_add(1, v)
-#define atomic_dec(v)  atomic_sub(1, v)
-
-#define atomic_inc_and_test(v) (atomic_add_return(1, v) == 0)
-#define atomic_dec_and_test(v) (atomic_sub_return(1, v) == 0)
-#define atomic_inc_return(v)(atomic_add_return(1, v))
-#define atomic_dec_return(v)(atomic_sub_return(1, v))
-#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0)
-
-#define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0)
-
 #endif
 /*
  * Local variables:
diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 29ab265..8e8c5d1 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -2,6 +2,7 @@
 #define __ARCH_ARM_ATOMIC__
 
 #include 
+#include 
 #include 
 #include 
 
@@ -95,15 +96,6 @@ void __bad_atomic_size(void);
 default: __bad_atomic_size(); break;\
 }   \
 })
-
-/*
- * NB. I've pushed the volatile qualifier into the operations. This allows
- * fast accessors such as _atomic_read() and _atomic_set() which don't give
- * the compiler a fit.
- */
-typedef struct { int counter; } atomic_t;
-
-#define ATOMIC_INIT(i) { (i) }
 
 /*
  * On ARM, ordinary assignment (str instruction) doesn't clear the local
@@ -138,6 +130,85 @@ static inline void _atomic_set(atomic_t *v, int i)
 # error "unknown ARM variant"
 #endif
 
+#define atomic_inc_return(v)(atomic_add_return(1, v))
+#define atomic_dec_return(v)(atomic_sub_return(1, v))
+
+/**
+ * atomic_sub_and_test - subtract value from variable and test result
+ * @i: integer value to subtract
+ * @v: pointer of type atomic_t
+ *
+ * Atomically subtracts @i from @v and returns
+ * true if the result is zero, or false for all
+ * other cases.
+ */
+static inline int atomic_sub_and_test(int i, atomic_t *v)
+{
+return 0 == atomic_sub_return(i, v);
+}
+
+/**
+ * atomic_inc - increment atomic variable
+ * @v: pointer of type atomic_t
+ *
+ * Atomically increments @v by 1.
+ */
+static inline void atomic_inc(atomic_t *v)
+{
+atomic_add(1, v);
+}
+
+/**
+ * atomic_inc_and_test - increment and test
+ * @v: pointer of type atomic_t
+ *
+ * Atomically increments @v by 1
+ * and returns true if the result is zero, or false for all
+ * other cases.
+ */
+static inline int atomic_inc_and_test(atomic_t *v)
+{
+return 0 == atomic_add_return(1, v);
+}
+
+/**
+ * atomic_dec - decrement atomic variable
+ * @v: pointer of type atomic_t
+ *
+ * Atomically decrements @v by 1.
+ */
+static inline void atomic_dec(atomic_t *v)
+{
+atomic_sub(1, v);
+}
+
+/**
+ * atomic_dec_and_test - decrement and test
+ * @v: pointer of type atomic_t
+ *
+ * Atomically decrements @v by 1 and
+ * returns true if the result is 0, or false for all other
+ * cases.
+ */
+static inline int atomic_dec_and_test(atomic_t *v)
+{
+return 0 == atomic_sub_ret

[Xen-devel] [PATCH 2/2] fix: make atomic_read() param const

[Corneliu ZUZU]

This wouldn't let me make a param of a function that used atomic_read() const.

Signed-off-by: Corneliu ZUZU 
---
 xen/include/asm-arm/atomic.h | 2 +-
 xen/include/asm-x86/atomic.h | 2 +-
 xen/include/xen/atomic.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 8e8c5d1..cd86cf0 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -102,7 +102,7 @@ void __bad_atomic_size(void);
  * strex/ldrex monitor on some implementations. The reason we can use it for
  * atomic_set() is the clrex or dummy strex done on every exception return.
  */
-static inline int atomic_read(atomic_t *v)
+static inline int atomic_read(const atomic_t *v)
 {
 return *(volatile int *)&v->counter;
 }
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index 9bf4bc1..a16f0f6 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -86,7 +86,7 @@ void __bad_atomic_size(void);
  *
  * Atomically reads the value of @v.
  */
-static inline int atomic_read(atomic_t *v)
+static inline int atomic_read(const atomic_t *v)
 {
 return read_atomic(&v->counter);
 }
diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
index 5d5c051..6cbc6d8 100644
--- a/xen/include/xen/atomic.h
+++ b/xen/include/xen/atomic.h
@@ -37,7 +37,7 @@ typedef struct { int counter; } atomic_t;
  *
  * Atomically reads the value of @v.
  */
-static inline int atomic_read(atomic_t *v);
+static inline int atomic_read(const atomic_t *v);
 
 /**
  * _atomic_read - read atomic variable non-atomically
-- 
2.5.0


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

Re: [Xen-devel] [PATCH] libs/gnttab: introduce BUILD_BUG_ON

[Paulina Szubarczyk]

On 07/13/2016 11:25 AM, Wei Liu wrote:

The implementation is taken from libxc.

Signed-off-by: Wei Liu 
---
Cc: Ian Jackson 
Cc: Paulina Szubarczyk 

I could have put it in a header file accessible to all libraries under
libs but this construct is only relevant to xengnttab library at the
moment so it's put under gnttab/private.h. It can be easily moved to
a common place when other libraries under libs require it.

This patch is necessary to unblock Paulina on her gnttab copy work.
---
  tools/libs/gnttab/private.h | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
index d286c86..da487f2 100644
--- a/tools/libs/gnttab/private.h
+++ b/tools/libs/gnttab/private.h
@@ -4,6 +4,13 @@
  #include 
  #include 

+/* Force a compilation error if condition is true */
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
+#define BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
+#else
+#define BUILD_BUG_ON(p) ((void)sizeof(struct { int:-!!(p); }))
+#endif
+
  struct xengntdev_handle {
  xentoollog_logger *logger, *logger_tofree;
  int fd;


Hi Wei,

thank you for the help with the grant copy patch. I have just rebuild 
Xen and there is a conflict with definition of BUILD_BUG_ON for mini-os 
build:


In file included from minios.c:33:0:
private.h:9:0: error: "BUILD_BUG_ON" redefined [-Werror]
 #define BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
 ^
In file included from minios.c:25:0:
xen/extras/mini-os-remote/include/lib.h:58:0: note: this is the location 
of the previous definition

 #define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); })
 ^
I thought about wrapping the code in #ifndef BUILD_BUG_ON #endif.

Paulina

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

Re: [Xen-devel] [PATCH] libs/gnttab: introduce BUILD_BUG_ON

[Wei Liu]

On Wed, Jul 13, 2016 at 01:32:10PM +0200, Paulina Szubarczyk wrote:
> 
> 
> On 07/13/2016 11:25 AM, Wei Liu wrote:
> >The implementation is taken from libxc.
> >
> >Signed-off-by: Wei Liu 
> >---
> >Cc: Ian Jackson 
> >Cc: Paulina Szubarczyk 
> >
> >I could have put it in a header file accessible to all libraries under
> >libs but this construct is only relevant to xengnttab library at the
> >moment so it's put under gnttab/private.h. It can be easily moved to
> >a common place when other libraries under libs require it.
> >
> >This patch is necessary to unblock Paulina on her gnttab copy work.
> >---
> >  tools/libs/gnttab/private.h | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> >diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
> >index d286c86..da487f2 100644
> >--- a/tools/libs/gnttab/private.h
> >+++ b/tools/libs/gnttab/private.h
> >@@ -4,6 +4,13 @@
> >  #include 
> >  #include 
> >
> >+/* Force a compilation error if condition is true */
> >+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
> >+#define BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
> >+#else
> >+#define BUILD_BUG_ON(p) ((void)sizeof(struct { int:-!!(p); }))
> >+#endif
> >+
> >  struct xengntdev_handle {
> >  xentoollog_logger *logger, *logger_tofree;
> >  int fd;
> >
> Hi Wei,
> 
> thank you for the help with the grant copy patch. I have just rebuild Xen
> and there is a conflict with definition of BUILD_BUG_ON for mini-os build:
> 
> In file included from minios.c:33:0:
> private.h:9:0: error: "BUILD_BUG_ON" redefined [-Werror]
>  #define BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
>  ^
> In file included from minios.c:25:0:
> xen/extras/mini-os-remote/include/lib.h:58:0: note: this is the location of
> the previous definition
>  #define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); })
>  ^
> I thought about wrapping the code in #ifndef BUILD_BUG_ON #endif.

We could use a prefix to the one used in gnttab.

Maybe call it XENGNTTAB_BUILD_BUG_ON for now?

All these are just cosmetic. You can modify my patch to use the name
above and continue your work. When we finally decide what it should be
called you can easily modify your code to adapt.

Wei.

> 
> Paulina

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

Re: [Xen-devel] [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)

[Andrew Cooper]

On 13/07/16 12:23, Corneliu ZUZU wrote:
> Following Andrew Cooper's suggestion, create a common-side  to
> establish, among others, prototypes of atomic functions called from 
> common-code.
> Done to avoid introducing inconsistencies between arch-side 
> headers when we make subtle changes to one of them.
>
> Some arm-side macros had to be turned into inline functions in the process.
>
> Also includes a minor adjustment asm-x86/atomic.h: reorder 
> atomic_inc_and_test()
> to follow after atomic_inc().

Suggested-by: Andrew Cooper 


> Signed-off-by: Corneliu ZUZU 

Thanks for doing this!

> diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
> new file mode 100644
> index 000..5d5c051
> --- /dev/null
> +++ b/xen/include/xen/atomic.h
> @@ -0,0 +1,155 @@
> +/*
> + * include/xen/atomic.h
> + *
> + * Common atomic operations entities (atomic_t, function prototypes).
> + * Include _from_ arch-side .
> + *
> + * Copyright (c) 2016 Bitdefender S.R.L.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program; If not, see .
> + */
> +
> +#ifndef __XEN_ATOMIC_H__
> +#define __XEN_ATOMIC_H__
> +
> +/*
> + * NB. I've pushed the volatile qualifier into the operations. This allows
> + * fast accessors such as _atomic_read() and _atomic_set() which don't give
> + * the compiler a fit.
> + */

I would recommend simply dropping this paragraph.  Is is very out of date.

> +typedef struct { int counter; } atomic_t;
> +
> +#define ATOMIC_INIT(i) { (i) }
> +
> +/**
> + * atomic_read - read atomic variable
> + * @v: pointer of type atomic_t
> + *
> + * Atomically reads the value of @v.
> + */
> +static inline int atomic_read(atomic_t *v);
> +
> +/**
> + * _atomic_read - read atomic variable non-atomically
> + * @v atomic_t
> + *
> + * Non-atomically reads the value of @v
> + */
> +static inline int _atomic_read(atomic_t v);
> +
> +/**
> + * atomic_set - set atomic variable
> + * @v: pointer of type atomic_t
> + * @i: required value
> + *
> + * Atomically sets the value of @v to @i.
> + */
> +static inline void atomic_set(atomic_t *v, int i);
> +
> +/**
> + * _atomic_set - set atomic variable non-atomically
> + * @v: pointer of type atomic_t
> + * @i: required value
> + *
> + * Non-atomically sets the value of @v to @i.
> + */
> +static inline void _atomic_set(atomic_t *v, int i);
> +

/**
 * atomic_cmpxchg - compare and exchange an atomic variable
 *...
 */

> +static inline int atomic_cmpxchg(atomic_t *v, int old, int new);

With those changes, Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH 2/2] fix: make atomic_read() param const

[Andrew Cooper]

On 13/07/16 12:23, Corneliu ZUZU wrote:
> This wouldn't let me make a param of a function that used atomic_read() const.
>
> Signed-off-by: Corneliu ZUZU 

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

Re: [Xen-devel] [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)

[Julien Grall]

Hi Corneliu,

On 13/07/2016 12:23, Corneliu ZUZU wrote:

Following Andrew Cooper's suggestion, create a common-side  to
establish, among others, prototypes of atomic functions called from common-code.
Done to avoid introducing inconsistencies between arch-side 
headers when we make subtle changes to one of them.

Some arm-side macros had to be turned into inline functions in the process.


You forgot to mention that you moved the code from 
asm-arm/arm{32,64}/atomic.h to asm-arm/arm.h.


Furthermore, this change should really be a separate patch.



Also includes a minor adjustment asm-x86/atomic.h: reorder atomic_inc_and_test()
to follow after atomic_inc().

Signed-off-by: Corneliu ZUZU 


[...]


diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 29ab265..8e8c5d1 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -2,6 +2,7 @@
 #define __ARCH_ARM_ATOMIC__

 #include 
+#include 
 #include 
 #include 

@@ -95,15 +96,6 @@ void __bad_atomic_size(void);
 default: __bad_atomic_size(); break;\
 }   \
 })
-
-/*
- * NB. I've pushed the volatile qualifier into the operations. This allows
- * fast accessors such as _atomic_read() and _atomic_set() which don't give
- * the compiler a fit.
- */
-typedef struct { int counter; } atomic_t;
-
-#define ATOMIC_INIT(i) { (i) }

 /*
  * On ARM, ordinary assignment (str instruction) doesn't clear the local
@@ -138,6 +130,85 @@ static inline void _atomic_set(atomic_t *v, int i)
 # error "unknown ARM variant"
 #endif

+#define atomic_inc_return(v)(atomic_add_return(1, v))
+#define atomic_dec_return(v)(atomic_sub_return(1, v))
+
+/**
+ * atomic_sub_and_test - subtract value from variable and test result
+ * @i: integer value to subtract
+ * @v: pointer of type atomic_t
+ *
+ * Atomically subtracts @i from @v and returns
+ * true if the result is zero, or false for all
+ * other cases.
+ */
+static inline int atomic_sub_and_test(int i, atomic_t *v)
+{
+return 0 == atomic_sub_return(i, v);
+}


Please don't use yoda condition. It's less readable and compiler have 
safety nowadays to prevent using "=".



+
+/**
+ * atomic_inc - increment atomic variable
+ * @v: pointer of type atomic_t
+ *
+ * Atomically increments @v by 1.
+ */
+static inline void atomic_inc(atomic_t *v)
+{
+atomic_add(1, v);
+}


Regards,


--
Julien Grall

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

Re: [Xen-devel] [PATCH] libs/gnttab: introduce BUILD_BUG_ON

[Andrew Cooper]

On 13/07/16 13:07, Wei Liu wrote:
> On Wed, Jul 13, 2016 at 01:32:10PM +0200, Paulina Szubarczyk wrote:
>>
>> On 07/13/2016 11:25 AM, Wei Liu wrote:
>>> The implementation is taken from libxc.
>>>
>>> Signed-off-by: Wei Liu 
>>> ---
>>> Cc: Ian Jackson 
>>> Cc: Paulina Szubarczyk 
>>>
>>> I could have put it in a header file accessible to all libraries under
>>> libs but this construct is only relevant to xengnttab library at the
>>> moment so it's put under gnttab/private.h. It can be easily moved to
>>> a common place when other libraries under libs require it.
>>>
>>> This patch is necessary to unblock Paulina on her gnttab copy work.
>>> ---
>>>  tools/libs/gnttab/private.h | 7 +++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
>>> index d286c86..da487f2 100644
>>> --- a/tools/libs/gnttab/private.h
>>> +++ b/tools/libs/gnttab/private.h
>>> @@ -4,6 +4,13 @@
>>>  #include 
>>>  #include 
>>>
>>> +/* Force a compilation error if condition is true */
>>> +#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
>>> +#define BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
>>> +#else
>>> +#define BUILD_BUG_ON(p) ((void)sizeof(struct { int:-!!(p); }))
>>> +#endif
>>> +
>>>  struct xengntdev_handle {
>>>  xentoollog_logger *logger, *logger_tofree;
>>>  int fd;
>>>
>> Hi Wei,
>>
>> thank you for the help with the grant copy patch. I have just rebuild Xen
>> and there is a conflict with definition of BUILD_BUG_ON for mini-os build:
>>
>> In file included from minios.c:33:0:
>> private.h:9:0: error: "BUILD_BUG_ON" redefined [-Werror]
>>  #define BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
>>  ^
>> In file included from minios.c:25:0:
>> xen/extras/mini-os-remote/include/lib.h:58:0: note: this is the location of
>> the previous definition
>>  #define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); })
>>  ^
>> I thought about wrapping the code in #ifndef BUILD_BUG_ON #endif.
> We could use a prefix to the one used in gnttab.
>
> Maybe call it XENGNTTAB_BUILD_BUG_ON for now?

The contents of gnttabs private.h is entirely up to gnttab.

It is MiniOS (who is poking about in someone elses private.h) with the
responsibility not to have a broken build.  It needs to do this by not
leaking so much stuff into its global namespace.

~Andrew

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

Re: [Xen-devel] [PATCH] libs/gnttab: introduce BUILD_BUG_ON

[Wei Liu]

On Wed, Jul 13, 2016 at 01:16:51PM +0100, Andrew Cooper wrote:
> On 13/07/16 13:07, Wei Liu wrote:
> > On Wed, Jul 13, 2016 at 01:32:10PM +0200, Paulina Szubarczyk wrote:
> >>
> >> On 07/13/2016 11:25 AM, Wei Liu wrote:
> >>> The implementation is taken from libxc.
> >>>
> >>> Signed-off-by: Wei Liu 
> >>> ---
> >>> Cc: Ian Jackson 
> >>> Cc: Paulina Szubarczyk 
> >>>
> >>> I could have put it in a header file accessible to all libraries under
> >>> libs but this construct is only relevant to xengnttab library at the
> >>> moment so it's put under gnttab/private.h. It can be easily moved to
> >>> a common place when other libraries under libs require it.
> >>>
> >>> This patch is necessary to unblock Paulina on her gnttab copy work.
> >>> ---
> >>>  tools/libs/gnttab/private.h | 7 +++
> >>>  1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
> >>> index d286c86..da487f2 100644
> >>> --- a/tools/libs/gnttab/private.h
> >>> +++ b/tools/libs/gnttab/private.h
> >>> @@ -4,6 +4,13 @@
> >>>  #include 
> >>>  #include 
> >>>
> >>> +/* Force a compilation error if condition is true */
> >>> +#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
> >>> +#define BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
> >>> +#else
> >>> +#define BUILD_BUG_ON(p) ((void)sizeof(struct { int:-!!(p); }))
> >>> +#endif
> >>> +
> >>>  struct xengntdev_handle {
> >>>  xentoollog_logger *logger, *logger_tofree;
> >>>  int fd;
> >>>
> >> Hi Wei,
> >>
> >> thank you for the help with the grant copy patch. I have just rebuild Xen
> >> and there is a conflict with definition of BUILD_BUG_ON for mini-os build:
> >>
> >> In file included from minios.c:33:0:
> >> private.h:9:0: error: "BUILD_BUG_ON" redefined [-Werror]
> >>  #define BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
> >>  ^
> >> In file included from minios.c:25:0:
> >> xen/extras/mini-os-remote/include/lib.h:58:0: note: this is the location of
> >> the previous definition
> >>  #define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); })
> >>  ^
> >> I thought about wrapping the code in #ifndef BUILD_BUG_ON #endif.
> > We could use a prefix to the one used in gnttab.
> >
> > Maybe call it XENGNTTAB_BUILD_BUG_ON for now?
> 
> The contents of gnttabs private.h is entirely up to gnttab.
> 
> It is MiniOS (who is poking about in someone elses private.h) with the
> responsibility not to have a broken build.  It needs to do this by not
> leaking so much stuff into its global namespace.
> 

Yeah, I know. It wouldn't hurt to prefix the name anyway.

The mini-os build system problem can be solve separately.

Wei.

> ~Andrew

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

Re: [Xen-devel] xenstored memory leak

[Juergen Gross]

On 06/07/16 09:31, Juergen Gross wrote:
> While testing some patches for support of ballooning in Mini-OS by using
> the xenstore domain I realized that each xl create/destroy pair would
> increase memory consumption in Mini-OS by about 5kB. Wondering whether
> this is a xenstore domain only effect I did the same test with xenstored
> and oxenstored daemons.
> 
> xenstored showed the same behavior, the "referenced" size showed by the
> pmap command grew by about 5kB for each create/destroy pair.
> 
> oxenstored seemed to be even worse in the beginning (about 6kB for each
> pair), but after about 100 create/destroys the value seemed to be
> rather stable.
> 
> Did anyone notice this memory leak before?

I think I've found the problem:

qemu as the device model is setting up a xenstore watch for each backend
type it is supporting. Unfortunately those watches are never removed
again. This sums up to the observed memory leak.

I'm not sure how oxenstored is avoiding the problem, may be by testing
socket connections to be still alive and so detecting qemu has gone.
OTOH this won't help for oxenstored running in another domain than the
device model (either due to oxenstore-stubdom, or a driver domain with
a qemu based device model).

I'll post a qemu patch to remove those watches on exit soon.

To find the problem I've added some debug aid to xenstored: when
a special parameter is specified on invocation it will dump its memory
allocation structure via talloc_report_full() to a file whenever it is
receiving a SIGUSR1 signal. Anybody interested in this patch?


Juergen

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

[Xen-devel] [PATCH 0/2] Allow crash dumps with crash_kexec_post_notifiers

[Petr Tesarik]

Hello all,

this patch series makes it possible to save a kernel crash dump when the
kernel command line includes "crash_kexec_post_notifiers". There might
be other approaches, but mine has the advantage that no new sysctl is
required, and the behaviour is the same whether panic notifiers are run
or not: If you load a crash kernel with kexec, it will be used, otherwise
the hypervisor facility is used (using a hypercall).

Feedback welcome!

Petr T

---

Petr Tesarik (2):
  Add a kexec_crash_loaded() function
  Allow kdump with crash_kexec_post_notifiers


 arch/x86/xen/enlighten.c |3 ++-
 include/linux/kexec.h|2 ++
 kernel/kexec_core.c  |5 +
 kernel/ksysfs.c  |2 +-
 4 files changed, 10 insertions(+), 2 deletions(-)

--
Signature

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

[Xen-devel] [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Petr Tesarik]

If a crash kernel is loaded, do not crash the running domain. This is
needed if the kernel is loaded with crash_kexec_post_notifiers, because
panic notifiers are run before __crash_kexec() in that case, and this
Xen hook prevents its being called later.

Signed-off-by: Petr Tesarik 
---
 arch/x86/xen/enlighten.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 760789a..6e3e7c6 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1325,7 +1325,8 @@ static void xen_crash_shutdown(struct pt_regs *regs)
 static int
 xen_panic_event(struct notifier_block *this, unsigned long event, void *ptr)
 {
-   xen_reboot(SHUTDOWN_crash);
+   if (!kexec_crash_loaded())
+   xen_reboot(SHUTDOWN_crash);
return NOTIFY_DONE;
 }
 


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

Re: [Xen-devel] [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)

[Corneliu ZUZU]

On 7/13/2016 3:12 PM, Andrew Cooper wrote:

On 13/07/16 12:23, Corneliu ZUZU wrote:

Following Andrew Cooper's suggestion, create a common-side  to
establish, among others, prototypes of atomic functions called from common-code.
Done to avoid introducing inconsistencies between arch-side 
headers when we make subtle changes to one of them.

Some arm-side macros had to be turned into inline functions in the process.

Also includes a minor adjustment asm-x86/atomic.h: reorder atomic_inc_and_test()
to follow after atomic_inc().

Suggested-by: Andrew Cooper 


Nice, didn't know that.





Signed-off-by: Corneliu ZUZU 

Thanks for doing this!


diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
new file mode 100644
index 000..5d5c051
--- /dev/null
+++ b/xen/include/xen/atomic.h
@@ -0,0 +1,155 @@
+/*
+ * include/xen/atomic.h
+ *
+ * Common atomic operations entities (atomic_t, function prototypes).
+ * Include _from_ arch-side .
+ *
+ * Copyright (c) 2016 Bitdefender S.R.L.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; If not, see .
+ */
+
+#ifndef __XEN_ATOMIC_H__
+#define __XEN_ATOMIC_H__
+
+/*
+ * NB. I've pushed the volatile qualifier into the operations. This allows
+ * fast accessors such as _atomic_read() and _atomic_set() which don't give
+ * the compiler a fit.
+ */

I would recommend simply dropping this paragraph.  Is is very out of date.


Ack.




+typedef struct { int counter; } atomic_t;
+
+#define ATOMIC_INIT(i) { (i) }
+
+/**
+ * atomic_read - read atomic variable
+ * @v: pointer of type atomic_t
+ *
+ * Atomically reads the value of @v.
+ */
+static inline int atomic_read(atomic_t *v);
+
+/**
+ * _atomic_read - read atomic variable non-atomically
+ * @v atomic_t
+ *
+ * Non-atomically reads the value of @v
+ */
+static inline int _atomic_read(atomic_t v);
+
+/**
+ * atomic_set - set atomic variable
+ * @v: pointer of type atomic_t
+ * @i: required value
+ *
+ * Atomically sets the value of @v to @i.
+ */
+static inline void atomic_set(atomic_t *v, int i);
+
+/**
+ * _atomic_set - set atomic variable non-atomically
+ * @v: pointer of type atomic_t
+ * @i: required value
+ *
+ * Non-atomically sets the value of @v to @i.
+ */
+static inline void _atomic_set(atomic_t *v, int i);
+

/**
  * atomic_cmpxchg - compare and exchange an atomic variable
  *...
  */


Ack.




+static inline int atomic_cmpxchg(atomic_t *v, int old, int new);

With those changes, Reviewed-by: Andrew Cooper 


Thanks too for the prompt review,
Zuzu C.

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

[Xen-devel] [PATCH 1/2] Add a kexec_crash_loaded() function

[Petr Tesarik]

Provide a wrapper function to be used by kernel code to check whether
a crash kernel is loaded. It returns the same value that can be seen
in /sys/kernel/kexec_crash_loaded by userspace programs.

I'm exporting the function, because it will be used by Xen, and it is
possible to compile Xen modules separately to enable the use of PV
drivers with unmodified bare-metal kernels.

Signed-off-by: Petr Tesarik 
---
 include/linux/kexec.h |2 ++
 kernel/kexec_core.c   |6 ++
 kernel/ksysfs.c   |2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index e8acb2b..d461d02 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -228,6 +228,7 @@ extern void *kexec_purgatory_get_symbol_addr(struct kimage 
*image,
 extern void __crash_kexec(struct pt_regs *);
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
+int kexec_crash_loaded(void);
 void crash_save_cpu(struct pt_regs *regs, int cpu);
 void crash_save_vmcoreinfo(void);
 void arch_crash_save_vmcoreinfo(void);
@@ -324,6 +325,7 @@ struct task_struct;
 static inline void __crash_kexec(struct pt_regs *regs) { }
 static inline void crash_kexec(struct pt_regs *regs) { }
 static inline int kexec_should_crash(struct task_struct *p) { return 0; }
+static inline int kexec_crash_loaded(void) { return 0; }
 #define kexec_in_progress false
 #endif /* CONFIG_KEXEC_CORE */
 
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 56b3ed0..a303dce 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -95,6 +95,12 @@ int kexec_should_crash(struct task_struct *p)
return 0;
 }
 
+int kexec_crash_loaded(void)
+{
+   return !!kexec_crash_image;
+}
+EXPORT_SYMBOL_GPL(kexec_crash_loaded);
+
 /*
  * When kexec transitions to the new kernel there is a one-to-one
  * mapping between physical and virtual addresses.  On processors
diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
index 152da4a..bf9fc9d 100644
--- a/kernel/ksysfs.c
+++ b/kernel/ksysfs.c
@@ -101,7 +101,7 @@ KERNEL_ATTR_RO(kexec_loaded);
 static ssize_t kexec_crash_loaded_show(struct kobject *kobj,
   struct kobj_attribute *attr, char *buf)
 {
-   return sprintf(buf, "%d\n", !!kexec_crash_image);
+   return sprintf(buf, "%d\n", kexec_crash_loaded());
 }
 KERNEL_ATTR_RO(kexec_crash_loaded);
 


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

Re: [Xen-devel] [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)

[Corneliu ZUZU]

Hi Julien,

On 7/13/2016 3:14 PM, Julien Grall wrote:

Hi Corneliu,

On 13/07/2016 12:23, Corneliu ZUZU wrote:
Following Andrew Cooper's suggestion, create a common-side 
 to
establish, among others, prototypes of atomic functions called from 
common-code.
Done to avoid introducing inconsistencies between arch-side 


headers when we make subtle changes to one of them.

Some arm-side macros had to be turned into inline functions in the 
process.


You forgot to mention that you moved the code from 
asm-arm/arm{32,64}/atomic.h to asm-arm/arm.h.


Furthermore, this change should really be a separate patch.


Noted, will do.





Also includes a minor adjustment asm-x86/atomic.h: reorder 
atomic_inc_and_test()

to follow after atomic_inc().

Signed-off-by: Corneliu ZUZU 


[...]


diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 29ab265..8e8c5d1 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -2,6 +2,7 @@
 #define __ARCH_ARM_ATOMIC__

 #include 
+#include 
 #include 
 #include 

@@ -95,15 +96,6 @@ void __bad_atomic_size(void);
 default: __bad_atomic_size(); 
break;\

} \
 })
-
-/*
- * NB. I've pushed the volatile qualifier into the operations. This 
allows
- * fast accessors such as _atomic_read() and _atomic_set() which 
don't give

- * the compiler a fit.
- */
-typedef struct { int counter; } atomic_t;
-
-#define ATOMIC_INIT(i) { (i) }

 /*
  * On ARM, ordinary assignment (str instruction) doesn't clear the 
local

@@ -138,6 +130,85 @@ static inline void _atomic_set(atomic_t *v, int i)
 # error "unknown ARM variant"
 #endif

+#define atomic_inc_return(v)(atomic_add_return(1, v))
+#define atomic_dec_return(v)(atomic_sub_return(1, v))
+
+/**
+ * atomic_sub_and_test - subtract value from variable and test result
+ * @i: integer value to subtract
+ * @v: pointer of type atomic_t
+ *
+ * Atomically subtracts @i from @v and returns
+ * true if the result is zero, or false for all
+ * other cases.
+ */
+static inline int atomic_sub_and_test(int i, atomic_t *v)
+{
+return 0 == atomic_sub_return(i, v);
+}


Please don't use yoda condition. It's less readable and compiler have 
safety nowadays to prevent using "=".


Oh yeah, given that Xen's compiled with -Wall -Werror that's not 
necessary. Ack.





+
+/**
+ * atomic_inc - increment atomic variable
+ * @v: pointer of type atomic_t
+ *
+ * Atomically increments @v by 1.
+ */
+static inline void atomic_inc(atomic_t *v)
+{
+atomic_add(1, v);
+}


Regards,


Thanks,
Corneliu.

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

Re: [Xen-devel] [PATCH 0/2] Allow crash dumps with crash_kexec_post_notifiers

[Petr Tesarik]

On Wed, 13 Jul 2016 14:19:50 +0200
Petr Tesarik  wrote:

> Hello all,
> 
> this patch series makes it possible to save a kernel crash dump when the
> kernel command line includes "crash_kexec_post_notifiers".

Oh ... I forgot to add: This only applies to running Linux under Xen.
If you run on bare metal, you can always save the dump already, as you
certainly know.

Petr T

> There might
> be other approaches, but mine has the advantage that no new sysctl is
> required, and the behaviour is the same whether panic notifiers are run
> or not: If you load a crash kernel with kexec, it will be used, otherwise
> the hypervisor facility is used (using a hypercall).
> 
> Feedback welcome!
> 
> Petr T
> 
> ---
> 
> Petr Tesarik (2):
>   Add a kexec_crash_loaded() function
>   Allow kdump with crash_kexec_post_notifiers
> 
> 
>  arch/x86/xen/enlighten.c |3 ++-
>  include/linux/kexec.h|2 ++
>  kernel/kexec_core.c  |5 +
>  kernel/ksysfs.c  |2 +-
>  4 files changed, 10 insertions(+), 2 deletions(-)
> 
> --
> Signature
> 
> ___
> kexec mailing list
> ke...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec


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

[Xen-devel] [PATCH] xen: remove xenstore watches of backends when terminating qemu

[Juergen Gross]

Xenstore watches of the /local/domain//backend/ directories
are never removed. This can lead to a memory leak in xenstored,
especially when xenstored is running in another domain (this will be
the case either for a system with xenstore-stubdom, or with driver
domains running qemu-based backends).

Avoid this problem by calling xs_unwatch() for these directories when
terminating qemu.

Signed-off-by: Juergen Gross 
---
 hw/xen/xen_backend.c | 28 ++--
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index bab79b1..6413475 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -52,6 +52,13 @@ struct xs_dirs {
 };
 static QTAILQ_HEAD(xs_dirs_head, xs_dirs) xs_cleanup =
 QTAILQ_HEAD_INITIALIZER(xs_cleanup);
+struct xs_watches {
+char path[XEN_BUFSIZE];
+char token[XEN_BUFSIZE];
+QTAILQ_ENTRY(xs_watches) list;
+};
+static QTAILQ_HEAD(xs_watches_head, xs_watches) xs_w_cleanup =
+QTAILQ_HEAD_INITIALIZER(xs_w_cleanup);
 
 static QTAILQ_HEAD(XenDeviceHead, XenDevice) xendevs = 
QTAILQ_HEAD_INITIALIZER(xendevs);
 static int debug = 0;
@@ -70,10 +77,14 @@ static void xenstore_cleanup_dir(char *dir)
 void xen_config_cleanup(void)
 {
 struct xs_dirs *d;
+struct xs_watches *w;
 
 QTAILQ_FOREACH(d, &xs_cleanup, list) {
 xs_rm(xenstore, 0, d->xs_dir);
 }
+QTAILQ_FOREACH(w, &xs_w_cleanup, list) {
+xs_unwatch(xenstore, w->path, w->token);
+}
 }
 
 int xenstore_write_str(const char *base, const char *node, const char *val)
@@ -629,20 +640,25 @@ void xen_be_check_state(struct XenDevice *xendev)
 static int xenstore_scan(const char *type, int dom, struct XenDevOps *ops)
 {
 struct XenDevice *xendev;
-char path[XEN_BUFSIZE], token[XEN_BUFSIZE];
+struct xs_watches *w;
 char **dev = NULL;
 unsigned int cdev, j;
 
 /* setup watch */
-snprintf(token, sizeof(token), "be:%p:%d:%p", type, dom, ops);
-snprintf(path, sizeof(path), "backend/%s/%d", type, dom);
-if (!xs_watch(xenstore, path, token)) {
-xen_be_printf(NULL, 0, "xen be: watching backend path (%s) failed\n", 
path);
+w = g_malloc(sizeof(*w));
+
+snprintf(w->token, sizeof(w->token), "be:%p:%d:%p", type, dom, ops);
+snprintf(w->path, sizeof(w->path), "backend/%s/%d", type, dom);
+if (!xs_watch(xenstore, w->path, w->token)) {
+xen_be_printf(NULL, 0, "xen be: watching backend path (%s) failed\n",
+  w->path);
+g_free(w);
 return -1;
 }
+QTAILQ_INSERT_TAIL(&xs_w_cleanup, w, list);
 
 /* look for backends */
-dev = xs_directory(xenstore, 0, path, &cdev);
+dev = xs_directory(xenstore, 0, w->path, &cdev);
 if (!dev) {
 return 0;
 }
-- 
2.6.6


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

Re: [Xen-devel] [PATCH v3 2/2] qdisk - hw/block/xen_disk: grant copy implementation

[Paulina Szubarczyk]

On 06/22/2016 10:38 AM, Paulina Szubarczyk wrote:

Copy data operated on during request from/to local buffers to/from
the grant references.

Before grant copy operation local buffers must be allocated what is
done by calling ioreq_init_copy_buffers. For the 'read' operation,
first, the qemu device invokes the read operation on local buffers
and on the completion grant copy is called and buffers are freed.
For the 'write' operation grant copy is performed before invoking
write by qemu device.

A new value 'feature_grant_copy' is added to recognize when the
grant copy operation is supported by a guest.
The body of the function 'ioreq_runio_qemu_aio' is moved to
'ioreq_runio_qemu_aio_blk' and in the 'ioreq_runio_qemu_aio' depending
on the support for grant copy according checks, initialization, grant
operation are made, then the 'ioreq_runio_qemu_aio_blk' function is
called.

Signed-off-by: Paulina Szubarczyk 
---
Changes since v2:
- to use the xengnttab_* function directly added -lxengnttab to configure
   and include  in include/hw/xen/xen_common.h
- in ioreq_copy removed an out path, changed a log level, made explicit
   assignement to 'xengnttab_copy_grant_segment'
* I did not change the way of testing if grant_copy operation is implemented.
   As far as I understand if the code from gnttab_unimp.c is used then the 
gnttab
   device is unavailable and the handler to gntdev would be invalid. But
   if the handler is valid then the ioctl should return operation unimplemented
   if the gntdev does not implement the operation.

  configure   |   2 +-
  hw/block/xen_disk.c | 171 
  include/hw/xen/xen_common.h |   2 +
  3 files changed, 162 insertions(+), 13 deletions(-)

diff --git a/configure b/configure
index e41876a..355d3fa 100755
--- a/configure
+++ b/configure
@@ -1843,7 +1843,7 @@ fi
  # xen probe

  if test "$xen" != "no" ; then
-  xen_libs="-lxenstore -lxenctrl -lxenguest"
+  xen_libs="-lxenstore -lxenctrl -lxenguest -lxengnttab"

# First we test whether Xen headers and libraries are available.
# If no, we are done and there is no Xen support.
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 37e14d1..4eca06a 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -131,6 +131,9 @@ struct XenBlkDev {
  unsigned intpersistent_gnt_count;
  unsigned intmax_grants;

+/* Grant copy */
+gbooleanfeature_grant_copy;
+
  /* qemu block driver */
  DriveInfo   *dinfo;
  BlockBackend*blk;
@@ -500,6 +503,99 @@ static int ioreq_map(struct ioreq *ioreq)
  return 0;
  }

+static void* get_buffer(int count)
+{
+return xc_memalign(xen_xc, XC_PAGE_SIZE, count*XC_PAGE_SIZE);
+}
+
+static void free_buffers(struct ioreq *ioreq)
+{
+int i;
+
+for (i = 0; i < ioreq->v.niov; i++) {
+ioreq->page[i] = NULL;
+}
+
+free(ioreq->pages);
+}
+
+static int ioreq_init_copy_buffers(struct ioreq *ioreq) {
+int i;
+
+if (ioreq->v.niov == 0) {
+return 0;
+}
+
+ioreq->pages = get_buffer(ioreq->v.niov);
+if (!ioreq->pages) {
+return -1;
+}
+
+for (i = 0; i < ioreq->v.niov; i++) {
+ioreq->page[i] = ioreq->pages + i*XC_PAGE_SIZE;
+ioreq->v.iov[i].iov_base += (uintptr_t)ioreq->page[i];
+}
+
+return 0;
+}
+
+static int ioreq_copy(struct ioreq *ioreq)
+{
+XenGnttab gnt = ioreq->blkdev->xendev.gnttabdev;
+xengnttab_grant_copy_segment_t segs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+int i, count = 0, r, rc;
+int64_t file_blk = ioreq->blkdev->file_blk;
+
+if (ioreq->v.niov == 0) {
+return 0;
+}
+
+count = ioreq->v.niov;
+
+for (i = 0; i < count; i++) {
+
+if (ioreq->req.operation == BLKIF_OP_READ) {
+segs[i].flags = GNTCOPY_dest_gref;
+segs[i].dest.foreign.ref = ioreq->refs[i];
+segs[i].dest.foreign.domid = ioreq->domids[i];
+segs[i].dest.foreign.offset = ioreq->req.seg[i].first_sect * 
file_blk;
+segs[i].source.virt = ioreq->v.iov[i].iov_base;
+} else {
+segs[i].flags = GNTCOPY_source_gref;
+segs[i].source.foreign.ref = ioreq->refs[i];
+segs[i].source.foreign.domid = ioreq->domids[i];
+segs[i].source.foreign.offset = ioreq->req.seg[i].first_sect * 
file_blk;
+segs[i].dest.virt = ioreq->v.iov[i].iov_base;
+}
+segs[i].len = (ioreq->req.seg[i].last_sect
+   - ioreq->req.seg[i].first_sect + 1) * file_blk;
+
+}
+
+rc = xengnttab_grant_copy(gnt, count, segs);
+
+if (rc) {
+xen_be_printf(&ioreq->blkdev->xendev, 0,
+  "failed to copy data %d \n", rc);
+ioreq->aio_errors++;
+return -1;
+} else {
+r = 0;
+}
+
+for (i = 0; i < count; i++) {
+if (segs[i].status != GNTST_okay) {
+xen_be_printf(&ioreq->blkd

Re: [Xen-devel] xenstored memory leak

[Andrew Cooper]

On 13/07/16 13:21, Juergen Gross wrote:
> On 06/07/16 09:31, Juergen Gross wrote:
>> While testing some patches for support of ballooning in Mini-OS by using
>> the xenstore domain I realized that each xl create/destroy pair would
>> increase memory consumption in Mini-OS by about 5kB. Wondering whether
>> this is a xenstore domain only effect I did the same test with xenstored
>> and oxenstored daemons.
>>
>> xenstored showed the same behavior, the "referenced" size showed by the
>> pmap command grew by about 5kB for each create/destroy pair.
>>
>> oxenstored seemed to be even worse in the beginning (about 6kB for each
>> pair), but after about 100 create/destroys the value seemed to be
>> rather stable.
>>
>> Did anyone notice this memory leak before?
> I think I've found the problem:
>
> qemu as the device model is setting up a xenstore watch for each backend
> type it is supporting. Unfortunately those watches are never removed
> again. This sums up to the observed memory leak.
>
> I'm not sure how oxenstored is avoiding the problem, may be by testing
> socket connections to be still alive and so detecting qemu has gone.
> OTOH this won't help for oxenstored running in another domain than the
> device model (either due to oxenstore-stubdom, or a driver domain with
> a qemu based device model).

I do seem to remember something along those lines.

>
> I'll post a qemu patch to remove those watches on exit soon.

That is good, but there needs to be further consideration as to how to
clean up after a crashed qemu/etc.

>
> To find the problem I've added some debug aid to xenstored: when
> a special parameter is specified on invocation it will dump its memory
> allocation structure via talloc_report_full() to a file whenever it is
> receiving a SIGUSR1 signal. Anybody interested in this patch?

Sounds useful.  +1

~Andrew

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

[Xen-devel] [xen-unstable-smoke test] 97261: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt 11 guest-start   fail REGR. vs. 96794
 test-amd64-amd64-xl-qemuu-debianhvm-i386 9 debian-hvm-install fail REGR. vs. 
96794
 build-armhf   5 xen-build fail REGR. vs. 96794

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a

version targeted for testing:
 xen  1fb0fe97a647549cc1bbe90f15b6cb2fcc3f79b6
baseline version:
 xen  7da483b0236d8974cc97f81780dcf8e559a63175

Last test of basis96794  2016-07-08 15:01:26 Z4 days
Testing same since97261  2016-07-13 11:00:54 Z0 days1 attempts


People who touched revisions under test:
  Corneliu ZUZU 
  Ian Jackson 
  Juergen Gross 
  Julien Grall 
  Kevin Tian 
  Stefano Stabellini 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  fail
 build-amd64-libvirt  pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 test-armhf-armhf-xl  blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-i386 fail
 test-amd64-amd64-libvirt fail



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

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

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

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


Not pushing.


commit 1fb0fe97a647549cc1bbe90f15b6cb2fcc3f79b6
Author: Corneliu ZUZU 
Date:   Sat Jul 9 07:12:07 2016 +0300

x86/vmx_update_guest_cr: minor optimization

Minor optimization @ vmx_update_guest_cr: checks if 
v->arch.hvm_vmx.exec_control
was modified before actually calling vmx_update_cpu_exec_control(v).

Signed-off-by: Corneliu ZUZU 
Acked-by: Kevin Tian 

commit 22ea8ad02e465e32cd40887c750b55c3a997a288
Author: Juergen Gross 
Date:   Wed Jul 6 16:55:34 2016 +0200

libxl: move DEFINE_DEVICE* macros to libxl_internal.h

In order to be able to have all functions related to a device type in
a single source file move the macros used to generate device type
specific functions to libxl_internal.h. Rename the macros as they are
no longer local to a source file. While at it hide device remove and
device destroy in one macro as those are always used in pairs. Move
usage of the macros to the appropriate source files.

Signed-off-by: Juergen Gross 
Acked-by: Ian Jackson 

commit 2c2c33f038c889acc9f73681c09320164516da47
Author: Juergen Gross 
Date:   Wed Jul 6 16:55:33 2016 +0200

libxl: refactor domcreate_attach_dtdev() to use device type framework

Signed-off-by: Juergen Gross 
Acked-by: Ian Jackson 

commit 26021d77dc14d441ff3b0094f2fafcc5d18aa599
Author: Juergen Gross 
Date:   Wed Jul 6 16:55:32 2016 +0200

libxl: refactor domcreate_attach_pci() to use device type framework

Signed-off-by: Juergen Gross 
Acked-by: Ian Jackson 

commit 74e857c6c7f9dc106fa7a7bbf49a9729f5841ad9
Author: Juergen Gross 
Date:   Wed Jul 6 16:55:31 2016 +0200

libxl: add framework for device types

Instead of duplicate coding for each device type (vtpms, usbctrls, ...)
especially on domain creation introduce a framework for that purpose.

Signed-off-by: Juergen Gross 
Acked-by: Ian Jackson 

commit 56bac262e097684b20f7753ceb6debe594e9725c
Author: Wei Liu 
Date:   Wed Jun 8 15:01:02 2016 +0100

libxl: only issue cpu-add call to QEMU for not present CPU

Calculate the final bitmap for CPUs to add to avoid having annoying
error messages complaining those CPUs are already present. Example
message is like (wrapped):

libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
error message from QMP server: Unable to add CPU: 0, it already exists

We can also properly handle error from QMP now.

Signed-off-by: Wei Liu 
Reviewed-by: Anthony PERARD 
Acked-by: Ian Jackson 

commit 01f3193e2e3b9b36bde027f909db496f7211c320
Author: Wei Liu 
Date:   Fri Jun 3 16:38:32 2016 +01

Re: [Xen-devel] [PATCH 1/2] Add a kexec_crash_loaded() function

[Josh Triplett]

On Wed, Jul 13, 2016 at 02:19:55PM +0200, Petr Tesarik wrote:
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -95,6 +95,12 @@ int kexec_should_crash(struct task_struct *p)
>   return 0;
>  }
>  
> +int kexec_crash_loaded(void)
> +{
> + return !!kexec_crash_image;
> +}

Nit: this function should return bool rather than int, since it
effectively returns true/false.

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

Re: [Xen-devel] [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[David Vrabel]

On 13/07/16 13:20, Petr Tesarik wrote:
> If a crash kernel is loaded, do not crash the running domain. This is
> needed if the kernel is loaded with crash_kexec_post_notifiers, because
> panic notifiers are run before __crash_kexec() in that case, and this
> Xen hook prevents its being called later.

Prioritising the in-kernel kexec image over the hypervisor one seems
sensible behaviour to me.

Reviewed-by: David Vrabel 

David

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

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

[Ian Jackson]

osstest service owner writes ("[xen-unstable-smoke test] 97261: regressions - 
FAIL"):
> flight 97261 xen-unstable-smoke real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/97261/
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  build-armhf   5 xen-buildfail REGR. vs. 96794

This is a real bug:

  libxl_nocpuid.c:59:6: error: conflicting types for
  'libxl_cpuid_policy_list_copy'

Probably due to 11316d31d684 "libxl: constify copy and length
calculation functions" ?

>  test-amd64-amd64-libvirt 11 guest-startfail REGR. vs. 96794

^ this is fallout from the new stricter firewall rules:

2016-07-13 12:02:30 Z FAILURE: guest debian.guest.osstest
5a:36:0e:ed:00:02 22 link/ip/tcp: wait timed out: connect to
infra:5556: Connection refused.

Because many other tests will be fail for similar reasons, I am going
to abort all currently running flights.  There will be a flurry of
emails reporting these unfinished flights.

Ian.

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

[Xen-devel] [PATCH] XSM-Policy: allow source domain access to setpodtarget for ballooning.

[Anshul Makkar]

Access to setpodtarget is required by dom0 to set the balloon targets for
domU. The patch gives source domain (dom0) access to set this target for
domU and resolve the following permission denied error message during
ballooning :
avc:  denied  { setpodtarget } for domid=0 target=9
scontext=system_u:system_r:dom0_t
tcontext=system_u:system_r:domU_t tclass=domain

Signed-off-by: Anshul Makkar 
---
---
 tools/flask/policy/modules/xen.if | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/flask/policy/modules/xen.if 
b/tools/flask/policy/modules/xen.if
index 8c43c28..8ae3c2e 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -83,7 +83,8 @@ define(`create_domain_build_label', `
 define(`manage_domain', `
allow $1 $2:domain { getdomaininfo getvcpuinfo getaffinity
getaddrsize pause unpause trigger shutdown destroy
-   setaffinity setdomainmaxmem getscheduler resume };
+   setaffinity setdomainmaxmem getscheduler resume
+   setpodtarget };
 allow $1 $2:domain2 set_vnumainfo;
 ')
 
-- 
1.9.1


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

Re: [Xen-devel] [PATCH 1/2] Add a kexec_crash_loaded() function

[Petr Tesarik]

On Wed, 13 Jul 2016 05:52:33 -0700
Josh Triplett  wrote:

> On Wed, Jul 13, 2016 at 02:19:55PM +0200, Petr Tesarik wrote:
> > --- a/kernel/kexec_core.c
> > +++ b/kernel/kexec_core.c
> > @@ -95,6 +95,12 @@ int kexec_should_crash(struct task_struct *p)
> > return 0;
> >  }
> >  
> > +int kexec_crash_loaded(void)
> > +{
> > +   return !!kexec_crash_image;
> > +}
> 
> Nit: this function should return bool rather than int, since it
> effectively returns true/false.

I thought about this for a moment. Since the return value should also
be used for the corresponding sysctl, which is an integer, I wasn't
sure if bool is the correct type, especially since other boolean
functions in kexec.h also return int... But that could be legacy.

I've got no problem changing the type to 'bool' if that's what it takes.

Petr T

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

[Xen-devel] [qemu-mainline test] 97264: trouble: pass/preparing/queued/running

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-libvirt  queued
 test-armhf-armhf-libvirt-qcow2  queued
 test-armhf-armhf-xl-arndale   queued
 test-armhf-armhf-xl   queued
 test-armhf-armhf-xl-credit2   queued
 test-armhf-armhf-xl-cubietruck  queued
 test-armhf-armhf-libvirt-xsm  queued
 test-armhf-armhf-xl-rtds  queued
 test-armhf-armhf-libvirt-raw  queued
 test-armhf-armhf-xl-xsm   queued
 test-armhf-armhf-xl-vhd   queued
 test-armhf-armhf-xl-multivcpu  queued
 test-amd64-amd64-xl-pvh-amd   2 hosts-allocate   running
 test-amd64-amd64-xl-pvh-intel  2 hosts-allocate   running
 test-amd64-amd64-xl   2 hosts-allocate   running
 test-amd64-amd64-libvirt-xsm  2 hosts-allocate   running
 test-amd64-i386-xl-xsm2 hosts-allocate   running
 test-amd64-i386-libvirt   2 hosts-allocate   running
 test-amd64-amd64-xl-credit2   2 hosts-allocate   running
 test-amd64-amd64-xl-xsm   2 hosts-allocate   running
 test-amd64-i386-xl2 hosts-allocate   running
 test-amd64-amd64-xl-multivcpu  2 hosts-allocate   running
 test-amd64-amd64-pygrub   2 hosts-allocate   running
 test-amd64-amd64-pair 2 hosts-allocate   running
 test-amd64-amd64-i386-pvgrub  2 hosts-allocate   running
 test-amd64-amd64-amd64-pvgrub  2 hosts-allocate   running
 test-amd64-amd64-libvirt-pair  2 hosts-allocate   running
 test-amd64-amd64-qemuu-nested-amd  2 hosts-allocate   running
 build-armhf-libvirt   5 libvirt-buildrunning
 test-amd64-i386-freebsd10-i386  2 hosts-allocate   running
 test-amd64-i386-qemuu-rhel6hvm-intel  2 hosts-allocate   running
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm  2 hosts-allocate   running
 test-amd64-i386-qemuu-rhel6hvm-amd  2 hosts-allocate   running
 test-amd64-i386-libvirt-pair  2 hosts-allocate   running
 test-amd64-amd64-qemuu-nested-intel  2 hosts-allocate   running
 test-amd64-i386-pair  2 hosts-allocate   running
 test-amd64-i386-libvirt-xsm   2 hosts-allocate   running
 test-amd64-amd64-xl-qemuu-win7-amd64  2 hosts-allocate   running
 test-amd64-amd64-libvirt  2 hosts-allocate   running
 test-amd64-i386-xl-qemuu-win7-amd64  2 hosts-allocate   running
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm  2 hosts-allocaterunning
 test-amd64-i386-xl-qemuu-debianhvm-amd64  2 hosts-allocate running
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm  2 hosts-allocaterunning
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  2 hosts-allocaterunning
 test-amd64-amd64-xl-qcow2 2 hosts-allocate   running
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  2 hosts-allocate running
 test-amd64-amd64-xl-qemuu-ovmf-amd64  2 hosts-allocate   running
 test-amd64-i386-freebsd10-amd64  2 hosts-allocate   running
 test-amd64-i386-xl-qemuu-ovmf-amd64  2 hosts-allocate   running
 test-amd64-amd64-libvirt-vhd  2 hosts-allocate   running
 test-amd64-i386-xl-raw2 hosts-allocate   running
 test-amd64-amd64-xl-rtds  2 hosts-allocate   running
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  2 hosts-allocate running
 build-armhf-pvops 5 kernel-build running
 test-amd64-amd64-xl-qemuu-winxpsp3  2 hosts-allocate   running
 test-amd64-i386-xl-qemuu-winxpsp3  2 hosts-allocate   running

version targeted for testing:
 qemuuca3d87d4c84032f19478010b5604cac88b045c25
baseline version:
 qemuu4f4a9ca4a4386c137301b3662faba076455ff15a

Last test of basis96791  2016-07-08 12:20:07 Z5 days
Testing same since0  1970-01-01 00:00:00 Z 16995 days0 attempts


People who touched revisions under test:
  Alex Bennée 
  Alexander Yarygin 
  Anthony PERARD 
  Ashok Raj 
  Cornelia Huck 
  Daniel P. Berrange 
  David Gibson 
  David Hildenbrand 
  Denis V. Lunev 
  Eduardo Habkost 
  Eric Blake 
  Eugene (jno) Dvurechenski 
  Evgeny Yakovlev 
  Gerd Hoffmann 
  Gonglei 
  Haibin Wang 
  Haozhong Zhang 
  Igor Mammedov 
  James Hogan 
  Jing Liu 
  Leon Alrae 
  Markus Armbruster 
  Paolo Bonzini 
  Paul Burton 
  Peter Lieven 
  Peter Maydell 
  Pierre Morel 
  Richard Henders

Re: [Xen-devel] xenstored memory leak

[Wei Liu]

On Wed, Jul 13, 2016 at 02:21:38PM +0200, Juergen Gross wrote:
> On 06/07/16 09:31, Juergen Gross wrote:
> > While testing some patches for support of ballooning in Mini-OS by using
> > the xenstore domain I realized that each xl create/destroy pair would
> > increase memory consumption in Mini-OS by about 5kB. Wondering whether
> > this is a xenstore domain only effect I did the same test with xenstored
> > and oxenstored daemons.
> > 
> > xenstored showed the same behavior, the "referenced" size showed by the
> > pmap command grew by about 5kB for each create/destroy pair.
> > 
> > oxenstored seemed to be even worse in the beginning (about 6kB for each
> > pair), but after about 100 create/destroys the value seemed to be
> > rather stable.
> > 
> > Did anyone notice this memory leak before?
> 
> I think I've found the problem:
> 
> qemu as the device model is setting up a xenstore watch for each backend
> type it is supporting. Unfortunately those watches are never removed
> again. This sums up to the observed memory leak.
> 
> I'm not sure how oxenstored is avoiding the problem, may be by testing
> socket connections to be still alive and so detecting qemu has gone.
> OTOH this won't help for oxenstored running in another domain than the
> device model (either due to oxenstore-stubdom, or a driver domain with
> a qemu based device model).
> 

How unfortunate.

My gut feeling is that xenstored shouldn't have the knowledge to
associate a watch with a "process". The concept of a process is only
meaningful to OS, which wouldn't work on cross-domain xenstored setup.
Maybe the OS xenbus driver should reap all watches on behalf the dead
process. This would also avoid a crashed QEMU leaking resources.

And xenstored should have proper quota support so that a domain can't
set up excessive numbers of watches.

> I'll post a qemu patch to remove those watches on exit soon.
> 
> To find the problem I've added some debug aid to xenstored: when
> a special parameter is specified on invocation it will dump its memory
> allocation structure via talloc_report_full() to a file whenever it is
> receiving a SIGUSR1 signal. Anybody interested in this patch?
> 

Wouldn't hurt to post to the list. If we take it we take it, if we don't
it would still be useful for people who want custom debug patch later.

Wei.

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

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

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

[Wei Liu]

On Wed, Jul 13, 2016 at 01:55:26PM +0100, Ian Jackson wrote:
> osstest service owner writes ("[xen-unstable-smoke test] 97261: regressions - 
> FAIL"):
> > flight 97261 xen-unstable-smoke real [real]
> > http://logs.test-lab.xenproject.org/osstest/logs/97261/
> > 
> > Regressions :-(
> > 
> > Tests which did not succeed and are blocking,
> > including tests which could not be run:
> >  build-armhf   5 xen-buildfail REGR. vs. 96794
> 
> This is a real bug:
> 
>   libxl_nocpuid.c:59:6: error: conflicting types for
>   'libxl_cpuid_policy_list_copy'
> 
> Probably due to 11316d31d684 "libxl: constify copy and length
> calculation functions" ?
> 

Yes, I didn't update the libxl_nocpuid.c. ARM uses that because it
doesn't have cpuid support.

I will go over libxl_no*.c and send out a patch soon.

Wei.

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

[Xen-devel] [seabios test] 97265: trouble: pass/preparing

[osstest service owner]

flight 97265 seabios running [real]
http://logs.test-lab.xenproject.org/osstest/logs/97265/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm  2 hosts-allocate   running
 test-amd64-i386-qemuu-rhel6hvm-amd  2 hosts-allocate   running
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  2 hosts-allocaterunning
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm  2 hosts-allocaterunning
 test-amd64-i386-xl-qemuu-debianhvm-amd64  2 hosts-allocate running
 test-amd64-amd64-qemuu-nested-amd  2 hosts-allocate   running
 test-amd64-i386-qemuu-rhel6hvm-intel  2 hosts-allocate   running
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm  2 hosts-allocaterunning
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  2 hosts-allocate running
 test-amd64-amd64-xl-qemuu-win7-amd64  2 hosts-allocate   running
 test-amd64-amd64-qemuu-nested-intel  2 hosts-allocate   running
 test-amd64-i386-xl-qemuu-win7-amd64  2 hosts-allocate   running
 test-amd64-amd64-xl-qemuu-winxpsp3  2 hosts-allocate   running
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  2 hosts-allocate running
 test-amd64-i386-xl-qemuu-winxpsp3  2 hosts-allocate   running

version targeted for testing:
 seabios  54e3a88609da074aaae2f04e592026ebf82169dc
baseline version:
 seabios  13213a252286372efa5f72b4119faafd5dff5db1

Last test of basis96767  2016-07-07 15:43:40 Z5 days
Testing same since0  1970-01-01 00:00:00 Z 16995 days0 attempts


People who touched revisions under test:
  Kevin O'Connor 
  Paolo Bonzini 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   preparing
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpreparing
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpreparing
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm preparing
 test-amd64-amd64-qemuu-nested-amdpreparing
 test-amd64-i386-qemuu-rhel6hvm-amd   preparing
 test-amd64-amd64-xl-qemuu-debianhvm-amd64preparing
 test-amd64-i386-xl-qemuu-debianhvm-amd64 preparing
 test-amd64-amd64-xl-qemuu-win7-amd64 preparing
 test-amd64-i386-xl-qemuu-win7-amd64  preparing
 test-amd64-amd64-qemuu-nested-intel  preparing
 test-amd64-i386-qemuu-rhel6hvm-intel preparing
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 preparing
 test-amd64-amd64-xl-qemuu-winxpsp3   preparing
 test-amd64-i386-xl-qemuu-winxpsp3preparing



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

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

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

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


Not pushing.


commit 54e3a88609da074aaae2f04e592026ebf82169dc
Author: Paolo Bonzini 
Date:   Thu Jul 7 16:00:40 2016 +0200

smp: restore MSRs on S3 resume

Currently the MTRRs and MSR_IA32_FEATURE_CONTROL are not restored on S3
resume.  Because these have to be applied to all processors, SMP setup
has to be added to S3 resume.

There are two differences between the boot and resume paths.  First,
romfile_* is not usable in the resume paths so we separate out the
remaining common code to a new smp_scan function.  Second, smp_msr has
to be walked on the BSP as well, so we extract that out of handle_smp
and into a new function smp_write_msrs.  Then, resume can call
smp_write_msrs on the BSP followed by smp_scan to initialize the APs.

Reported-by: Laszlo Ersek 
Signed-off-by: Paolo Bonzini 
Signed-

Re: [Xen-devel] [PATCH v7 00/14] xen/arm: Use the typesafes gfn and mfn

[Andrew Cooper]

On 12/07/16 14:59, Julien Grall wrote:
> Hello all,
>
> Some of the ARM functions are mixing gfn vs mfn and even physical vs frame.
>
> To avoid more confusion, this patch series makes use of the terminology
> described in xen/include/xen/mm.h and the associated typesafe.
>
> I pushed a branch with this series applied on top of staging:
> git://xenbits.xen.org/people/julieng/xen-unstable.git branch typesafe-v7

Patches 1-6 committed.

~Andrew

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

[Xen-devel] [ovmf test] 97263: trouble: pass/preparing

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-ovmf-amd64  2 hosts-allocate   running
 test-amd64-i386-xl-qemuu-ovmf-amd64  2 hosts-allocate   running

version targeted for testing:
 ovmf 31441f298365dd182a7a672f1b40fbdb6115c12f
baseline version:
 ovmf dc99315b8732b6e3032d01319d3f534d440b43d0

Last test of basis94748  2016-05-24 22:43:25 Z   49 days
Failing since 94750  2016-05-25 03:43:08 Z   49 days  107 attempts
Testing same since0  1970-01-01 00:00:00 Z 16995 days0 attempts


People who touched revisions under test:
  Anandakrishnan Loganathan 
  Ard Biesheuvel 
  Bi, Dandan 
  Bret Barkelew 
  Bruce Cran 
  Bruce Cran 
  Chao Zhang 
  Cinnamon Shia 
  Cohen, Eugene 
  Dandan Bi 
  Darbin Reyes 
  david wei 
  Eric Dong 
  Eugene Cohen 
  Evan Lloyd 
  Evgeny Yakovlev 
  Feng Tian 
  Fu Siyuan 
  Fu, Siyuan 
  Gary Li 
  Gary Lin 
  Giri P Mudusuru 
  Graeme Gregory 
  Hao Wu 
  Hegde Nagaraj P 
  Hegde, Nagaraj P 
  hegdenag 
  Heyi Guo 
  Jan D?bro? 
  Jan Dabros 
  Jeff Fan 
  Jiaxin Wu 
  Jiewen Yao 
  Joe Zhou 
  Jordan Justen 
  Katie Dellaquila 
  Laszlo Ersek 
  Liming Gao 
  Lu, ShifeiX A 
  lushifex 
  Marcin Wojtas 
  Mark Rutland 
  Marvin H?user 
  Marvin Haeuser 
  Maurice Ma 
  Michael Zimmermann 
  Mudusuru, Giri P 
  Ni, Ruiyu 
  Qiu Shumin 
  Ruiyu Ni 
  Ruiyu Ni　
  Ryan Harkin 
  Sami Mujawar 
  Satya Yarlagadda 
  Shannon Zhao 
  Sriram Subramanian 
  Star Zeng 
  Subramanian, Sriram (EG Servers Platform SW) 
  Sunny Wang 
  Tapan Shah 
  Thomas Palmer 
  Yarlagadda, Satya P 
  Yonghong Zhu 
  Zhang Lubo 
  Zhang, Chao B 
  Zhang, Lubo 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 preparing
 test-amd64-i386-xl-qemuu-ovmf-amd64  preparing



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

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

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

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


Not pushing.

(No revision log; it would be 9737 lines long.)

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

Re: [Xen-devel] xenstored memory leak

[David Vrabel]

On 13/07/16 14:07, Wei Liu wrote:
> 
> My gut feeling is that xenstored shouldn't have the knowledge to
> associate a watch with a "process". The concept of a process is only
> meaningful to OS, which wouldn't work on cross-domain xenstored setup.
> Maybe the OS xenbus driver should reap all watches on behalf the dead
> process. This would also avoid a crashed QEMU leaking resources.

The Linux driver already cleans up open transactions and removes watches
when the file handle is released.

David

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

Re: [Xen-devel] [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)

[Corneliu ZUZU]

On 7/13/2016 3:12 PM, Andrew Cooper wrote:

On 13/07/16 12:23, Corneliu ZUZU wrote:

+typedef struct { int counter; } atomic_t;
+
+#define ATOMIC_INIT(i) { (i) }
+
+/**
+ * atomic_read - read atomic variable
+ * @v: pointer of type atomic_t
+ *
+ * Atomically reads the value of @v.
+ */
+static inline int atomic_read(atomic_t *v);
+
+/**
+ * _atomic_read - read atomic variable non-atomically
+ * @v atomic_t
+ *
+ * Non-atomically reads the value of @v
+ */
+static inline int _atomic_read(atomic_t v);
+
+/**
+ * atomic_set - set atomic variable
+ * @v: pointer of type atomic_t
+ * @i: required value
+ *
+ * Atomically sets the value of @v to @i.
+ */
+static inline void atomic_set(atomic_t *v, int i);
+
+/**
+ * _atomic_set - set atomic variable non-atomically
+ * @v: pointer of type atomic_t
+ * @i: required value
+ *
+ * Non-atomically sets the value of @v to @i.
+ */
+static inline void _atomic_set(atomic_t *v, int i);
+




Is it ok if I also omit repeating the comments which are already @ 
xen/atomic.h from the arch-side asm/atomic.h ?


Thanks,
Zuzu C.

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

Re: [Xen-devel] xenstored memory leak

[Ian Jackson]

Wei Liu writes ("Re: [Xen-devel] xenstored memory leak"):
> On Wed, Jul 13, 2016 at 02:21:38PM +0200, Juergen Gross wrote:
> > qemu as the device model is setting up a xenstore watch for each backend
> > type it is supporting. Unfortunately those watches are never removed
> > again. This sums up to the observed memory leak.

I think this must be a bug in C xenstored.

> > I'm not sure how oxenstored is avoiding the problem, may be by testing
> > socket connections to be still alive and so detecting qemu has gone.
> > OTOH this won't help for oxenstored running in another domain than the
> > device model (either due to oxenstore-stubdom, or a driver domain with
> > a qemu based device model).
> 
> How unfortunate.
> 
> My gut feeling is that xenstored shouldn't have the knowledge to
> associate a watch with a "process".

xenstored needs to associate watches with connections.  If a
connection is terminated, the watches need to be cleaned up, along
with whatever other things "belong" to that connection (notably
transactions, and replies in flight).

Here a `connection' might be a socket, or a ring.

C xenstored does have code which tries to do this.  It's a bit
impenetrable, though, because it's done through destructors provided
to the reference counting membery allocator (!)

> The concept of a process is only meaningful to OS, which wouldn't
> work on cross-domain xenstored setup.  Maybe the OS xenbus driver
> should reap all watches on behalf the dead process. This would also
> avoid a crashed QEMU leaking resources.

The OS xenbus driver needs to mediate everything, so that it can
direct replies to the right places etc.  It needs to (and does)
maintain a list of watches.  When a process closes the xenbus device,
it destroys the watches by issuing commands to the actual xenstored
via its ring connection.

I guess that the qemu in this case is making a socket connection to
xenstored.

Ian.

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

[Xen-devel] [PATCH] libxl: constify src parameter of libxl_nocpuid.c:libxl_cpuid_policy_list_copy

[Wei Liu]

In 11316d31 ("libxl: constify copy and length calculation functions") I
forgot to take care of libxl_nocpuid.c which also contains an
implementation of libxl_cpuid_policy_list_copy. That broke ARM build.

Fix it by constifying the src parameter.

Signed-off-by: Wei Liu 
---
Cc: Ian Jackson 

I've checked, other libxl_no*.c files don't have _length or _copy
functions.
---
 tools/libxl/libxl_nocpuid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_nocpuid.c b/tools/libxl/libxl_nocpuid.c
index 3dcaef2..ef1161c 100644
--- a/tools/libxl/libxl_nocpuid.c
+++ b/tools/libxl/libxl_nocpuid.c
@@ -58,7 +58,7 @@ int libxl__cpuid_policy_list_parse_json(libxl__gc *gc,
 
 void libxl_cpuid_policy_list_copy(libxl_ctx *ctx,
   libxl_cpuid_policy_list *dst,
-  libxl_cpuid_policy_list *src)
+  const libxl_cpuid_policy_list *src)
 {
 }
 
-- 
2.1.4


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

Re: [Xen-devel] xenstored memory leak

[Juergen Gross]

On 13/07/16 14:40, Andrew Cooper wrote:
> On 13/07/16 13:21, Juergen Gross wrote:
>> On 06/07/16 09:31, Juergen Gross wrote:
>>> While testing some patches for support of ballooning in Mini-OS by using
>>> the xenstore domain I realized that each xl create/destroy pair would
>>> increase memory consumption in Mini-OS by about 5kB. Wondering whether
>>> this is a xenstore domain only effect I did the same test with xenstored
>>> and oxenstored daemons.
>>>
>>> xenstored showed the same behavior, the "referenced" size showed by the
>>> pmap command grew by about 5kB for each create/destroy pair.
>>>
>>> oxenstored seemed to be even worse in the beginning (about 6kB for each
>>> pair), but after about 100 create/destroys the value seemed to be
>>> rather stable.
>>>
>>> Did anyone notice this memory leak before?
>> I think I've found the problem:
>>
>> qemu as the device model is setting up a xenstore watch for each backend
>> type it is supporting. Unfortunately those watches are never removed
>> again. This sums up to the observed memory leak.
>>
>> I'm not sure how oxenstored is avoiding the problem, may be by testing
>> socket connections to be still alive and so detecting qemu has gone.
>> OTOH this won't help for oxenstored running in another domain than the
>> device model (either due to oxenstore-stubdom, or a driver domain with
>> a qemu based device model).
> 
> I do seem to remember something along those lines.
> 
>>
>> I'll post a qemu patch to remove those watches on exit soon.
> 
> That is good, but there needs to be further consideration as to how to
> clean up after a crashed qemu/etc.

Hmm, I see two possibilities:

a) Add a possibility to do xs_unwatch() with a path and a token with
   wildcards (e.g.: xs_unwatch(xs, "/local/domain/0/backend/qdisk",
   "be:*:97:*") for removing all qdisk watches for domid 97) and use
   this when doing the xenstore cleanup from libxl on domain
   destruction.

b) Instead of watching /local/domain//backend/ let qemu
   create /local/domain//backend// and watch
   this. When this directory is being removed by libxl all the watches
   will be gone automatically.


Juergen

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

Re: [Xen-devel] [PATCH 1/2] asm/atomic.h: common prototyping (add xen/atomic.h)

[Andrew Cooper]

On 13/07/16 14:20, Corneliu ZUZU wrote:
> On 7/13/2016 3:12 PM, Andrew Cooper wrote:
>> On 13/07/16 12:23, Corneliu ZUZU wrote:
>>> +typedef struct { int counter; } atomic_t;
>>> +
>>> +#define ATOMIC_INIT(i) { (i) }
>>> +
>>> +/**
>>> + * atomic_read - read atomic variable
>>> + * @v: pointer of type atomic_t
>>> + *
>>> + * Atomically reads the value of @v.
>>> + */
>>> +static inline int atomic_read(atomic_t *v);
>>> +
>>> +/**
>>> + * _atomic_read - read atomic variable non-atomically
>>> + * @v atomic_t
>>> + *
>>> + * Non-atomically reads the value of @v
>>> + */
>>> +static inline int _atomic_read(atomic_t v);
>>> +
>>> +/**
>>> + * atomic_set - set atomic variable
>>> + * @v: pointer of type atomic_t
>>> + * @i: required value
>>> + *
>>> + * Atomically sets the value of @v to @i.
>>> + */
>>> +static inline void atomic_set(atomic_t *v, int i);
>>> +
>>> +/**
>>> + * _atomic_set - set atomic variable non-atomically
>>> + * @v: pointer of type atomic_t
>>> + * @i: required value
>>> + *
>>> + * Non-atomically sets the value of @v to @i.
>>> + */
>>> +static inline void _atomic_set(atomic_t *v, int i);
>>> +
>>
>
> Is it ok if I also omit repeating the comments which are already @
> xen/atomic.h from the arch-side asm/atomic.h ?

Yes - that should be fine.  No point having identical comments twice.

~Andrew

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

Re: [Xen-devel] ACPI builder re-licensing

[Lars Kurth]

Boris,

I can't remember how we managed this process the last time round (see for 
https://patchwork.kernel.org/patch/9172431/), but in that case we already had a 
patch. As far as I can see, we don't have the complete patch yet.

Thus, the question I would have to you is whether you want to prepare the 
complete patch first or get the approvals of all stake-holders first?

I also think we should establish two groups of people regardless of approach
A) A list of individuals with e-mails we need to get approval from 
B) A list of company reps which can provide approval on behalf of a companies 
contribution

What we did in https://patchwork.kernel.org/patch/9172431/ was to get ACKs from 
people in group A, and e-mail confirmation from people in group B.

Regardless of the approach, we also ought to write an explanation that is 
easily consumable for people in group B which may not be very involved in Xen.
- A short introduction outlining what see are trying to do and why (maybe refer 
back to this thread)
- Explain why we sent the explanation to person X (that would probably need to 
contain some boilerplate what (c) they and/or their orgs hold and why we 
contacted them)
- End with a question which they clearly can answer (and which we can copy as 
evidence into the changelog)

Would you be able to draft something? We can then distribute some of the 
activities based on existing personal relationships.

> On 12 Jul 2016, at 18:09, Ian Jackson  wrote:
> 
> Boris Ostrovsky writes ("[Xen-devel] ACPI builder re-licensing"):

>> Who needs to be notified
>> ===
> 
> NB that what is required is not notification, but permission.
> 
>> which indicated major contributions (and therefore a required ack) from
>> Citrix/Xensource, Suse/Novell, Oracle/Sun, Intel.

This is group B stuff
- Citrix/Xensource can be James Bulpin or me
- Suse/Novell can be arranged by Jan or Juergen 
- Intel ought to be Susie Li 
- Oracle/Sun can be arranged by Konrad or you

Could you double check whether any of these maybe should go into group A? 

>> 
>> For the rest ("trivial" or "simple" is, of course, a matter of opinion):
>> 
>> * debian.com is a single trivial patch to a Makefile
>> 
>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=83f34fdcdd26c3dcc793c571e7b75c705bd92e7a
> 
> You mean debian.org.  Of course debian.org is not the author here and
> would not be the copyrightholder; Bastian Blank would be.  But I agree
> that this patch is trivial.

ACK

> 
>> * Fujitsu provided two patches, one trivial the other not completely trivial
>> 
>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=e451db15ef6198f5d21b84618c833ac276087d70
>> 
>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=ab438874b6a8ae955b337c36e7b3204e29b8d407
> 
> This second patch is nontrivial and therefore we need approval from
> Fujitsu.

The second patch was signed off by Kouya Shimura 
I did see him post something on LKLM in Feb 2016 using the above e-mail 
address. 
So probably this could go into group A. If not, Kouya probably would know who 
can make the call on behalf of Fujitsu. 

> 
>> * net-space.pl is Daniel Kiper's (now, but not at the patch's time, at
>> Oracle) ISP. The patch is
>> 
>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=37fddaa5fe1a7e369827e4b9e25cdae5df9b3d7d
> 
> This patch is not trivial in copyright terms, so we need Daniel
> Kiper's approval.

We have Daniel's e-mail address, so this should go into group A. Not sure 
whether Daniel uses his old address still, which would be preferable.

>> * redhat is one trivial patch by Paolo Bonzini that has been reversed later:
>> 
>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=e4fd0475a08fda414da27c4e57b568f147cfc07e
> 
> If the patch has been reverted then we are not proposing to relicence.
> We need to be careful not to re-apply it without getting a new signoff
> from Red Hat.

We ought to check that. If not true, Paolo Bonzini should go into group A.

>> * IBM one patch from Stefan Berger
>> 
>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=9fd9787b0e7995ac5f2da504b92723c24d6a3737
>> 
>>  (plus what seems to inclusion of his work in
>> 
>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=883236e49a86a0174c6df61cac995ebf16d72b35)
> 
> The latter is rather sloppy practice :-/.
> 
>>  Also, ssdt_tpm.asl is explicitly copyrighted by IBM
> 
> So we need approval from IBM.

Stefan still seems to work for IBM, with stef...@us.ibm.com as e0-mail address. 
So Group A.

>> * A bunch of patches from from Simon Horman at Verge
> 
> So we need approval from Verge.

Group A: ho...@verge.net.au

>> * xen.org are S-o-b by Keir, all from 2011.
> 
> I think those were work-for-hire by Keir with his XenSource/Citrix hat
> on and are now owned by Citrix.  We should reconfirm with Keir.

Group A. Should be straightforward.

Best Regards
Lars


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

Re: [Xen-devel] [PATCH] libxl: constify src parameter of libxl_nocpuid.c:libxl_cpuid_policy_list_copy

[Ian Jackson]

Wei Liu writes ("[PATCH] libxl: constify src parameter of 
libxl_nocpuid.c:libxl_cpuid_policy_list_copy"):
> In 11316d31 ("libxl: constify copy and length calculation functions") I
> forgot to take care of libxl_nocpuid.c which also contains an
> implementation of libxl_cpuid_policy_list_copy. That broke ARM build.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] xenstored memory leak

[Juergen Gross]

On 13/07/16 15:07, Wei Liu wrote:
> On Wed, Jul 13, 2016 at 02:21:38PM +0200, Juergen Gross wrote:
>> On 06/07/16 09:31, Juergen Gross wrote:
>>> While testing some patches for support of ballooning in Mini-OS by using
>>> the xenstore domain I realized that each xl create/destroy pair would
>>> increase memory consumption in Mini-OS by about 5kB. Wondering whether
>>> this is a xenstore domain only effect I did the same test with xenstored
>>> and oxenstored daemons.
>>>
>>> xenstored showed the same behavior, the "referenced" size showed by the
>>> pmap command grew by about 5kB for each create/destroy pair.
>>>
>>> oxenstored seemed to be even worse in the beginning (about 6kB for each
>>> pair), but after about 100 create/destroys the value seemed to be
>>> rather stable.
>>>
>>> Did anyone notice this memory leak before?
>>
>> I think I've found the problem:
>>
>> qemu as the device model is setting up a xenstore watch for each backend
>> type it is supporting. Unfortunately those watches are never removed
>> again. This sums up to the observed memory leak.
>>
>> I'm not sure how oxenstored is avoiding the problem, may be by testing
>> socket connections to be still alive and so detecting qemu has gone.
>> OTOH this won't help for oxenstored running in another domain than the
>> device model (either due to oxenstore-stubdom, or a driver domain with
>> a qemu based device model).
>>
> 
> How unfortunate.
> 
> My gut feeling is that xenstored shouldn't have the knowledge to
> associate a watch with a "process". The concept of a process is only
> meaningful to OS, which wouldn't work on cross-domain xenstored setup.

Right.

> Maybe the OS xenbus driver should reap all watches on behalf the dead
> process. This would also avoid a crashed QEMU leaking resources.
> 
> And xenstored should have proper quota support so that a domain can't
> set up excessive numbers of watches.

This would be dom0 unless you arrange the device model to be accounted
as the domid it is running for. But this is problematic with a xenstore
domain again.


Juergen

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

Re: [Xen-devel] [PATCH v2] vmx/monitor: CPUID events

[Andrew Cooper]

On 12/07/16 19:13, Tamas K Lengyel wrote:
> This patch implements sending notification to a monitor subscriber when an
> x86/vmx guest executes the CPUID instruction.
>
> Signed-off-by: Tamas K Lengyel 
> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 
> Cc: Razvan Cojocaru 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Jun Nakajima 
> Cc: Kevin Tian 

Committed.

~Andrew

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

Re: [Xen-devel] [PATCH] x86, hvm: document the de facto policy for vCPU ids

[Andrew Cooper]

On 12/07/16 17:28, Konrad Rzeszutek Wilk wrote:
> On Tue, Jul 12, 2016 at 7:44 AM, Vitaly Kuznetsov  wrote:
>> PVHVM guests may need to know Xen's idea of vCPU ids they have and the
>> only way they can figure them out is to use ACPI ids from MADT table.
>> Document the de facto policy.
>>
>> Signed-off-by: Vitaly Kuznetsov 
> Acked-by: Konrad Rzeszutek Wilk 

Reviewed and committed.

~Andrew

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

Re: [Xen-devel] xenstored memory leak

[Ian Jackson]

Juergen Gross writes ("Re: [Xen-devel] xenstored memory leak"):
> On 13/07/16 14:40, Andrew Cooper wrote:
> > On 13/07/16 13:21, Juergen Gross wrote:
> >> I'll post a qemu patch to remove those watches on exit soon.

I don't think this is right.  qemu should not explictly remove watches
when it is exiting.  The cleanup should be handled by xenstored
directly (if qemu is connecting via a socket) or by the OS xenbus
driver (otherwise).  Each of those entities keeps a list of which of
their clients owns what watches.

> > That is good, but there needs to be further consideration as to how to
> > clean up after a crashed qemu/etc.
> 
> Hmm, I see two possibilities:
> 
> a) Add a possibility to do xs_unwatch() with a path and a token with
>wildcards (e.g.: xs_unwatch(xs, "/local/domain/0/backend/qdisk",
>"be:*:97:*") for removing all qdisk watches for domid 97) and use
>this when doing the xenstore cleanup from libxl on domain
>destruction.
> 
> b) Instead of watching /local/domain//backend/ let qemu
>create /local/domain//backend// and watch
>this. When this directory is being removed by libxl all the watches
>will be gone automatically.

This is going in entirely the wrong direction.

Ian.

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

Re: [Xen-devel] xenstored memory leak

[David Vrabel]

On 13/07/16 14:32, Juergen Gross wrote:
> On 13/07/16 15:17, David Vrabel wrote:
>> On 13/07/16 14:07, Wei Liu wrote:
>>>
>>> My gut feeling is that xenstored shouldn't have the knowledge to
>>> associate a watch with a "process". The concept of a process is only
>>> meaningful to OS, which wouldn't work on cross-domain xenstored setup.
>>> Maybe the OS xenbus driver should reap all watches on behalf the dead
>>> process. This would also avoid a crashed QEMU leaking resources.
>>
>> The Linux driver already cleans up open transactions and removes watches
>> when the file handle is released.
> 
> Hmm, does this work reliably? I observed a memory leak of about 5kB per
> create/destroy domain pair with xenstored _and_ with xenstore domain.

Don't know.

I only took a quick look at the xenbus_file_release() function and it
looked like it ought to be doing the right thing...

David

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

Re: [Xen-devel] [PATCH 1/6] xen/build: Allow the use of C freestanding headers

[Andrew Cooper]

On 22/06/16 14:12, Tim Deegan wrote:
> At 12:24 +0100 on 22 Jun (1466598248), Andrew Cooper wrote:
>> The C standard defines two types of conforming implementation; hosted and
>> freestanding.  A subset of the standard headers are the freestanding headers,
>> requiring no library support at all to use, and therefore usable by Xen.
>>
>> Unfortunately, -nostdinc is an overly large switch, and there is no
>> alternative to only permit inclusion of the freestanding headers.  Removing 
>> it
>> is unfortunate, as we lose the protection it offers, but anyone who does try
>> to use other parts of the standard library will still fail to link.
> I'm afraid I don't think this is a good idea:
>  - Leaving the standard include path around in the Xen build means
>that the build may differ based on what (unrelated) libraries are
>installed on the build machines.
>  - There are plenty of ways for an unexpected header to break things
>that don't fail at link time, e.g. macros and inlines.
>  - "Freestanding" headers can bring in quite a lot of unrelated cruft.
>See Jan's email about linux/glibc, and I remember seeing similar
>things on solaris and *BSD when I tidied up stdarg.h. E.g. looking
>at two machines I'm working on today, on one of them,
>#include  defines __packed, and on the other it does not.
>
> Since what we have already works fine for all the compilers we
> support, I think it ain't broke and we shouldn't fix it.

Except it is broken.

We cannot expect to use -Wformat and not the compiler provided
inttypes.h.  The sizes of constructs like "long long" are implementation
defined, not spec defined.  The compiler is perfectly free to choose
something which doesn't match our inttypes.h, and we would be in the
wrong when it fails to compile.

~Andrew

>
> Nacked-by: Tim Deegan 
>
> OTOH, I am in favour of s/bool_t/bool/g, at least wherever field size
> is not an issue, and I can see an argument for moving the type and
> limits definitions into files called stdint.h and limits.h.
>
> Cheers,
>
> Tim.
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


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

Re: [Xen-devel] xenstored memory leak

[Wei Liu]

On Wed, Jul 13, 2016 at 02:20:28PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [Xen-devel] xenstored memory leak"):
> > On Wed, Jul 13, 2016 at 02:21:38PM +0200, Juergen Gross wrote:
> > > qemu as the device model is setting up a xenstore watch for each backend
> > > type it is supporting. Unfortunately those watches are never removed
> > > again. This sums up to the observed memory leak.
> 
> I think this must be a bug in C xenstored.
> 
> > > I'm not sure how oxenstored is avoiding the problem, may be by testing
> > > socket connections to be still alive and so detecting qemu has gone.
> > > OTOH this won't help for oxenstored running in another domain than the
> > > device model (either due to oxenstore-stubdom, or a driver domain with
> > > a qemu based device model).
> > 
> > How unfortunate.
> > 
> > My gut feeling is that xenstored shouldn't have the knowledge to
> > associate a watch with a "process".
> 
> xenstored needs to associate watches with connections.  If a
> connection is terminated, the watches need to be cleaned up, along
> with whatever other things "belong" to that connection (notably
> transactions, and replies in flight).
> 
> Here a `connection' might be a socket, or a ring.
> 

Agreed.

> C xenstored does have code which tries to do this.  It's a bit
> impenetrable, though, because it's done through destructors provided
> to the reference counting membery allocator (!)
> 
> > The concept of a process is only meaningful to OS, which wouldn't
> > work on cross-domain xenstored setup.  Maybe the OS xenbus driver
> > should reap all watches on behalf the dead process. This would also
> > avoid a crashed QEMU leaking resources.
> 
> The OS xenbus driver needs to mediate everything, so that it can
> direct replies to the right places etc.  It needs to (and does)
> maintain a list of watches.  When a process closes the xenbus device,
> it destroys the watches by issuing commands to the actual xenstored
> via its ring connection.
> 
> I guess that the qemu in this case is making a socket connection to
> xenstored.
> 

Yeah, I suspect that as well.

Wei.

> Ian.

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

Re: [Xen-devel] xenstored memory leak

[Juergen Gross]

On 13/07/16 15:17, David Vrabel wrote:
> On 13/07/16 14:07, Wei Liu wrote:
>>
>> My gut feeling is that xenstored shouldn't have the knowledge to
>> associate a watch with a "process". The concept of a process is only
>> meaningful to OS, which wouldn't work on cross-domain xenstored setup.
>> Maybe the OS xenbus driver should reap all watches on behalf the dead
>> process. This would also avoid a crashed QEMU leaking resources.
> 
> The Linux driver already cleans up open transactions and removes watches
> when the file handle is released.

Hmm, does this work reliably? I observed a memory leak of about 5kB per
create/destroy domain pair with xenstored _and_ with xenstore domain.


Juergen


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

Re: [Xen-devel] xenstored memory leak

[Wei Liu]

On Wed, Jul 13, 2016 at 03:25:45PM +0200, Juergen Gross wrote:
> On 13/07/16 15:07, Wei Liu wrote:
> > On Wed, Jul 13, 2016 at 02:21:38PM +0200, Juergen Gross wrote:
> >> On 06/07/16 09:31, Juergen Gross wrote:
> >>> While testing some patches for support of ballooning in Mini-OS by using
> >>> the xenstore domain I realized that each xl create/destroy pair would
> >>> increase memory consumption in Mini-OS by about 5kB. Wondering whether
> >>> this is a xenstore domain only effect I did the same test with xenstored
> >>> and oxenstored daemons.
> >>>
> >>> xenstored showed the same behavior, the "referenced" size showed by the
> >>> pmap command grew by about 5kB for each create/destroy pair.
> >>>
> >>> oxenstored seemed to be even worse in the beginning (about 6kB for each
> >>> pair), but after about 100 create/destroys the value seemed to be
> >>> rather stable.
> >>>
> >>> Did anyone notice this memory leak before?
> >>
> >> I think I've found the problem:
> >>
> >> qemu as the device model is setting up a xenstore watch for each backend
> >> type it is supporting. Unfortunately those watches are never removed
> >> again. This sums up to the observed memory leak.
> >>
> >> I'm not sure how oxenstored is avoiding the problem, may be by testing
> >> socket connections to be still alive and so detecting qemu has gone.
> >> OTOH this won't help for oxenstored running in another domain than the
> >> device model (either due to oxenstore-stubdom, or a driver domain with
> >> a qemu based device model).
> >>
> > 
> > How unfortunate.
> > 
> > My gut feeling is that xenstored shouldn't have the knowledge to
> > associate a watch with a "process". The concept of a process is only
> > meaningful to OS, which wouldn't work on cross-domain xenstored setup.
> 
> Right.
> 
> > Maybe the OS xenbus driver should reap all watches on behalf the dead
> > process. This would also avoid a crashed QEMU leaking resources.
> > 
> > And xenstored should have proper quota support so that a domain can't
> > set up excessive numbers of watches.
> 
> This would be dom0 unless you arrange the device model to be accounted
> as the domid it is running for. But this is problematic with a xenstore
> domain again.
> 

The quota could be based on "connection" (ring or socket) and
counted as per-connection? Just throwing ideas around, not necessarily
saying this is the way to go.

Wei.

> 
> Juergen

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

Re: [Xen-devel] xenstored memory leak

[Juergen Gross]

On 13/07/16 15:52, Wei Liu wrote:
> On Wed, Jul 13, 2016 at 03:25:45PM +0200, Juergen Gross wrote:
>> On 13/07/16 15:07, Wei Liu wrote:
>>> On Wed, Jul 13, 2016 at 02:21:38PM +0200, Juergen Gross wrote:
 On 06/07/16 09:31, Juergen Gross wrote:
> While testing some patches for support of ballooning in Mini-OS by using
> the xenstore domain I realized that each xl create/destroy pair would
> increase memory consumption in Mini-OS by about 5kB. Wondering whether
> this is a xenstore domain only effect I did the same test with xenstored
> and oxenstored daemons.
>
> xenstored showed the same behavior, the "referenced" size showed by the
> pmap command grew by about 5kB for each create/destroy pair.
>
> oxenstored seemed to be even worse in the beginning (about 6kB for each
> pair), but after about 100 create/destroys the value seemed to be
> rather stable.
>
> Did anyone notice this memory leak before?

 I think I've found the problem:

 qemu as the device model is setting up a xenstore watch for each backend
 type it is supporting. Unfortunately those watches are never removed
 again. This sums up to the observed memory leak.

 I'm not sure how oxenstored is avoiding the problem, may be by testing
 socket connections to be still alive and so detecting qemu has gone.
 OTOH this won't help for oxenstored running in another domain than the
 device model (either due to oxenstore-stubdom, or a driver domain with
 a qemu based device model).

>>>
>>> How unfortunate.
>>>
>>> My gut feeling is that xenstored shouldn't have the knowledge to
>>> associate a watch with a "process". The concept of a process is only
>>> meaningful to OS, which wouldn't work on cross-domain xenstored setup.
>>
>> Right.
>>
>>> Maybe the OS xenbus driver should reap all watches on behalf the dead
>>> process. This would also avoid a crashed QEMU leaking resources.
>>>
>>> And xenstored should have proper quota support so that a domain can't
>>> set up excessive numbers of watches.
>>
>> This would be dom0 unless you arrange the device model to be accounted
>> as the domid it is running for. But this is problematic with a xenstore
>> domain again.
>>
> 
> The quota could be based on "connection" (ring or socket) and
> counted as per-connection? Just throwing ideas around, not necessarily
> saying this is the way to go.

Sure. But with xenstore domain all xenstore access of dom0 is via one
ring. And how would you want to apply any quota here solving our
problem with one qemu process in dom0 creating stale watches? You
could open a new connection for the device model, of course, but this
would require some xenbus rework.


Juergen

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

Re: [Xen-devel] [PATCH v2 0/7] Remove hard-coded /var/run in tools

[Wei Liu]

On Tue, Jul 12, 2016 at 04:21:00PM +, Rusty Bird wrote:
> Hello Wei,
> 
> For systemd/xendriverdomain.service.in, the hardcoded PID file could be
> removed altogether -- systemd has no trouble figuring out the PID with
> only one process. But I wasn't sure if maybe something outside of the
> xen tree uses it?

Oh, thanks for the heads-up. I will just remove the path with a separate
patch if that's not very useful.

Out-of-tree users have the option patch the script as they see fit. But
if they use systemd it would be better to use the "systemd-way" to
obtain the pid anyway.

Wei.

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

Re: [Xen-devel] xenstored memory leak

[Wei Liu]

On Wed, Jul 13, 2016 at 04:09:26PM +0200, Juergen Gross wrote:
> On 13/07/16 15:52, Wei Liu wrote:
> > On Wed, Jul 13, 2016 at 03:25:45PM +0200, Juergen Gross wrote:
> >> On 13/07/16 15:07, Wei Liu wrote:
> >>> On Wed, Jul 13, 2016 at 02:21:38PM +0200, Juergen Gross wrote:
>  On 06/07/16 09:31, Juergen Gross wrote:
> > While testing some patches for support of ballooning in Mini-OS by using
> > the xenstore domain I realized that each xl create/destroy pair would
> > increase memory consumption in Mini-OS by about 5kB. Wondering whether
> > this is a xenstore domain only effect I did the same test with xenstored
> > and oxenstored daemons.
> >
> > xenstored showed the same behavior, the "referenced" size showed by the
> > pmap command grew by about 5kB for each create/destroy pair.
> >
> > oxenstored seemed to be even worse in the beginning (about 6kB for each
> > pair), but after about 100 create/destroys the value seemed to be
> > rather stable.
> >
> > Did anyone notice this memory leak before?
> 
>  I think I've found the problem:
> 
>  qemu as the device model is setting up a xenstore watch for each backend
>  type it is supporting. Unfortunately those watches are never removed
>  again. This sums up to the observed memory leak.
> 
>  I'm not sure how oxenstored is avoiding the problem, may be by testing
>  socket connections to be still alive and so detecting qemu has gone.
>  OTOH this won't help for oxenstored running in another domain than the
>  device model (either due to oxenstore-stubdom, or a driver domain with
>  a qemu based device model).
> 
> >>>
> >>> How unfortunate.
> >>>
> >>> My gut feeling is that xenstored shouldn't have the knowledge to
> >>> associate a watch with a "process". The concept of a process is only
> >>> meaningful to OS, which wouldn't work on cross-domain xenstored setup.
> >>
> >> Right.
> >>
> >>> Maybe the OS xenbus driver should reap all watches on behalf the dead
> >>> process. This would also avoid a crashed QEMU leaking resources.
> >>>
> >>> And xenstored should have proper quota support so that a domain can't
> >>> set up excessive numbers of watches.
> >>
> >> This would be dom0 unless you arrange the device model to be accounted
> >> as the domid it is running for. But this is problematic with a xenstore
> >> domain again.
> >>
> > 
> > The quota could be based on "connection" (ring or socket) and
> > counted as per-connection? Just throwing ideas around, not necessarily
> > saying this is the way to go.
> 
> Sure. But with xenstore domain all xenstore access of dom0 is via one
> ring. And how would you want to apply any quota here solving our
> problem with one qemu process in dom0 creating stale watches? You

That's a job for the kernel driver.

The quota inside xenstored is to protect itself from abuse from one
single connection and punish the bad actors.

> could open a new connection for the device model, of course, but this
> would require some xenbus rework.
> 

I wouldn't go down that route unless absolutely necessary  because that
seems to require xenbus protocol extension.

Wei.

> 
> Juergen

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

[Xen-devel] [PATCH v2 0/5] adjustments

[Corneliu ZUZU]

Corneliu ZUZU (5):
  asm-arm/atomic.h: fix arm32|arm64 macros duplication
  asm-x86/atomic.h: minor: proper atomic_inc_and_test() placement
  asm-arm/atomic.h: reorder macros to match x86-side
  asm/atomic.h: common prototyping (add xen/atomic.h)
  fix: make atomic_read() param const

 xen/include/asm-arm/arm32/atomic.h |  11 ---
 xen/include/asm-arm/arm64/atomic.h |  11 ---
 xen/include/asm-arm/atomic.h   |  46 +++---
 xen/include/asm-x86/atomic.h   | 128 +++
 xen/include/xen/atomic.h   | 171 +
 5 files changed, 220 insertions(+), 147 deletions(-)
 create mode 100644 xen/include/xen/atomic.h

-- 
2.5.0


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

[Xen-devel] [PATCH v2 1/5] asm-arm/atomic.h: fix arm32|arm64 macros duplication

[Corneliu ZUZU]

Move duplicate macros between asm-arm/arm32/atomic.h and asm-arm/arm64/atomic.h
to asm-arm/atomic.h.

Signed-off-by: Corneliu ZUZU 
---
 xen/include/asm-arm/arm32/atomic.h | 11 ---
 xen/include/asm-arm/arm64/atomic.h | 11 ---
 xen/include/asm-arm/atomic.h   | 11 +++
 3 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/xen/include/asm-arm/arm32/atomic.h 
b/xen/include/asm-arm/arm32/atomic.h
index 7ec712f..be08ff1 100644
--- a/xen/include/asm-arm/arm32/atomic.h
+++ b/xen/include/asm-arm/arm32/atomic.h
@@ -149,17 +149,6 @@ static inline int __atomic_add_unless(atomic_t *v, int a, 
int u)
 
 #define atomic_xchg(v, new) (xchg(&((v)->counter), new))
 
-#define atomic_inc(v)  atomic_add(1, v)
-#define atomic_dec(v)  atomic_sub(1, v)
-
-#define atomic_inc_and_test(v) (atomic_add_return(1, v) == 0)
-#define atomic_dec_and_test(v) (atomic_sub_return(1, v) == 0)
-#define atomic_inc_return(v)(atomic_add_return(1, v))
-#define atomic_dec_return(v)(atomic_sub_return(1, v))
-#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0)
-
-#define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0)
-
 #endif /* __ARCH_ARM_ARM32_ATOMIC__ */
 /*
  * Local variables:
diff --git a/xen/include/asm-arm/arm64/atomic.h 
b/xen/include/asm-arm/arm64/atomic.h
index b49219e..80d07bf 100644
--- a/xen/include/asm-arm/arm64/atomic.h
+++ b/xen/include/asm-arm/arm64/atomic.h
@@ -125,17 +125,6 @@ static inline int __atomic_add_unless(atomic_t *v, int a, 
int u)
return c;
 }
 
-#define atomic_inc(v)  atomic_add(1, v)
-#define atomic_dec(v)  atomic_sub(1, v)
-
-#define atomic_inc_and_test(v) (atomic_add_return(1, v) == 0)
-#define atomic_dec_and_test(v) (atomic_sub_return(1, v) == 0)
-#define atomic_inc_return(v)(atomic_add_return(1, v))
-#define atomic_dec_return(v)(atomic_sub_return(1, v))
-#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0)
-
-#define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0)
-
 #endif
 /*
  * Local variables:
diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 29ab265..41d1b6c 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -138,6 +138,17 @@ static inline void _atomic_set(atomic_t *v, int i)
 # error "unknown ARM variant"
 #endif
 
+#define atomic_inc(v)   atomic_add(1, v)
+#define atomic_dec(v)   atomic_sub(1, v)
+
+#define atomic_inc_and_test(v)  (atomic_add_return(1, v) == 0)
+#define atomic_dec_and_test(v)  (atomic_sub_return(1, v) == 0)
+#define atomic_inc_return(v)(atomic_add_return(1, v))
+#define atomic_dec_return(v)(atomic_sub_return(1, v))
+#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0)
+
+#define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0)
+
 #endif /* __ARCH_ARM_ATOMIC__ */
 /*
  * Local variables:
-- 
2.5.0


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

[Xen-devel] [PATCH v2 2/5] asm-x86/atomic.h: minor: proper atomic_inc_and_test() placement

[Corneliu ZUZU]

Place atomic_inc_and_test() implementation after atomic_inc().
Also remove unneeded empty line and add a needed one.

Signed-off-by: Corneliu ZUZU 
---
 xen/include/asm-arm/atomic.h |  1 +
 xen/include/asm-x86/atomic.h | 39 +++
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 41d1b6c..19a87a5 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -150,6 +150,7 @@ static inline void _atomic_set(atomic_t *v, int i)
 #define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0)
 
 #endif /* __ARCH_ARM_ATOMIC__ */
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index d246b70..5f9f2dd 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -110,7 +110,6 @@ static inline int _atomic_read(atomic_t v)
 return v.counter;
 }
 
-
 /**
  * atomic_set - set atomic variable
  * @v: pointer of type atomic_t
@@ -217,6 +216,25 @@ static inline void atomic_inc(atomic_t *v)
 }
 
 /**
+ * atomic_inc_and_test - increment and test
+ * @v: pointer of type atomic_t
+ *
+ * Atomically increments @v by 1
+ * and returns true if the result is zero, or false for all
+ * other cases.
+ */
+static inline int atomic_inc_and_test(atomic_t *v)
+{
+unsigned char c;
+
+asm volatile (
+"lock; incl %0; sete %1"
+: "=m" (*(volatile int *)&v->counter), "=qm" (c)
+: "m" (*(volatile int *)&v->counter) : "memory" );
+return c != 0;
+}
+
+/**
  * atomic_dec - decrement atomic variable
  * @v: pointer of type atomic_t
  * 
@@ -250,25 +268,6 @@ static inline int atomic_dec_and_test(atomic_t *v)
 }
 
 /**
- * atomic_inc_and_test - increment and test 
- * @v: pointer of type atomic_t
- * 
- * Atomically increments @v by 1
- * and returns true if the result is zero, or false for all
- * other cases.
- */ 
-static inline int atomic_inc_and_test(atomic_t *v)
-{
-unsigned char c;
-
-asm volatile (
-"lock; incl %0; sete %1"
-: "=m" (*(volatile int *)&v->counter), "=qm" (c)
-: "m" (*(volatile int *)&v->counter) : "memory" );
-return c != 0;
-}
-
-/**
  * atomic_add_negative - add and test if negative
  * @v: pointer of type atomic_t
  * @i: integer value to add
-- 
2.5.0


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

[Xen-devel] [PATCH v2 3/5] asm-arm/atomic.h: reorder macros to match x86-side

[Corneliu ZUZU]

Reorder macro definitions to match x86-side.

Signed-off-by: Corneliu ZUZU 
---
 xen/include/asm-arm/atomic.h | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 19a87a5..e8f7340 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -138,16 +138,15 @@ static inline void _atomic_set(atomic_t *v, int i)
 # error "unknown ARM variant"
 #endif
 
-#define atomic_inc(v)   atomic_add(1, v)
-#define atomic_dec(v)   atomic_sub(1, v)
-
-#define atomic_inc_and_test(v)  (atomic_add_return(1, v) == 0)
-#define atomic_dec_and_test(v)  (atomic_sub_return(1, v) == 0)
-#define atomic_inc_return(v)(atomic_add_return(1, v))
-#define atomic_dec_return(v)(atomic_sub_return(1, v))
-#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0)
-
-#define atomic_add_negative(i,v) (atomic_add_return(i, v) < 0)
+#define atomic_inc_return(v)(atomic_add_return(1, v))
+#define atomic_dec_return(v)(atomic_sub_return(1, v))
+
+#define atomic_sub_and_test(i, v)   (atomic_sub_return(i, v) == 0)
+#define atomic_inc(v)   atomic_add(1, v)
+#define atomic_inc_and_test(v)  (atomic_add_return(1, v) == 0)
+#define atomic_dec(v)   atomic_sub(1, v)
+#define atomic_dec_and_test(v)  (atomic_sub_return(1, v) == 0)
+#define atomic_add_negative(i,v)(atomic_add_return(i, v) < 0)
 
 #endif /* __ARCH_ARM_ATOMIC__ */
 
-- 
2.5.0


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

Re: [Xen-devel] ACPI builder re-licensing

[Boris Ostrovsky]

On 07/13/2016 09:21 AM, Lars Kurth wrote:
> Boris,
>
> I can't remember how we managed this process the last time round (see for 
> https://patchwork.kernel.org/patch/9172431/), but in that case we already had 
> a patch. As far as I can see, we don't have the complete patch yet.
>
> Thus, the question I would have to you is whether you want to prepare the 
> complete patch first or get the approvals of all stake-holders first?

I certainly can (and should) prepare and post the patch but I thought we
should first come up with (A) list at the least. OTOH, we can at least
review the patch first here on xen-devel without bothering people from
that list with revisions. So yes, I will.

>
> I also think we should establish two groups of people regardless of approach
> A) A list of individuals with e-mails we need to get approval from 
> B) A list of company reps which can provide approval on behalf of a companies 
> contribution
>
> What we did in https://patchwork.kernel.org/patch/9172431/ was to get ACKs 
> from people in group A, and e-mail confirmation from people in group B.
>
> Regardless of the approach, we also ought to write an explanation that is 
> easily consumable for people in group B which may not be very involved in Xen.
> - A short introduction outlining what see are trying to do and why (maybe 
> refer back to this thread)
> - Explain why we sent the explanation to person X (that would probably need 
> to contain some boilerplate what (c) they and/or their orgs hold and why we 
> contacted them)
> - End with a question which they clearly can answer (and which we can copy as 
> evidence into the changelog)
>
> Would you be able to draft something? We can then distribute some of the 
> activities based on existing personal relationships.


Will do and post as the preamble to the patch for review.


>
>> On 12 Jul 2016, at 18:09, Ian Jackson  wrote:
>>
>> Boris Ostrovsky writes ("[Xen-devel] ACPI builder re-licensing"):
>>> Who needs to be notified
>>> ===
>> NB that what is required is not notification, but permission.
>>
>>> which indicated major contributions (and therefore a required ack) from
>>> Citrix/Xensource, Suse/Novell, Oracle/Sun, Intel.
> This is group B stuff
> - Citrix/Xensource can be James Bulpin or me
> - Suse/Novell can be arranged by Jan or Juergen 
> - Intel ought to be Susie Li 
> - Oracle/Sun can be arranged by Konrad or you

Konrad is already doing this for Oracle.

>
> Could you double check whether any of these maybe should go into group A? 

Changes from those companies came from multiple submitters so it seems
that getting a single ACK from a rep would be easier?


...

>>> * net-space.pl is Daniel Kiper's (now, but not at the patch's time, at
>>> Oracle) ISP. The patch is
>>>
>>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=37fddaa5fe1a7e369827e4b9e25cdae5df9b3d7d
>> This patch is not trivial in copyright terms, so we need Daniel
>> Kiper's approval.
> We have Daniel's e-mail address, so this should go into group A. Not sure 
> whether Daniel uses his old address still, which would be preferable.

I spoke with Daniel and he is fine ACKing this.

>
>>> * redhat is one trivial patch by Paolo Bonzini that has been reversed later:
>>>
>>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=e4fd0475a08fda414da27c4e57b568f147cfc07e
>> If the patch has been reverted then we are not proposing to relicence.
>> We need to be careful not to re-apply it without getting a new signoff
>> from Red Hat.
> We ought to check that. If not true, Paolo Bonzini should go into group A.

In principle we could in the future decide to do exactly what Paolo's
patch originally did (which was to remove the 'if' condition). There is
no way to do it any differently than how Paolo's patch did it.

-boris



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

[Xen-devel] [PATCH v2 5/5] fix: make atomic_read() param const

[Corneliu ZUZU]

This wouldn't let me make a param of a function that used atomic_read() const.

Signed-off-by: Corneliu ZUZU 
Reviewed-by: Andrew Cooper 

---
Changed since v1: 
---
 xen/include/asm-arm/atomic.h | 2 +-
 xen/include/asm-x86/atomic.h | 2 +-
 xen/include/xen/atomic.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 01af43b..25a33cb 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -102,7 +102,7 @@ void __bad_atomic_size(void);
  * strex/ldrex monitor on some implementations. The reason we can use it for
  * atomic_set() is the clrex or dummy strex done on every exception return.
  */
-static inline int atomic_read(atomic_t *v)
+static inline int atomic_read(const atomic_t *v)
 {
 return *(volatile int *)&v->counter;
 }
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index 3e99b03..1729e29 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -80,7 +80,7 @@ void __bad_atomic_size(void);
 } \
 })
 
-static inline int atomic_read(atomic_t *v)
+static inline int atomic_read(const atomic_t *v)
 {
 return read_atomic(&v->counter);
 }
diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
index 3517bc9..d97f91d 100644
--- a/xen/include/xen/atomic.h
+++ b/xen/include/xen/atomic.h
@@ -32,7 +32,7 @@ typedef struct { int counter; } atomic_t;
  *
  * Atomically reads the value of @v.
  */
-static inline int atomic_read(atomic_t *v);
+static inline int atomic_read(const atomic_t *v);
 
 /**
  * _atomic_read - read atomic variable non-atomically
-- 
2.5.0


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

[Xen-devel] [PATCH v2 4/5] asm/atomic.h: common prototyping (add xen/atomic.h)

[Corneliu ZUZU]

Create a common-side  to establish, among others, prototypes of
atomic functions called from common-code. Done to avoid introducing
inconsistencies between arch-side  headers when we make subtle
changes to one of them.

Some arm-side macros had to be turned into inline functions in the process.
Removed outdated comment ("NB. I've [...]").

Signed-off-by: Corneliu ZUZU 
Suggested-by: Andrew Cooper 
Reviewed-by: Andrew Cooper 
---
Changed since v1:
  * removed comments that were duplicate between asm-x86/atomic.h and
xen/atomic.h
  * remove outdated comment ("NB. [...]")
  * add atomic_cmpxchg doc-comment
  * don't use yoda condition
---
 xen/include/asm-arm/atomic.h |  45 
 xen/include/asm-x86/atomic.h | 103 +-
 xen/include/xen/atomic.h | 171 +++
 3 files changed, 202 insertions(+), 117 deletions(-)
 create mode 100644 xen/include/xen/atomic.h

diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index e8f7340..01af43b 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -2,6 +2,7 @@
 #define __ARCH_ARM_ATOMIC__
 
 #include 
+#include 
 #include 
 #include 
 
@@ -95,15 +96,6 @@ void __bad_atomic_size(void);
 default: __bad_atomic_size(); break;\
 }   \
 })
-
-/*
- * NB. I've pushed the volatile qualifier into the operations. This allows
- * fast accessors such as _atomic_read() and _atomic_set() which don't give
- * the compiler a fit.
- */
-typedef struct { int counter; } atomic_t;
-
-#define ATOMIC_INIT(i) { (i) }
 
 /*
  * On ARM, ordinary assignment (str instruction) doesn't clear the local
@@ -141,12 +133,35 @@ static inline void _atomic_set(atomic_t *v, int i)
 #define atomic_inc_return(v)(atomic_add_return(1, v))
 #define atomic_dec_return(v)(atomic_sub_return(1, v))
 
-#define atomic_sub_and_test(i, v)   (atomic_sub_return(i, v) == 0)
-#define atomic_inc(v)   atomic_add(1, v)
-#define atomic_inc_and_test(v)  (atomic_add_return(1, v) == 0)
-#define atomic_dec(v)   atomic_sub(1, v)
-#define atomic_dec_and_test(v)  (atomic_sub_return(1, v) == 0)
-#define atomic_add_negative(i,v)(atomic_add_return(i, v) < 0)
+static inline int atomic_sub_and_test(int i, atomic_t *v)
+{
+return atomic_sub_return(i, v) == 0;
+}
+
+static inline void atomic_inc(atomic_t *v)
+{
+atomic_add(1, v);
+}
+
+static inline int atomic_inc_and_test(atomic_t *v)
+{
+return atomic_add_return(1, v) == 0;
+}
+
+static inline void atomic_dec(atomic_t *v)
+{
+atomic_sub(1, v);
+}
+
+static inline int atomic_dec_and_test(atomic_t *v)
+{
+return atomic_sub_return(1, v) == 0;
+}
+
+static inline int atomic_add_negative(int i, atomic_t *v)
+{
+return atomic_add_return(i, v) < 0;
+}
 
 #endif /* __ARCH_ARM_ATOMIC__ */
 
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index 5f9f2dd..3e99b03 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -2,6 +2,7 @@
 #define __ARCH_X86_ATOMIC__
 
 #include 
+#include 
 #include 
 
 #define build_read_atomic(name, size, type, reg, barrier) \
@@ -79,56 +80,21 @@ void __bad_atomic_size(void);
 } \
 })
 
-/*
- * NB. I've pushed the volatile qualifier into the operations. This allows
- * fast accessors such as _atomic_read() and _atomic_set() which don't give
- * the compiler a fit.
- */
-typedef struct { int counter; } atomic_t;
-
-#define ATOMIC_INIT(i) { (i) }
-
-/**
- * atomic_read - read atomic variable
- * @v: pointer of type atomic_t
- *
- * Atomically reads the value of @v.
- */
 static inline int atomic_read(atomic_t *v)
 {
 return read_atomic(&v->counter);
 }
 
-/**
- * _atomic_read - read atomic variable non-atomically
- * @v atomic_t
- *
- * Non-atomically reads the value of @v
- */
 static inline int _atomic_read(atomic_t v)
 {
 return v.counter;
 }
 
-/**
- * atomic_set - set atomic variable
- * @v: pointer of type atomic_t
- * @i: required value
- *
- * Atomically sets the value of @v to @i.
- */
 static inline void atomic_set(atomic_t *v, int i)
 {
 write_atomic(&v->counter, i);
 }
 
-/**
- * _atomic_set - set atomic variable non-atomically
- * @v: pointer of type atomic_t
- * @i: required value
- *
- * Non-atomically sets the value of @v to @i.
- */
 static inline void _atomic_set(atomic_t *v, int i)
 {
 v->counter = i;
@@ -139,13 +105,6 @@ static inline int atomic_cmpxchg(atomic_t *v, int old, int 
new)
 return cmpxchg(&v->counter, old, new);
 }
 
-/**
- * atomic_add - add integer to atomic variable
- * @i: integer value to add
- * @v: pointer of type atomic_t
- * 
- * Atomically adds @i to @v.
- */
 static inline void atomic_add(int i, atomic_t *v)
 {
 asm volatile (
@@ -154,25 +113,11 @@ static inline void atomic_add(int i, atomic_t *v)
 : "ir"

Re: [Xen-devel] xenstored memory leak

[Ian Jackson]

David Vrabel writes ("Re: [Xen-devel] xenstored memory leak"):
> On 13/07/16 14:32, Juergen Gross wrote:
> > On 13/07/16 15:17, David Vrabel wrote:
> >> The Linux driver already cleans up open transactions and removes watches
> >> when the file handle is released.
> > 
> > Hmm, does this work reliably? I observed a memory leak of about 5kB per
> > create/destroy domain pair with xenstored _and_ with xenstore domain.
> 
> Don't know.
> 
> I only took a quick look at the xenbus_file_release() function and it
> looked like it ought to be doing the right thing...

If it didn't, it would leak with oxenstored too.

Ian.

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

Re: [Xen-devel] ACPI builder re-licensing

[Lars Kurth]

On 13/07/2016 15:22, "Boris Ostrovsky"  wrote:

>On 07/13/2016 09:21 AM, Lars Kurth wrote:
>> Boris,
>>
>> I can't remember how we managed this process the last time round (see
>>for https://patchwork.kernel.org/patch/9172431/), but in that case we
>>already had a patch. As far as I can see, we don't have the complete
>>patch yet.
>>
>> Thus, the question I would have to you is whether you want to prepare
>>the complete patch first or get the approvals of all stake-holders first?
>
>I certainly can (and should) prepare and post the patch but I thought we
>should first come up with (A) list at the least.

It's basically in this thread already and is
- Kouya Shimura 
- Daniel's old private e-mail address, if still active.
- Stefan Berger 
- Simon Horman 
- Keir 
 
Possibly Paolo

>OTOH, we can at least
>review the patch first here on xen-devel without bothering people from
>that list with revisions. So yes, I will.
...
>
>Will do and post as the preamble to the patch for review.

Thanks

Regards
Lars

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

[Xen-devel] [xen-unstable-smoke test] 97269: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf   5 xen-build fail REGR. vs. 96794

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass

version targeted for testing:
 xen  1fb0fe97a647549cc1bbe90f15b6cb2fcc3f79b6
baseline version:
 xen  7da483b0236d8974cc97f81780dcf8e559a63175

Last test of basis96794  2016-07-08 15:01:26 Z4 days
Testing same since97261  2016-07-13 11:00:54 Z0 days2 attempts


People who touched revisions under test:
  Corneliu ZUZU 
  Ian Jackson 
  Juergen Gross 
  Julien Grall 
  Kevin Tian 
  Stefano Stabellini 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  fail
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  blocked 
 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


Not pushing.


commit 1fb0fe97a647549cc1bbe90f15b6cb2fcc3f79b6
Author: Corneliu ZUZU 
Date:   Sat Jul 9 07:12:07 2016 +0300

x86/vmx_update_guest_cr: minor optimization

Minor optimization @ vmx_update_guest_cr: checks if 
v->arch.hvm_vmx.exec_control
was modified before actually calling vmx_update_cpu_exec_control(v).

Signed-off-by: Corneliu ZUZU 
Acked-by: Kevin Tian 

commit 22ea8ad02e465e32cd40887c750b55c3a997a288
Author: Juergen Gross 
Date:   Wed Jul 6 16:55:34 2016 +0200

libxl: move DEFINE_DEVICE* macros to libxl_internal.h

In order to be able to have all functions related to a device type in
a single source file move the macros used to generate device type
specific functions to libxl_internal.h. Rename the macros as they are
no longer local to a source file. While at it hide device remove and
device destroy in one macro as those are always used in pairs. Move
usage of the macros to the appropriate source files.

Signed-off-by: Juergen Gross 
Acked-by: Ian Jackson 

commit 2c2c33f038c889acc9f73681c09320164516da47
Author: Juergen Gross 
Date:   Wed Jul 6 16:55:33 2016 +0200

libxl: refactor domcreate_attach_dtdev() to use device type framework

Signed-off-by: Juergen Gross 
Acked-by: Ian Jackson 

commit 26021d77dc14d441ff3b0094f2fafcc5d18aa599
Author: Juergen Gross 
Date:   Wed Jul 6 16:55:32 2016 +0200

libxl: refactor domcreate_attach_pci() to use device type framework

Signed-off-by: Juergen Gross 
Acked-by: Ian Jackson 

commit 74e857c6c7f9dc106fa7a7bbf49a9729f5841ad9
Author: Juergen Gross 
Date:   Wed Jul 6 16:55:31 2016 +0200

libxl: add framework for device types

Instead of duplicate coding for each device type (vtpms, usbctrls, ...)
especially on domain creation introduce a framework for that purpose.

Signed-off-by: Juergen Gross 
Acked-by: Ian Jackson 

commit 56bac262e097684b20f7753ceb6debe594e9725c
Author: Wei Liu 
Date:   Wed Jun 8 15:01:02 2016 +0100

libxl: only issue cpu-add call to QEMU for not present CPU

Calculate the final bitmap for CPUs to add to avoid having annoying
error messages complaining those CPUs are already present. Example
message is like (wrapped):

libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
error message from QMP server: Unable to add CPU: 0, it already exists

We can also properly handle error from QMP now.

Signed-off-by: Wei Liu 
Reviewed-by: Anthony PERARD 
Acked-by: Ian Jackson 

commit 01f3193e2e3b9b36bde027f909db496f7211c320
Author: Wei Liu 
Date:   Fri Jun 3 16:38:32 2016 +0100

libxl: update vcpus bitmap in retrieved guest config

... because the available vcpu bitmap can change during domain life time
due to cpu hotplug and unplug.

For QEMU upstream, we interrogate QEMU f

Re: [Xen-devel] xenstored memory leak

[Juergen Gross]

On 13/07/16 16:28, Ian Jackson wrote:
> David Vrabel writes ("Re: [Xen-devel] xenstored memory leak"):
>> On 13/07/16 14:32, Juergen Gross wrote:
>>> On 13/07/16 15:17, David Vrabel wrote:
 The Linux driver already cleans up open transactions and removes watches
 when the file handle is released.
>>>
>>> Hmm, does this work reliably? I observed a memory leak of about 5kB per
>>> create/destroy domain pair with xenstored _and_ with xenstore domain.
>>
>> Don't know.
>>
>> I only took a quick look at the xenbus_file_release() function and it
>> looked like it ought to be doing the right thing...
> 
> If it didn't, it would leak with oxenstored too.

This I can't tell. I've got no oxenstore domain running.


Juergen

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

Re: [Xen-devel] [PATCH] xl: add option to leave domain paused afer migration

[Wei Liu]

On Wed, Jul 13, 2016 at 10:53:13AM +0200, Roger Pau Monne wrote:
> This is useful for debugging domains that crash on resume from migration.
> 
> Signed-off-by: Roger Pau Monné 

This sounds like a useful thing to have and the patch looks good.

But you forgot to patch xl(1) manpage.

Wei.

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

Re: [Xen-devel] [PATCH 1/6] xen/build: Allow the use of C freestanding headers

[Tim Deegan]

At 14:46 +0100 on 13 Jul (1468421211), Andrew Cooper wrote:
> On 22/06/16 14:12, Tim Deegan wrote:
> > At 12:24 +0100 on 22 Jun (1466598248), Andrew Cooper wrote:
> >> The C standard defines two types of conforming implementation; hosted and
> >> freestanding.  A subset of the standard headers are the freestanding 
> >> headers,
> >> requiring no library support at all to use, and therefore usable by Xen.
> >>
> >> Unfortunately, -nostdinc is an overly large switch, and there is no
> >> alternative to only permit inclusion of the freestanding headers.  
> >> Removing it
> >> is unfortunate, as we lose the protection it offers, but anyone who does 
> >> try
> >> to use other parts of the standard library will still fail to link.
> > I'm afraid I don't think this is a good idea:
> >  - Leaving the standard include path around in the Xen build means
> >that the build may differ based on what (unrelated) libraries are
> >installed on the build machines.
> >  - There are plenty of ways for an unexpected header to break things
> >that don't fail at link time, e.g. macros and inlines.
> >  - "Freestanding" headers can bring in quite a lot of unrelated cruft.
> >See Jan's email about linux/glibc, and I remember seeing similar
> >things on solaris and *BSD when I tidied up stdarg.h. E.g. looking
> >at two machines I'm working on today, on one of them,
> >#include  defines __packed, and on the other it does not.
> >
> > Since what we have already works fine for all the compilers we
> > support, I think it ain't broke and we shouldn't fix it.
> 
> Except it is broken.

Broken in theory or actually causing a problem right now?

> We cannot expect to use -Wformat and not the compiler provided
> inttypes.h.

Since we're not using the compiler-provided printf(), I think that
we're on pretty thin ice with -Wformat anyway.  But as long as our
own PRI* macros match our type definitions, all should be well, right?

> The sizes of constructs like "long long" are implementation
> defined, not spec defined.  The compiler is perfectly free to choose
> something which doesn't match our inttypes.h, and we would be in the
> wrong when it fails to compile.

If a compiler we support does that, we can update our integer type
definitions, like we do for attribute annotations, &c.

BTW, I can absolutely see the argument for deferring to the compiler
in C implementation details.  I think that would have to include
removing all the implementation-reserved names from Xen so we don't
clash with compiler headers.

But in practice it seems to be messy enough to be better avoided --
having /usr/include on the search path is pretty silly.

Tim.

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

Re: [Xen-devel] [PATCH v2 16/17] libxc/xc_dom_arm: Copy ACPI tables to guest space

[Julien Grall]

Hello,

On 12/07/2016 17:58, Boris Ostrovsky wrote:

On 07/12/2016 12:10 PM, Julien Grall wrote:

On 12/07/2016 16:08, Boris Ostrovsky wrote:

On 07/12/2016 10:57 AM, Shannon Zhao wrote:

On 2016年07月12日 22:50, Wei Liu wrote:

On Tue, Jul 12, 2016 at 10:42:07PM +0800, Shannon Zhao wrote:

Does it mean we would need to update the slack
to take into account the ACPI
blob?

Yes, we need to take into account the ACPI blob.
Probably not in the
slack but directly in mam_memkb.

Sorry, I'm not sure understand this. I found the
b_info->max_memkb but
didn't find the slack you said. And how to fix this? Update
b_info->max_memkb or the slack?

Can you calculate the size of your payload and add that to
max_memkb?


Yeah, but the size will be changed if we change the tables in the
future
and this also should consider x86, right?

That could easily be solved by introducing a function to calculate the
size, right?

Oh, I'm not familiar with this. Let's clarify on this. It can add the
size to max_memkb after generating the ACPI tables and before loading
the tables to guest space and it doesn't have to add the size at
libxl__domain_build_info_setdefault(), right?


This was discussed before: ACPI tables are part of RAM whose size is
specified by the config file (and is reflected in max_memkb I believe).
It may not be presented to the guest as RAM (i.e. on x86 it is labeled
by BIOS (or whoever) as a dedicated type in e820) but it still resides
in DIMMs.


I don't think this was the conclusion of the thread. IHMO, "maxmem" is
the amount of RAM a guest could effectively use.

Whilst the ACPI tables will be in the DIMM from the host point of
view. From a guest point of view it will be a ROM.


The config file specifies resources provided by the host. How the guest
views those resources is not important, I think.


This would need to be clarified. For instance special pages (Xenstore, 
Console...) are RAM from the host point of view but not taken into 
account in the "maxmem" provided by the user. For my understanding, some 
kB of the slack is used for that.






It will affect some others part of the guest if we don't increment the
"maxmem" requested by the user. For ARM the ACPI blob will be exposed
at a specific address that is outside of the guest RAM (see the guest
memory layout in public/arch-arm.h).

We chose this solution over putting in the RAM because the ACPI tables
are not easily relocatable (compare to the device tree, initrd and
kernel) so we could not take advantage of superpage in both stage-2
(hypervisor) and stage-1 (kernel) page table.


Maybe this is something ARM-specific then. For x86 we will want to keep
maxmem unchanged.


I don't think what I described in my previous mail is ARM-specific. The 
pressure will be more important on the TLBs, if Xen does not use 
superpage in the stage 2 page tables (i.e EPT for x86) no matter the 
architecture.


IHMO, this seems to be a bigger drawback compare to add few more 
kilobytes to maxmem in the toolstack for the ACPI blob. You will loose 
them when creating the intermediate page table in any case.


May I ask why you want to keep maxmem unchanged on x86?

Regards,

--
Julien Grall

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

Re: [Xen-devel] No graphics with xen pv and Fedora qemu

[Andrew Cooper]

On 10/07/16 16:44, Michael Young wrote:
> On Wed, 29 Jun 2016, Michael Young wrote:
>
>> I have been trying to trace a problem when using Fedora's qemu with a
>> pv guest which is that no graphics are available. I get the errors
>>
>> xen be core: xen be: watching backend path (backend/console/2) failed
>> xen be core: xen be: watching backend path (backend/vkbd/2) failed
>> xen be core: xen be: watching backend path (backend/vfb/2) failed
>> xen be core: xen be: watching backend path (backend/qdisk/2) failed
>> xen be core: xen be: watching backend path (backend/qnic/2) failed
>>
>> in the qemu log file in /var/log/xen . So far I have traced it to rbd
>> support in qemu, because qemu-system-i386 built with the
>> --disable-rbd does have working graphics.
>
> I tracked this down eventually. The failure is in tools/xenstore/xs.c
> in xs_watch when it tries to create a read pthread, because the
> initial stack size of 16384 is apparently too small with qemu with rbd
> enabled. I did get it to work if I increased the stack size to 24576.

This is rather curious (and confusing).

I presume this is the xenstore reader thread in qemu which is causing
problems?

The stack size of the reader thread shouldn't need to be big at all, and
I can't see why rbd specifically would cause an issue.  Is there a stack
overflow happening?

~Andrew

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

[Xen-devel] [DRAFT v2] XenSock protocol design document

[Stefano Stabellini]

Hi all,

This is the design document of the XenSock protocol. You can find
prototypes of the Linux frontend and backend drivers here:

git://git.kernel.org/pub/scm/linux/kernel/git/sstabellini/xen.git xensock-2

To use them, make sure to enable CONFIG_XENSOCK in your kernel config
and add "xensock=1" to the command line of your DomU Linux kernel. You
also need the toolstack to create the initial xenstore nodes for the
protocol. To do that, please apply the attached patch to libxl (the
patch is based on Xen 4.7.0-rc3) and add "xensock=1" to your DomU config
file.

Cheers,

Stefano


Changes in v2:
- add max-dataring-page-order
- add "Publish backend features and transport parameters" to backend
  xenbus workflow
- update new cmd values
- update xen_xensock_request
- add backlog parameter to listen and binary layout
- add description of new data ring format (interface+data)
- modify connect and accept to reflect new data ring format
- add link to POSIX docs
- add error numbers
- add address format section and relevant numeric definitions
- add explicit mention of unimplemented commands
- add protocol node name
- add xenbus shutdown diagram
- add socket operation

---

# XenSocks Protocol v1

## Rationale

XenSocks is a paravirtualized protocol for the POSIX socket API.

The purpose of XenSocks is to allow the implementation of a specific set
of POSIX functions to be done in a domain other than your own. It allows
connect, accept, bind, release, listen, poll, recvmsg and sendmsg to be
implemented in another domain.

XenSocks provides the following benefits:
* guest networking works out of the box with VPNs, wireless networks and
  any other complex configurations on the host
* guest services listen on ports bound directly to the backend domain IP
  addresses
* localhost becomes a secure namespace for inter-VMs communications
* full visibility of the guest behavior on the backend domain, allowing
  for inexpensive filtering and manipulation of any guest calls
* excellent performance


## Design

### Xenstore

The frontend and the backend connect to each other exchanging information via
xenstore. The toolstack creates front and back nodes with state
XenbusStateInitialising. The protocol node name is **xensock**. There can only
be one XenSock frontend per domain.

 Frontend XenBus Nodes

port
 Values: 

 The identifier of the Xen event channel used to signal activity
 in the ring buffer.

ring-ref
 Values: 

 The Xen grant reference granting permission for the backend to map
 the sole page in a single page sized ring buffer.

 Backend XenBus Nodes

max-dataring-page-order
Values: 

The maximum supported size of the data ring in units of lb(machine
pages). (e.g. 0 == 1 page,  1 = 2 pages, 2 == 4 pages, etc.).

 State Machine

Initialization:

*Front*   *Back*
XenbusStateInitialising   XenbusStateInitialising
- Query virtual device- Query backend device
  properties.   identification data.
- Setup OS device instance.   - Publish backend features
- Allocate and initialize the   and transport parameters
  request ring.  |
- Publish transport parameters   |
  that will be in effect during  V
  this connection.XenbusStateInitWait
 |
 |
 V
   XenbusStateInitialised

  - Query frontend transport parameters.
  - Connect to the request ring and
event channel.
 |
 |
 V
 XenbusStateConnected

 - Query backend device properties.
 - Finalize OS virtual device
   instance.
 |
 |
 V
XenbusStateConnected

Once frontend and backend are connected, they have a shared page, which
will is used to exchange messages over a ring, and an event channel,
which is used to send notifications.

Shutdown:

*Front**Back*
XenbusStateConnected   XenbusStateConnected
|
|
V
   XenbusStateClosing

   - Unmap grants
   - Unbind evtchns
 |
 |
 V
 XenbusStateClosing

- Unbind evtchns
- Free rings
- Free data structures
   

Re: [Xen-devel] [DRAFT v2] XenSock protocol design document

[Andrew Cooper]

On 13/07/16 16:47, Stefano Stabellini wrote:
> Hi all,
>
> This is the design document of the XenSock protocol. You can find
> prototypes of the Linux frontend and backend drivers here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/sstabellini/xen.git xensock-2
>
> To use them, make sure to enable CONFIG_XENSOCK in your kernel config
> and add "xensock=1" to the command line of your DomU Linux kernel. You
> also need the toolstack to create the initial xenstore nodes for the
> protocol. To do that, please apply the attached patch to libxl (the
> patch is based on Xen 4.7.0-rc3) and add "xensock=1" to your DomU config
> file.

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 9e48199..478eb0a 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -120,6 +120,7 @@
 #include 
 #endif
 #include 
+#include 
 
 
 /* The inetsw table contains everything that inet_create needs to
@@ -1775,6 +1776,11 @@ static int __init inet_init(void)
 for (r = &inetsw[0]; r < &inetsw[SOCK_MAX]; ++r)
 INIT_LIST_HEAD(r);
 
+if (xensock) {
+pr_info("Enabling xensock for AF_INET SOCK_STREAM\n");
+inetsw_array[0].ops = &xensock_stream_ops;
+}
+
 for (q = inetsw_array; q < &inetsw_array[INETSW_ARRAY_LEN]; ++q)
 inet_register_protosw(q);
 

I am curious as to how you plan to persuade Dave Miller to take this patch?

What is your planned mitigation for the fact that INET sockets stop
behaving like INET sockets.  Are there any plans for IPv6?

~Andrew

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

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  ea210c52abb6458e39f5365f7f2c3abb9c191c47
baseline version:
 xen  7da483b0236d8974cc97f81780dcf8e559a63175

Last test of basis96794  2016-07-08 15:01:26 Z5 days
Failing since 97261  2016-07-13 11:00:54 Z0 days3 attempts
Testing same since97274  2016-07-13 15:02:31 Z0 days1 attempts


People who touched revisions under test:
  Corneliu ZUZU 
  Elena Ufimtseva 
  George Dunlap 
  Ian Jackson 
  Jan Beulich 
  Juergen Gross 
  Julien Grall 
  Kevin Tian 
  Konrad Rzeszutek Wilk 
  Razvan Cojocaru 
  Stefano Stabellini 
  Tamas K Lengyel 
  Tim Deegan 
  Vitaly Kuznetsov 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

+ branch=xen-unstable-smoke
+ revision=ea210c52abb6458e39f5365f7f2c3abb9c191c47
+ . ./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 
ea210c52abb6458e39f5365f7f2c3abb9c191c47
+ branch=xen-unstable-smoke
+ revision=ea210c52abb6458e39f5365f7f2c3abb9c191c47
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.7-testing
+ '[' xea210c52abb6458e39f5365f7f2c3abb9c191c47 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git
+++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src 
'[fetch=try]'
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src

Re: [Xen-devel] [PATCH v2 16/17] libxc/xc_dom_arm: Copy ACPI tables to guest space

[Boris Ostrovsky]

On 07/13/2016 11:22 AM, Julien Grall wrote:
> Hello,
>
> On 12/07/2016 17:58, Boris Ostrovsky wrote:
>> On 07/12/2016 12:10 PM, Julien Grall wrote:
>>> On 12/07/2016 16:08, Boris Ostrovsky wrote:
 On 07/12/2016 10:57 AM, Shannon Zhao wrote:
> On 2016年07月12日 22:50, Wei Liu wrote:
>> On Tue, Jul 12, 2016 at 10:42:07PM +0800, Shannon Zhao wrote:
>> Does it mean we would need to update the slack
>> to take into account the ACPI
>> blob?
>> Yes, we need to take into account the ACPI blob.
>> Probably not in the
>> slack but directly in mam_memkb.
>> Sorry, I'm not sure understand this. I found the
>> b_info->max_memkb but
>> didn't find the slack you said. And how to fix this? Update
>> b_info->max_memkb or the slack?
>> Can you calculate the size of your payload and add that to
>> max_memkb?
>>
 Yeah, but the size will be changed if we change the tables in the
 future
 and this also should consider x86, right?
>> That could easily be solved by introducing a function to
>> calculate the
>> size, right?
> Oh, I'm not familiar with this. Let's clarify on this. It can add the
> size to max_memkb after generating the ACPI tables and before loading
> the tables to guest space and it doesn't have to add the size at
> libxl__domain_build_info_setdefault(), right?

 This was discussed before: ACPI tables are part of RAM whose size is
 specified by the config file (and is reflected in max_memkb I
 believe).
 It may not be presented to the guest as RAM (i.e. on x86 it is labeled
 by BIOS (or whoever) as a dedicated type in e820) but it still resides
 in DIMMs.
>>>
>>> I don't think this was the conclusion of the thread. IHMO, "maxmem" is
>>> the amount of RAM a guest could effectively use.
>>>
>>> Whilst the ACPI tables will be in the DIMM from the host point of
>>> view. From a guest point of view it will be a ROM.
>>
>> The config file specifies resources provided by the host. How the guest
>> views those resources is not important, I think.
>
> This would need to be clarified. For instance special pages (Xenstore,
> Console...) are RAM from the host point of view but not taken into
> account in the "maxmem" provided by the user. For my understanding,
> some kB of the slack is used for that.


Are these pages part of guest's address space?


>
>>
>>>
>>> It will affect some others part of the guest if we don't increment the
>>> "maxmem" requested by the user. For ARM the ACPI blob will be exposed
>>> at a specific address that is outside of the guest RAM (see the guest
>>> memory layout in public/arch-arm.h).
>>>
>>> We chose this solution over putting in the RAM because the ACPI tables
>>> are not easily relocatable (compare to the device tree, initrd and
>>> kernel) so we could not take advantage of superpage in both stage-2
>>> (hypervisor) and stage-1 (kernel) page table.
>>
>> Maybe this is something ARM-specific then. For x86 we will want to keep
>> maxmem unchanged.
>
> I don't think what I described in my previous mail is ARM-specific.
> The pressure will be more important on the TLBs, if Xen does not use
> superpage in the stage 2 page tables (i.e EPT for x86) no matter the
> architecture.
>
> IHMO, this seems to be a bigger drawback compare to add few more
> kilobytes to maxmem in the toolstack for the ACPI blob. You will loose
> them when creating the intermediate page table in any case.


Why not have the guest ask for more memory in the config file then?

(OK, I can see that they can't ask for a few KB since we have MB
resolution but they can ask for an extra 1MB)


>
> May I ask why you want to keep maxmem unchanged on x86?

Only to keep resource accounting fair. The guest may decide to use
memory reserved for ACPI as a regular memory.



-boris



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

[Xen-devel] [patch V2 49/67] arm/xen: Convert to hotplug state machine

[Anna-Maria Gleixner]

From: Richard Cochran 

Install the callbacks via the state machine and let the core invoke
the callbacks on the already online CPUs.

The get_cpu() in xen_starting_cpu() boils down to preempt_disable() since
we already know the CPU we run on. Disabling preemption shouldn't be required
here from what I see since it we don't switch CPUs while invoking the function.

Signed-off-by: Richard Cochran 
Reviewed-by: Sebastian Andrzej Siewior 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Russell King 
Cc: Stefano Stabellini 
Cc: Stefano Stabellini 
Cc: Thomas Gleixner 
Cc: linux-arm-ker...@lists.infradead.org
Cc: xen-de...@lists.xenproject.org
Signed-off-by: Anna-Maria Gleixner 
---
 arch/arm/xen/enlighten.c   | 41 +++--
 include/linux/cpuhotplug.h |  1 +
 2 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 75cd734..d822e23 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -161,12 +161,11 @@ static struct notifier_block xen_pvclock_gtod_notifier = {
.notifier_call = xen_pvclock_gtod_notify,
 };
 
-static void xen_percpu_init(void)
+static int xen_starting_cpu(unsigned int cpu)
 {
struct vcpu_register_vcpu_info info;
struct vcpu_info *vcpup;
int err;
-   int cpu = get_cpu();
 
/* 
 * VCPUOP_register_vcpu_info cannot be called twice for the same
@@ -190,7 +189,13 @@ static void xen_percpu_init(void)
 
 after_register_vcpu_info:
enable_percpu_irq(xen_events_irq, 0);
-   put_cpu();
+   return 0;
+}
+
+static int xen_dying_cpu(unsigned int cpu)
+{
+   disable_percpu_irq(xen_events_irq);
+   return 0;
 }
 
 static void xen_restart(enum reboot_mode reboot_mode, const char *cmd)
@@ -209,28 +214,6 @@ static void xen_power_off(void)
BUG_ON(rc);
 }
 
-static int xen_cpu_notification(struct notifier_block *self,
-   unsigned long action,
-   void *hcpu)
-{
-   switch (action) {
-   case CPU_STARTING:
-   xen_percpu_init();
-   break;
-   case CPU_DYING:
-   disable_percpu_irq(xen_events_irq);
-   break;
-   default:
-   break;
-   }
-
-   return NOTIFY_OK;
-}
-
-static struct notifier_block xen_cpu_notifier = {
-   .notifier_call = xen_cpu_notification,
-};
-
 static irqreturn_t xen_arm_callback(int irq, void *arg)
 {
xen_hvm_evtchn_do_upcall();
@@ -351,16 +334,14 @@ static int __init xen_guest_init(void)
return -EINVAL;
}
 
-   xen_percpu_init();
-
-   register_cpu_notifier(&xen_cpu_notifier);
-
pv_time_ops.steal_clock = xen_stolen_accounting;
static_key_slow_inc(¶virt_steal_enabled);
if (xen_initial_domain())
pvclock_gtod_register_notifier(&xen_pvclock_gtod_notifier);
 
-   return 0;
+   return cpuhp_setup_state(CPUHP_AP_ARM_XEN_STARTING,
+"AP_ARM_XEN_STARTING", xen_starting_cpu,
+xen_dying_cpu);
 }
 early_initcall(xen_guest_init);
 
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 86ac982..f986963 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -48,6 +48,7 @@ enum cpuhp_state {
CPUHP_AP_KVM_STARTING,
CPUHP_AP_KVM_ARM_VGIC_STARTING,
CPUHP_AP_KVM_ARM_TIMER_STARTING,
+   CPUHP_AP_ARM_XEN_STARTING,
CPUHP_AP_LEDTRIG_STARTING,
CPUHP_AP_NOTIFY_STARTING,
CPUHP_AP_ONLINE,
-- 
2.8.1




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

Re: [Xen-devel] [DRAFT v2] XenSock protocol design document

[Stefano Stabellini]

On Wed, 13 Jul 2016, Andrew Cooper wrote:
> On 13/07/16 16:47, Stefano Stabellini wrote:
> > Hi all,
> >
> > This is the design document of the XenSock protocol. You can find
> > prototypes of the Linux frontend and backend drivers here:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/sstabellini/xen.git xensock-2
> >
> > To use them, make sure to enable CONFIG_XENSOCK in your kernel config
> > and add "xensock=1" to the command line of your DomU Linux kernel. You
> > also need the toolstack to create the initial xenstore nodes for the
> > protocol. To do that, please apply the attached patch to libxl (the
> > patch is based on Xen 4.7.0-rc3) and add "xensock=1" to your DomU config
> > file.
> 
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 9e48199..478eb0a 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -120,6 +120,7 @@
>  #include 
>  #endif
>  #include 
> +#include 
>  
>  
>  /* The inetsw table contains everything that inet_create needs to
> @@ -1775,6 +1776,11 @@ static int __init inet_init(void)
>  for (r = &inetsw[0]; r < &inetsw[SOCK_MAX]; ++r)
>  INIT_LIST_HEAD(r);
>  
> +if (xensock) {
> +pr_info("Enabling xensock for AF_INET SOCK_STREAM\n");
> +inetsw_array[0].ops = &xensock_stream_ops;
> +}
> +
>  for (q = inetsw_array; q < &inetsw_array[INETSW_ARRAY_LEN]; ++q)
>  inet_register_protosw(q);
>  
> 
> I am curious as to how you plan to persuade Dave Miller to take this patch?
>
> What is your planned mitigation for the fact that INET sockets stop
> behaving like INET sockets.

Let me premise that obviously those 4 lines of code are unlikely to be
the way this code will actually be merged in Linux. As I said these are
just prototypes. But they are far better than the very first version I
had :-)

That's the chicken and egg problem: I cannot really have a proper
conversation about it on the LKML until the protocol is, if not
accepted, at least a bit farther down the review process. In my
experience discussing about design docs on the LKML doesn't work well --
they want to see the code first. Once the design is somewhat "settled",
I'll engage with netdev. They are the ones with the best expertize on
how to integrate this work in Linux.

I did have informal conversations with two or three Linux maintainers
(but not Dave Miller) -- they seemed positive about it and they thought
XenSock is useful. We'll have to see.

At the end of the day, if the Linux drivers are no-go, the protocol can
still be used for fully userspace implementations or other OSes.


> Are there any plans for IPv6?

I don't have any at the moment, but I would be happy to see that
happening.

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

Re: [Xen-devel] [PATCH] XSM-Policy: allow source domain access to setpodtarget for ballooning.

[Daniel De Graaf]

On 07/13/2016 08:59 AM, Anshul Makkar wrote:

Access to setpodtarget is required by dom0 to set the balloon targets for
domU. The patch gives source domain (dom0) access to set this target for
domU and resolve the following permission denied error message during
ballooning :
avc:  denied  { setpodtarget } for domid=0 target=9
scontext=system_u:system_r:dom0_t
tcontext=system_u:system_r:domU_t tclass=domain

Signed-off-by: Anshul Makkar 


This seems to indicate that getpodtarget should also be added to the list.

Either as-is or with getpodtarget also added,
Acked-by: Daniel De Graaf 

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

Re: [Xen-devel] [PATCH v7 07/14] xen/arm: map_regions_rw_cache: Map the region with p2m->default_access

[Stefano Stabellini]

On Tue, 12 Jul 2016, Julien Grall wrote:
> The parameter 'access' is used by memaccess to restrict temporarily the
> permission. This parameter should not be used for other purpose (such
> as restricting permanently the permission).
> 
> Instead, we should use the default access requested by memacess. When it
> is not enabled, the access will be p2m_access_rwx (i.e no restriction
> applied).
> 
> The type p2m_mmio_direct will map the region read-write and
> non-executable before any further restriction by memaccess. Note that
> this is already the resulting permission with the curreent combination
> of the type and the access. So there is no functional change.
> 
> Signed-off-by: Julien Grall 

Acked-by: Stefano Stabellini 


> ---
> Cc: Shannon Zhao 
> 
> This patch is a candidate for Xen 4.7. Currently this function is
> only used to map ACPI regions.
> 
> I am wondering if we should introduce a new p2m type for it. And map
> this region RO (I am not sure why a guest would want to
> modify this region).
> 
> Changes in v2:
> - Reword the commit message
> 
> Changes in v4:
> - Patch added
> ---
>  xen/arch/arm/p2m.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 1cfb62b..fcc4513 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1231,7 +1231,7 @@ int map_regions_rw_cache(struct domain *d,
>   pfn_to_paddr(start_gfn + nr),
>   pfn_to_paddr(mfn),
>   MATTR_MEM, 0, p2m_mmio_direct,
> - p2m_access_rw);
> + d->arch.p2m.default_access);
>  }
>  
>  int unmap_regions_rw_cache(struct domain *d,
> @@ -1244,7 +1244,7 @@ int unmap_regions_rw_cache(struct domain *d,
>   pfn_to_paddr(start_gfn + nr),
>   pfn_to_paddr(mfn),
>   MATTR_MEM, 0, p2m_invalid,
> - p2m_access_rw);
> + d->arch.p2m.default_access);
>  }
>  
>  int map_mmio_regions(struct domain *d,
> -- 
> 1.9.1
> 

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

Re: [Xen-devel] [PATCH v7 10/14] xen/arm: Use the typesafes mfn and gfn in map_dev_mmio_region...

[Stefano Stabellini]

On Tue, 12 Jul 2016, Julien Grall wrote:
> to avoid mixing machine frame with guest frame. Also drop the prefix start_.
> 
> Signed-off-by: Julien Grall 

Acked-by: Stefano Stabellini 


> ---
> Changes in v6:
> - Qualify what is being mapped
> - Use PRI_mfn
> 
> Changes in v4:
> - Patch added
> ---
>  xen/arch/arm/mm.c |  2 +-
>  xen/arch/arm/p2m.c| 12 ++--
>  xen/include/asm-arm/p2m.h |  4 ++--
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 0e408f8..b5fc034 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1145,7 +1145,7 @@ int xenmem_add_to_physmap_one(
>  if ( extra.res0 )
>  return -EOPNOTSUPP;
>  
> -rc = map_dev_mmio_region(d, gfn_x(gfn), 1, idx);
> +rc = map_dev_mmio_region(d, gfn, 1, _mfn(idx));
>  return rc;
>  
>  default:
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index f11094e..5fe1b91 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1211,20 +1211,20 @@ int unmap_mmio_regions(struct domain *d,
>  }
>  
>  int map_dev_mmio_region(struct domain *d,
> -unsigned long start_gfn,
> +gfn_t gfn,
>  unsigned long nr,
> -unsigned long mfn)
> +mfn_t mfn)
>  {
>  int res;
>  
> -if ( !(nr && iomem_access_permitted(d, mfn, mfn + nr - 1)) )
> +if ( !(nr && iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + nr - 1)) 
> )
>  return 0;
>  
> -res = map_mmio_regions(d, _gfn(start_gfn), nr, _mfn(mfn));
> +res = map_mmio_regions(d, gfn, nr, mfn);
>  if ( res < 0 )
>  {
> -printk(XENLOG_G_ERR "Unable to map [%#lx - %#lx] in Dom%d\n",
> -   mfn, mfn + nr - 1, d->domain_id);
> +printk(XENLOG_G_ERR "Unable to map MFNs [%#"PRI_mfn" - %#"PRI_mfn" 
> in Dom%d\n",
> +   mfn_x(mfn), mfn_x(mfn) + nr - 1, d->domain_id);
>  return res;
>  }
>  
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index 4752161..8d29eda 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -152,9 +152,9 @@ int unmap_regions_rw_cache(struct domain *d,
> unsigned long mfn);
>  
>  int map_dev_mmio_region(struct domain *d,
> -unsigned long start_gfn,
> +gfn_t gfn,
>  unsigned long nr,
> -unsigned long mfn);
> +mfn_t mfn);
>  
>  int guest_physmap_add_entry(struct domain *d,
>  gfn_t gfn,
> -- 
> 1.9.1
> 

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

Re: [Xen-devel] [PATCH v7 14/14] xen/arm: p2m: Rework the interface of apply_p2m_changes and use typesafe

[Stefano Stabellini]

On Tue, 12 Jul 2016, Julien Grall wrote:
> Most of the callers of apply_p2m_changes have a GFN, a MFN and the
> number of frame to change in hand.
> 
> Rather than asking each caller to convert the frame to an address,
> rework the interfaces to pass the GFN, MFN and the number of frame.
> 
> Note that it would be possible to do more clean-up in apply_p2m_changes,
> but this will be done in a follow-up series.
> 
> Signed-off-by: Julien Grall 

Acked-by: Stefano Stabellini 


> Changes in v4:
> - Patch added
> ---
>  xen/arch/arm/p2m.c | 62 
> --
>  1 file changed, 28 insertions(+), 34 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index de2ab05..976f97b 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -906,25 +906,26 @@ static void update_reference_mapping(struct page_info 
> *page,
>  
>  static int apply_p2m_changes(struct domain *d,
>   enum p2m_operation op,
> - paddr_t start_gpaddr,
> - paddr_t end_gpaddr,
> - paddr_t maddr,
> + gfn_t sgfn,
> + unsigned long nr,
> + mfn_t smfn,
>   int mattr,
>   uint32_t mask,
>   p2m_type_t t,
>   p2m_access_t a)
>  {
> +paddr_t start_gpaddr = pfn_to_paddr(gfn_x(sgfn));
> +paddr_t end_gpaddr = pfn_to_paddr(gfn_x(sgfn) + nr);
> +paddr_t maddr = pfn_to_paddr(mfn_x(smfn));
>  int rc, ret;
>  struct p2m_domain *p2m = &d->arch.p2m;
>  lpae_t *mappings[4] = { NULL, NULL, NULL, NULL };
>  struct page_info *pages[4] = { NULL, NULL, NULL, NULL };
> -paddr_t addr, orig_maddr = maddr;
> +paddr_t addr;
>  unsigned int level = 0;
>  unsigned int cur_root_table = ~0;
>  unsigned int cur_offset[4] = { ~0, ~0, ~0, ~0 };
>  unsigned int count = 0;
> -const unsigned long sgfn = paddr_to_pfn(start_gpaddr),
> -egfn = paddr_to_pfn(end_gpaddr);
>  const unsigned int preempt_count_limit = (op == MEMACCESS) ? 1 : 0x2000;
>  const bool_t preempt = !is_idle_vcpu(current);
>  bool_t flush = false;
> @@ -986,9 +987,9 @@ static int apply_p2m_changes(struct domain *d,
>   * Preempt setting mem_access permissions as required by 
> XSA-89,
>   * if it's not the last iteration.
>   */
> -uint32_t progress = paddr_to_pfn(addr) - sgfn + 1;
> +uint32_t progress = paddr_to_pfn(addr) - gfn_x(sgfn) + 1;
>  
> -if ( (egfn - sgfn) > progress && !(progress & mask) )
> +if ( nr > progress && !(progress & mask) )
>  {
>  rc = progress;
>  goto out;
> @@ -1117,8 +1118,9 @@ static int apply_p2m_changes(struct domain *d,
>  
>  if ( op == INSERT )
>  {
> -p2m->max_mapped_gfn = gfn_max(p2m->max_mapped_gfn, _gfn(egfn));
> -p2m->lowest_mapped_gfn = gfn_min(p2m->lowest_mapped_gfn, _gfn(sgfn));
> +p2m->max_mapped_gfn = gfn_max(p2m->max_mapped_gfn,
> +  gfn_add(sgfn, nr));
> +p2m->lowest_mapped_gfn = gfn_min(p2m->lowest_mapped_gfn, sgfn);
>  }
>  
>  rc = 0;
> @@ -1127,7 +1129,7 @@ out:
>  if ( flush )
>  {
>  flush_tlb_domain(d);
> -ret = iommu_iotlb_flush(d, sgfn, egfn - sgfn);
> +ret = iommu_iotlb_flush(d, gfn_x(sgfn), nr);
>  if ( !rc )
>  rc = ret;
>  }
> @@ -1146,12 +1148,14 @@ out:
>  if ( rc < 0 && ( op == INSERT ) &&
>   addr != start_gpaddr )
>  {
> +unsigned long gfn = paddr_to_pfn(addr);
> +
>  BUG_ON(addr == end_gpaddr);
>  /*
>   * addr keeps the address of the end of the last 
> successfully-inserted
>   * mapping.
>   */
> -apply_p2m_changes(d, REMOVE, start_gpaddr, addr, orig_maddr,
> +apply_p2m_changes(d, REMOVE, sgfn, gfn - gfn_x(sgfn), smfn,
>mattr, 0, p2m_invalid, d->arch.p2m.default_access);
>  }
>  
> @@ -1164,10 +1168,7 @@ static inline int p2m_insert_mapping(struct domain *d,
>   mfn_t mfn,
>   int mattr, p2m_type_t t)
>  {
> -return apply_p2m_changes(d, INSERT,
> - pfn_to_paddr(gfn_x(start_gfn)),
> - pfn_to_paddr(gfn_x(start_gfn) + nr),
> - pfn_to_paddr(mfn_x(mfn)),
> +return apply_p2m_changes(d, INSERT, start_gfn, nr, mfn,
>   mattr, 0, t, d->arch.p2m.default_access);
>  }
>  
> @@ -1176,10 +1177,7 @@ static inline int p2m_remove_mapping(struct domain *d,
>   unsigned long nr,
>   mfn_t mfn)
>  {
> -retur