date:20190731

flight 139555 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139555/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-libvirt   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 133580
 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-freebsd10-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-freebsd10-i386  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-libvirt-xsm   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl-qemut-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot  fail REGR. vs. 133580
 build-armhf-pvops 6 kernel-build fail REGR. vs. 133580
 test-amd64-amd64-xl-qcow2   19 guest-start/debian.repeat fail REGR. vs. 133580
 test-arm64-arm64-libvirt-xsm 16 guest-start/debian.repeat fail REGR. vs. 133580

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   blocked  n/a
 test-armhf-armhf-examine  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  7 xen-boot fail baseline untested
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  7 xen-boot fail baseline untested
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13

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

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  d6d385de8c7be2ae2715de2a1348214dfe0791ee
baseline version:
 xen  2adc580bd59f5c3034fd6ecacd5748678373f17a

Last test of basis   139568  2019-07-31 15:00:46 Z0 days
Testing same since   139576  2019-07-31 20:00:52 Z0 days1 attempts


People who touched revisions under test:
  Andre Przywara 
  Julien Grall 
  Stefano Stabellini 
  Stewart Hildebrand 
  Viktor Mitin 
  Viktor Mitin 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 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 :

To xenbits.xen.org:/home/xen/git/xen.git
   2adc580bd5..d6d385de8c  d6d385de8c7be2ae2715de2a1348214dfe0791ee -> smoke

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

[Xen-devel] [freebsd-master test] 139559: all pass - PUSHED

flight 139559 freebsd-master real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139559/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 freebsd  58ae190bd3c0e785e5895867257c9926915fb818
baseline version:
 freebsd  51e3e3ac0b7ca4c2619be3df90d1c0a9ca3176d2

Last test of basis   139488  2019-07-29 09:27:26 Z2 days
Testing same since   139559  2019-07-31 09:19:32 Z0 days1 attempts


People who touched revisions under test:
  ae 
  alc 
  araujo 
  asomers 
  br 
  delphij 
  emaste 
  ian 
  jilles 
  kp 
  manu 
  markj 
  mav 
  oshogbo 
  tuexen 

jobs:
 build-amd64-freebsd-againpass
 build-amd64-freebsd  pass
 build-amd64-xen-freebsd  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 :

To xenbits.xen.org:/home/xen/git/freebsd.git
   51e3e3ac0b7..58ae190bd3c  58ae190bd3c0e785e5895867257c9926915fb818 -> 
tested/master

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

Re: [Xen-devel] [RFC] Generating Go bindings for libxl

2019-07-31 Thread Nicholas Rosbrook

> I return to the question I stated before.  At the moment, your bindings
> have the following call chain:
> 
> * DomainInfo(), hand-crafted.  Calls domainInfo().
> * domainInfo(), automaticall generated.  Calls C.libxl_domain_info().
> 
> The in-tree bindings have the following call chain:
> 
> * DomainInfo(), hand-crafted.  Calls C.libxl_domain_info().
> 
> Since DomainInfo() is hand-crafted in both cases, what's the advantage
> of having domainInfo() at all?

Point well taken.

> So just to clarify terminology: The IDL is the description language
> itself, which at the moment only contains information about the libxl
> structures.  We have generators for various C bits of libxl which read
> the IDL and spit out boilerplate C.  The idea would be that we write a
> new generator for Go which reads the IDL and spits out boilerplate Go.

Yes, I know. Sorry for the strange phrasing. I'll try again:

From what I understand, the IDL is only used to generate libxl types, and
the boiler-plate init, dispose, etc. functions. However, if we want to have a
generator that produces Go code that calls libxl functions, those function
signatures must be known during code generation. However, the description
of those functions is outside the scope of the IDL.

So, in order to write such a generator we would need to either:

1. Expand the IDL (significantly) so that function signatures could be described
in the general case.

2. Parse the C code.

With that said, what are your expectations for the generated Go code at this 
point?
Do you think we should try to generate the pieces that call into libxl? Or, do 
you think
the code generation should be limited to the structs and boiler-plate C <-> Go 
"type
marshaling?" 

> I looked at the thing about naked returns, and didn't really understand
> it; but anyway I'm happy to have things modified to be more Go-like.  I
> definitely "speak" Go with a funny accent.

TL;DR: Naked returns exist; don't use them (with the exception of defer'd 
closures if necessary).

> Can I say -- I've been going open-source for so long, that I feel almost
> unsafe when nobody reviews my stuff.  Most of this code was written by
> me and reviewed by nobody (since I was the only person interested); it's
> good to have someone else take a critical look at it.

Makes sense to me. Glad to be involved :)

> And if we had a an IDL for the libxl functions, we could have it
> generate the code above for the vast majority of cases.

I guess that answers my question above.

> If we wrote a generator from the IDL, we could make it smart enough to
> use []Disks as the type there, and make the marshallers know how to use
> num_disks to appropriately size the resulting slice and copy the right
> number of values across.  To do that with c-for-go, we'd have to do a
> lot of work teaching it what to do, if that's even possible.

Good point. AFAICT there isn't a way to provide such information to c-for-go.

> So you mean, for example, after DomainInfo() calls DomInfo.Deref(), it
> will then call libxl_dominfo_dispose() on the C struct?

Yes.

> Right, and personally I'm not in principle opposed to using c-for-go, if
> it can be made to generate code the way we like it.  My main fear is
> that we'll spend a bunch of time tweaking c-for-go, and after going back
> and forth and investing a lot of time in it, we'll end up either 1)
> giving up and writing our own generator anyway, or 2) accepting
> something sub-optimal because it's the best thing we could make c-for-go do.

I agree, those are legitimate concerns for using c-for-go. OTOH, one concern
I have for a custom generator is that it is indeed custom, and maybe wouldn't
be of much use outside of libxl. It would be great if the extra work in writing 
a new
generator meant that it could be used by other projects with similar needs. 
However,
I understand that concern may not be shared by others.

I think we have a decent enough idea for what a c-for-go version of this might 
look like. So,
what are the next steps in exploring the custom generator route?

-NR
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [linux-4.14 test] 139553: tolerable FAIL - PUSHED

flight 139553 linux-4.14 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139553/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 linux10d6aa565d0593fe4e152e49ab58f47a2952f902
baseline version:
 linuxff33472c282e209da54cbc0c7c1c06ddfcc93d33

Last test of basis   139242  2019-07-22 01:53:46 Z9 days
Testing same since   139553  2019-07-31 05:40:06 Z0 days1 attempts


343 people

Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used


Hi Stefano,

On 7/31/19 9:40 PM, Stefano Stabellini wrote:

On Tue, 30 Jul 2019, Julien Grall wrote:

Hi Stefano,

On 7/30/19 6:33 PM, Stefano Stabellini wrote:

On Thu, 27 Jun 2019, Julien Grall wrote:

On 6/27/19 7:55 PM, Stefano Stabellini wrote:

On Mon, 10 Jun 2019, Julien Grall wrote:

+1:
+/*
+ * Find the second slot used. Remove the entry for the first
+ * table if the slot is not 1 (runtime Xen mapping is 2M -
4M).
+ * For slot 1, it means the ID map was not created.
+ */
+lsr   x1, x19, #SECOND_SHIFT
+and   x1, x1, #LPAE_ENTRY_MASK  /* x1 := first slot */
+cmp   x1, #1
+beq   id_map_removed
+/* It is not in slot 1, remove the entry */
+ldr   x0, =boot_second  /* x0 := second table */
+str   xzr, [x0, x1, lsl #3]


Wouldn't it be a bit more reliable if we checked whether the slot in
question for x19 (whether zero, first, second) is a pagetable pointer or
section map, then zero it if it is a section map, otherwise go down one
level? If we did it this way it would be independent from the way
create_page_tables is written.


Your suggestion will not comply with the architecture compliance and how
Xen
is/will be working after the full rework. We want to remove everything
(mapping + table) added specifically for the 1:1 mapping.

Otherwise, you may end up in a position where boot_first_id is still in
place.
We would need to use the break-before-make sequence in subsequent code if
we
were about to insert 1GB mapping at the same place.

After my rework, we would have virtually no place where break-before-make
will
be necessary as it will enforce all the mappings to be destroyed before
hand.
So I would rather avoid to make a specific case for the 1:1 mapping.


I don't fully understand your explanation. I understand the final goal
of "removing everything (mapping + table) added specifically for the 1:1
mapping". I don't understand why my suggestion would be a hindrance
toward that goal, compared to what it is done in this patch.


Because, AFAICT, your suggestion will only remove the mapping and not the
tables (such as boot_first_id). This is different from this patch where both
mapping and tables are removed.

So yes, my suggestion is not generic, but at least it does the job that is
expected by this function. I.e removing anything that was specifically created
for the identity mapping.


I understand your comment now, and of course I agree that both mapping
and tables need to be removed.

I am careful making suggestions for assembly coding because I don't
really want to suggest something that doesn't work, or even if it works
that it's worse than the original.

It should be possible to remove both the table and the mapping in a
generic way. Instead of hardcoding the assemply equivalent of "It is not
in slot 0, remove the entry", we could check whether the table offset
matches the table offset of the mapping that we want to preserve. That
way, "slot 0" would be calculate instead of hardcoded, and the code
would be pretty generic. What do you think? It should only be a small
addition.


It should be feasible and may actually help the next step in my plan 
where I need to make Xen relocatable.


I will have a look for both the arm32 and arm64 code.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

On Wed, Jul 31, 2019 at 12:46 PM Andrew Cooper
 wrote:
>
> On 31/07/2019 20:35, Roman Shaposhnik wrote:
> > On Wed, Jul 31, 2019 at 1:43 AM Roger Pau Monné  
> > wrote:
> >> On Wed, Jul 31, 2019 at 10:36:31AM +0200, Roger Pau Monné wrote:
> >>> On Tue, Jul 30, 2019 at 10:55:24AM -0700, Roman Shaposhnik wrote:
>  Sorry -- got a bit distracted yesterday. Attached is the log with only
>  your latest patch attached. Interestingly enough the box booted fine
>  without screen artifacts. So I guess we're getting closer...
> 
>  Thanks for all the help!
> >>> That's quite weird, there's no functional changes between the
> >>> previous patches and this one, the only difference is that this patch
> >>> has more verbose output.
> >>>
> >>> Are you sure you didn't have any local patches on top of Xen that
> >>> could explain this difference in behaviour?
> >> FWIW, can you please try the plain patch again:
> >>
> >> https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01547.html
> >>
> >> And report back?
> >>
> >> I would like to get this committed ASAP if it does fix your issue.
> > I'd like to say that it did -- but I tried it again just now and it
> > still garbled screen and tons of:
> >
> > (XEN) printk: 26665 messages suppressed.
> > (XEN) [VT-D]DMAR:[DMA Read] Request device [:00:02.0] fault addr
> > 8e14c000, iommu reg = 82c0008de000
> >
> > I'm very much confused by what's going on, but it seems that's the
> > case -- adding those debug print statements make the issue go away
> >
> > Here are the patches that are being applied:
> >NOT WORKING:
> > https://github.com/rvs/eve/blob/xen-bug/pkg/xen/01-iommu-mappings.patch
> >
> >WORKING: 
> > https://github.com/rvs/eve/blob/a1291fcd4e669df2a63285afb5e8b4841f45c1c8/pkg/xen/01-iommu-mappings.patch
> >
> > At this point I'm really not sure what's going on.
>
> Ok.  seeing as you've double checked this, the mystery deepens.
>
> My bet is on the intel_iommu_lookup_page() call having side effects[1].
> If you take out the debugging in the middle of the loop in
> rmrr_identity_mapping(), does the problem reproduce again?
>
> ~Andrew
>
> [1] Looking at the internals of addr_to_dma_page_maddr(), it does 100%
> more memory allocation and higher-level PTE construction than looks wise
> for what is supposed to be a getter.

Yup. That's what it is -- intel_iommu_lookup_page() seems to be the culprit.

I've did the experiment in the other direction -- adding a dummy call:
 
https://github.com/rvs/eve/blob/36aeeaa7c0a53474fb1ecef2ff587a86637df858/pkg/xen/01-iommu-mappings.patch#L23
on top of original Roger's patch makes system boot NORMALLY.

Thanks,
Roman.

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

Re: [Xen-devel] [PATCH v2 34/35] xen/arm32: head: Setup HTTBR in enable_mmu() and add missing isb


Hi Stefano,

On 7/30/19 10:26 PM, Stefano Stabellini wrote:

On Mon, 22 Jul 2019, Julien Grall wrote:

At the moment, HTTBR is setup in create_page_tables(). This is fine as
it is called by every CPUs.

However, such assumption may not hold in the future. To make change
easier, the HTTBR is not setup in enable_mmu().

Take the opportunity to add the missing isb() to ensure the HTTBR is
seen before the MMU is turned on.

Signed-off-by: Julien Grall 

---
 Changes in v2:
 - Patch added
---
  xen/arch/arm/arm32/head.S | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 6d55a2119a..8a1e272aab 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -373,8 +373,6 @@ create_page_tables:
  /* Write Xen's PT's paddr into the HTTBR */


This comment needs to be moved


Good spot!





  ldr   r4, =boot_pgtable
  add   r4, r4, r10/* r4 := paddr (boot_pagetable) */
-mov   r5, #0 /* r4:r5 is paddr (boot_pagetable) */
-mcrr  CP64(r4, r5, HTTBR)


Interestingly r5 is not clobbered by create_page_tables anymore, we need
to update the comment on top.


I knew documenting the clobbered registers are going to cause some 
trouble when updating the code :). I will fix it in the next version.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 32/35] xen/arm32: head: Rework and document setup_fixmap()


Hi Stefano,

On 7/30/19 10:14 PM, Stefano Stabellini wrote:

On Mon, 22 Jul 2019, Julien Grall wrote:

At the moment, the fixmap table is only hooked when earlyprintk is used.
This is fine today because in C land, the fixmap is not used by anyone
until the the boot CPU is switching to the runtime page-tables.

In the future, the boot CPU will not switch between page-tables to
avoid TLB incoherency. Thus, the fixmap table will need to be always
hooked beofre any use. Let's start doing it now in setup_fixmap().

Lastly, document the behavior and the main registers usage within the
function.

Signed-off-by: Julien Grall 

---
 Changes in v2:
 - Patch added
---
  xen/arch/arm/arm32/head.S | 19 ---
  1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 56e2d09ed4..e0f8c2e0cb 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -536,8 +536,21 @@ identity_mapping_removed:
  mov   pc, lr
  ENDPROC(remove_identity_mapping)
  
+/*

+ * Map the UART in the fixmap (when earlyprintk is used) and hook the
+ * fixmap table in the page tables.
+ *
+ * The fixmap cannot be mapped in create_page_tables because it may
+ * clash with the 1:1 mapping.
+ *
+ * Inputs:
+ *   r10: Physical offset
+ *   r11: Early UART base physical address
+ *
+ * Clobbers r1 - r4
+ */
  setup_fixmap:
-#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
+#if defined(CONFIG_EARLY_PRINTK)
  /* Add UART to the fixmap table */
  ldr   r1, =xen_fixmap/* r1 := vaddr (xen_fixmap) */
  lsr   r2, r11, #THIRD_SHIFT
@@ -546,7 +559,7 @@ setup_fixmap:
  orr   r2, r2, #PT_LOWER(DEV_L3) /* r2:r3 := 4K dev map including UART 
*/
  mov   r3, #0x0
  strd  r2, r3, [r1, #(FIXMAP_CONSOLE*8)] /* Map it in the first 
fixmap's slot */
-1:
+#endif


Patch is OK. However, the 1: should be removed in the previous patch
"xen/arm32: head: Don't setup the fixmap on secondary CPUs", where we
took away the beq.


Good point. I have now moved this to the previous patch.



Reviewed-by: Stefano Stabellini 


Thank you!

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used

2019-07-31 Thread Stefano Stabellini

On Tue, 30 Jul 2019, Julien Grall wrote:
> Hi Stefano,
> 
> On 7/30/19 6:33 PM, Stefano Stabellini wrote:
> > On Thu, 27 Jun 2019, Julien Grall wrote:
> > > On 6/27/19 7:55 PM, Stefano Stabellini wrote:
> > > > On Mon, 10 Jun 2019, Julien Grall wrote:
> > > > > +1:
> > > > > +/*
> > > > > + * Find the second slot used. Remove the entry for the first
> > > > > + * table if the slot is not 1 (runtime Xen mapping is 2M -
> > > > > 4M).
> > > > > + * For slot 1, it means the ID map was not created.
> > > > > + */
> > > > > +lsr   x1, x19, #SECOND_SHIFT
> > > > > +and   x1, x1, #LPAE_ENTRY_MASK  /* x1 := first slot */
> > > > > +cmp   x1, #1
> > > > > +beq   id_map_removed
> > > > > +/* It is not in slot 1, remove the entry */
> > > > > +ldr   x0, =boot_second  /* x0 := second table */
> > > > > +str   xzr, [x0, x1, lsl #3]
> > > > 
> > > > Wouldn't it be a bit more reliable if we checked whether the slot in
> > > > question for x19 (whether zero, first, second) is a pagetable pointer or
> > > > section map, then zero it if it is a section map, otherwise go down one
> > > > level? If we did it this way it would be independent from the way
> > > > create_page_tables is written.
> > > 
> > > Your suggestion will not comply with the architecture compliance and how
> > > Xen
> > > is/will be working after the full rework. We want to remove everything
> > > (mapping + table) added specifically for the 1:1 mapping.
> > > 
> > > Otherwise, you may end up in a position where boot_first_id is still in
> > > place.
> > > We would need to use the break-before-make sequence in subsequent code if
> > > we
> > > were about to insert 1GB mapping at the same place.
> > > 
> > > After my rework, we would have virtually no place where break-before-make
> > > will
> > > be necessary as it will enforce all the mappings to be destroyed before
> > > hand.
> > > So I would rather avoid to make a specific case for the 1:1 mapping.
> > 
> > I don't fully understand your explanation. I understand the final goal
> > of "removing everything (mapping + table) added specifically for the 1:1
> > mapping". I don't understand why my suggestion would be a hindrance
> > toward that goal, compared to what it is done in this patch.
> 
> Because, AFAICT, your suggestion will only remove the mapping and not the
> tables (such as boot_first_id). This is different from this patch where both
> mapping and tables are removed.
>
> So yes, my suggestion is not generic, but at least it does the job that is
> expected by this function. I.e removing anything that was specifically created
> for the identity mapping.

I understand your comment now, and of course I agree that both mapping
and tables need to be removed.

I am careful making suggestions for assembly coding because I don't
really want to suggest something that doesn't work, or even if it works
that it's worse than the original.

It should be possible to remove both the table and the mapping in a
generic way. Instead of hardcoding the assemply equivalent of "It is not
in slot 0, remove the entry", we could check whether the table offset
matches the table offset of the mapping that we want to preserve. That
way, "slot 0" would be calculate instead of hardcoded, and the code
would be pretty generic. What do you think? It should only be a small
addition.

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

Re: [Xen-devel] [PATCH v2 24/35] xen/arm32: head: Introduce distinct paths for the boot CPU and secondary CPUs


Hi Stefano,

On 7/30/19 9:07 PM, Stefano Stabellini wrote:

On Mon, 22 Jul 2019, Julien Grall wrote:

The boot code is currently quite difficult to go through because of the
lack of documentation and a number of indirection to avoid executing
some path in either the boot CPU or secondary CPUs.

In an attempt to make the boot code easier to follow, each parts of the
boot are now in separate functions. Furthermore, the paths for the boot
CPU and secondary CPUs are now distinct and for now will call each
functions.

Follow-ups will remove unnecessary calls and do further improvement
(such as adding documentation and reshuffling).

Note that the switch from using the ID mapping to the runtime mapping
is duplicated for each path. This is because in the future we will need
to stay longer in the ID mapping for the boot CPU.

Lastly, it is now required to save lr in cpu_init() becauswe the
function will call other functions and therefore clobber lr.

Signed-off-by: Julien Grall 

---
 Changes in v2:
 - Patch added
---
  xen/arch/arm/arm32/head.S | 64 +++
  1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index bbcfdcd351..13793e85d8 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -148,7 +148,19 @@ past_zImage:
  
  mov   r12, #0/* r12 := is_secondary_cpu */
  
-b common_start

+blcheck_cpu_mode
+blzero_bss
+blcpu_init
+blcreate_page_tables
+blenable_mmu
+
+/* We are still in the ID map. Jump to the runtime Virtual Address */


The arm64 patch has been switched to use "1:1", it would be good to be
consistent. In the commit message too.



+ldr   r0, =primary_switched
+mov   pc, r0
+primary_switched:
+blsetup_fixmap
+b launch
+ENDPROC(start)
  
  GLOBAL(init_secondary)

  cpsid aif/* Disable all interrupts */
@@ -179,8 +191,21 @@ GLOBAL(init_secondary)
  print_reg r7
  PRINT(" booting -\r\n")
  #endif
-
-common_start:
+blcheck_cpu_mode
+blzero_bss
+blcpu_init
+blcreate_page_tables
+blenable_mmu
+
+/* We are still in the ID map. Jump to the runtime Virtual Address. */


Same here.



+ldr   r0, =secondary_switched
+mov   pc, r0
+secondary_switched:
+blsetup_fixmap
+b launch
+ENDPROC(init_secondary)
+
+check_cpu_mode:
  /* Check that this CPU has Hyp mode */
  mrc   CP32(r0, ID_PFR1)
  and   r0, r0, #0xf000/* Bits 12-15 define virt extensions */
@@ -202,7 +227,10 @@ common_start:
  b fail
  
  hyp:PRINT("- Xen starting in Hyp mode -\r\n")

+mov   pc, lr
+ENDPROC(check_cpu_mode)
  
+zero_bss:

  /* Zero BSS On the boot CPU to avoid nasty surprises */
  teq   r12, #0
  bne   skip_bss
@@ -219,8 +247,14 @@ hyp:PRINT("- Xen starting in Hyp mode -\r\n")
  blo   1b
  
  skip_bss:

+mov   pc, lr
+ENDPROC(zero_bss)
+
+cpu_init:
  PRINT("- Setting up control registers -\r\n")
  
+mov   r5, lr			/* r5 := return address */


Please align the comment with the others in this proc.


It looks like the line was containing some hard tab. I have replaced 
them with soft tab.




Other than these minor comments the patch looks fine. Have you had a
chance to test it on real hardware?


I actually didn't test the SMP path on the model until I sent it because 
I had some issues getting secondary CPU up. I wanted to get some 
feedback first. Hence the "lightly tested" in the cover letter :).


This series fully boot on the model and I still need to test on real 
hardware. I have a TC2 (Cortex-A15 + Cortex-A7) on my desk, so I will 
give it a try there first.


Anyway, I probably need to resend this patch to replace all the "mov pc, 
lr" with "bx lr"


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 33/35] xen/arm32: head: Rework and document launch()

2019-07-31 Thread Stefano Stabellini

On Tue, 30 Jul 2019, Julien Grall wrote:
> On 30/07/2019 22:21, Stefano Stabellini wrote:
> > On Mon, 22 Jul 2019, Julien Grall wrote:
> >> Boot CPU and secondary CPUs will use different entry point to C code. At
> >> the moment, the decision on which entry to use is taken within launch().
> >>
> >> In order to avoid using conditional instruction and make the call
> >> clearer, launch() is reworked to take in parameters the entry point and its
> >> arguments.
> >>
> >> Lastly, document the behavior and the main registers usage within the
> >> function.
> >>
> >> Signed-off-by: Julien Grall 
> >>
> >> ---
> >>  Changes in v2:
> >>  - Patch added
> >> ---
> >>   xen/arch/arm/arm32/head.S | 34 --
> >>   1 file changed, 24 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> >> index e0f8c2e0cb..6d55a2119a 100644
> >> --- a/xen/arch/arm/arm32/head.S
> >> +++ b/xen/arch/arm/arm32/head.S
> >> @@ -170,6 +170,11 @@ primary_switched:
> >>   /* Use a virtual address to access the UART. */
> >>   mov_w r11, EARLY_UART_VIRTUAL_ADDRESS
> >>   #endif
> >> +PRINT("- Ready -\r\n")
> >> +/* Setup the arguments for start_xen and jump to C world */
> >> +mov   r0, r10/* r0 := Physical offset */
> >> +mov   r1, r8 /* r1 := paddr(FDT) */
> >> +ldr   r2, =start_xen
> >>   b launch
> >>   ENDPROC(start)
> >>   
> >> @@ -234,6 +239,9 @@ secondary_switched:
> >>   /* Use a virtual address to access the UART. */
> >>   mov_w r11, EARLY_UART_VIRTUAL_ADDRESS
> >>   #endif
> >> +PRINT("- Ready -\r\n")
> >> +/* Jump to C world */
> >> +ldr   r2, =start_secondary
> >>   b launch
> >>   ENDPROC(init_secondary)
> >>   
> >> @@ -578,19 +586,25 @@ setup_fixmap:
> >>   mov   pc, lr
> >>   ENDPROC(setup_fixmap)
> >>   
> >> +/*
> >> + * Setup the initial stack and jump to the C world
> >> + *
> >> + * Inputs:
> >> + *   r0 : Argument 0 of the C function to call
> >> + *   r1 : Argument 1 of the C function to call
> >> + *   r2 : C entry point
> >> + *
> >> + * Clobbers r3
> >> + */
> >>   launch:
> >> -PRINT("- Ready -\r\n")
> >> -
> >> -ldr   r0, =init_data
> >> -add   r0, #INITINFO_stack/* Find the boot-time stack */
> >> -ldr   sp, [r0]
> >> +ldr   r3, =init_data
> >> +add   r3, #INITINFO_stack/* Find the boot-time stack */
> >> +ldr   sp, [r3]
> >>   add   sp, #STACK_SIZE/* (which grows down from the top). 
> >> */
> >>   sub   sp, #CPUINFO_sizeof/* Make room for CPU save record */
> >> -teq   r12, #0
> >> -moveq r0, r10/* Marshal args: - phys_offset */
> >> -moveq r1, r8 /*   - DTB address */
> >> -beq   start_xen  /* and disappear into the land of C 
> >> */
> >> -b start_secondary/* (to the appropriate entry point) 
> >> */
> >> +
> >> +/* Jump to C world */
> >> +   bxr2
> > 
> > Why bx?
> The only two other possible instructions would be:
> 1) blx r2: we don't need to save the return address
> 2) mov pc, r2: The Arm Arm recommends to use bx/blx instead of this.
> 
> So bx seems the best fit. Any other suggestion?
> 
> Also, I would probably replace all the "mov pc, lr" I added with "bx lr".

There is really no alternative to bx. I forgot that "b" doesn't support
a register as an operand.

Reviewed-by: Stefano Stabellini 

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

Re: [Xen-devel] [PATCH v2 22/35] xen/arm32: head: Rework UART initialization on boot CPU


Hi Stefano,

On 7/30/19 8:40 PM, Stefano Stabellini wrote:

On Mon, 22 Jul 2019, Julien Grall wrote:

@@ -497,11 +497,15 @@ ENTRY(switch_ttbr)
  
  #ifdef CONFIG_EARLY_PRINTK

  /*
- * Bring up the UART.
- * r11: Early UART base address
- * Clobbers r0-r2
+ * Initialize the UART. Should only be called on the boot CPU.
+ *
+ * Ouput:

   ^ this should be output, and in the arm64 patch too (already committed)


Most of the arm32 code is a copy and paste from arm64 with little 
adaptation! Hence the same typo :).


I have fixed the arm32 part directly in this patch and create a new one 
for fixing the arm64 part.





Reviewed-by: Stefano Stabellini 


Thank you!

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 16/35] xen/arm64: head: Rework and document launch()


Hi Stefano,

On 7/30/19 6:45 PM, Stefano Stabellini wrote:

On Mon, 22 Jul 2019, Julien Grall wrote:

Boot CPU and secondary CPUs will use different entry point to C code. At
the moment, the decision on which entry to use is taken within launch().

In order to avoid a branch for the decision and make the code clearer,
launch() is reworked to take in parameters the entry point and its
arguments.

Lastly, document the behavior and the main registers usage within the
function.

Signed-off-by: Julien Grall 

---
 Changes in v2:
 - Use x3 instead of x4
 - Add a clobbers section
---
  xen/arch/arm/arm64/head.S | 43 +++
  1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index f165dd61ca..7541635102 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -312,6 +312,11 @@ primary_switched:
  /* Use a virtual address to access the UART. */
  ldr   x23, =EARLY_UART_VIRTUAL_ADDRESS
  #endif
+PRINT("- Ready -\r\n")
+/* Setup the arguments for start_xen and jump to C world */
+mov   x0, x20/* x0 := Physical offset */
+mov   x1, x21/* x1 := paddr(FDT) */
+ldr   x2, =start_xen
  b launch
  ENDPROC(real_start)
  
@@ -374,6 +379,9 @@ secondary_switched:

  /* Use a virtual address to access the UART. */
  ldr   x23, =EARLY_UART_VIRTUAL_ADDRESS
  #endif
+PRINT("- Ready -\r\n")
+/* Jump to C world */
+ldr   x2, =start_secondary
  b launch
  ENDPROC(init_secondary)
  
@@ -732,23 +740,26 @@ setup_fixmap:

  ret
  ENDPROC(setup_fixmap)
  
+/*

+ * Setup the initial stack and jump to the C world
+ *
+ * Inputs:
+ *   x0 : Argument 0 of the C function to call
+ *   x1 : Argument 1 of the C function to call
+ *   x2 : C entry point
+ *
+ * Clobbers x3
+ */
  launch:
-PRINT("- Ready -\r\n")
-
-ldr   x0, =init_data
-add   x0, x0, #INITINFO_stack /* Find the boot-time stack */
-ldr   x0, [x0]
-add   x0, x0, #STACK_SIZE/* (which grows down from the top). */
-sub   x0, x0, #CPUINFO_sizeof /* Make room for CPU save record */
-mov   sp, x0
-
-cbnz  x22, 1f
-
-mov   x0, x20/* Marshal args: - phys_offset */
-mov   x1, x21/*   - FDT */
-b start_xen  /* and disappear into the land of C */
-1:
-b start_secondary/* (to the appropriate entry point) */
+ldr   x3, =init_data
+add   x3, x3, #INITINFO_stack /* Find the boot-time stack */
+ldr   x3, [x3]
+add   x3, x3, #STACK_SIZE/* (which grows down from the top). */

 ^ please move 1 space to the
 right

Aside from this minor code style thing


If I wanted to be picky, all the rest of the code in this file is 
indentation at column 38. So this line has the correct indentation but 
the two others are not. However, this would means there are not space 
between the instruction and the comment:


foobar/* ... */

So I guess, indenting at column 39 would be the best here, although I 
already know someone that will be unhappy with in the future ;).




Reviewed-by: Stefano Stabellini 


Thank you!

Cheers,

--
Julien Grall

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

[Xen-devel] [BUG] Assertion failed: !blk->legacy_dev

Hi!

Andrew reminded me that EVE has a weird in-tree patch for Xen's qemu
to deal with an issue we can't quite explain:

https://github.com/lf-edge/eve/blob/master/pkg/xen-tools/patches-4.12.0/01-remove-assert.patch

The way this problem manifests itself is *sometime after* an HVM domain
with a qcow2 disk attached get started we get QEMU failing with:
Assertion failed: !blk->legacy_dev
(/xen/tools/qemu-xen/block/block-backend.c: blk_get_attached_dev_id:
903)

The domain configuration is super, plain vanilla (see xl.cfg below) and the most
annoying thing is that after inspecting qemu source I can't possibly understand
how this is happening. After all,  blk_attach_dev_legacy() is
literally the only place
where that variable is being set to true and I don't quite understand how do we
end up there.

Now, you may say that disabling that assertion is not the right fix --
which I would
totally agree with. However, we've been running like this for a few months now
and it doesn't seem to be causing any kind of cascading errors.

Thanks,
Roman.

name = "roman-container.1"
type = "hvm"
uuid = "f98b451a-67e2-4323-9194-0323cfcaf319"
memory = 1024
maxmem = 1024
vcpus = 1
maxcpus = 1
boot = "dc"
disk = 
['/persist/sha512-50c181f684ff7d86307e57398a4ba7ca38f1f18996e71929fe291758de6b9bcd,8,xvda,rw']
vif = []
serial = ['pty']

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

Re: [Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

On 31/07/2019 20:35, Roman Shaposhnik wrote:
> On Wed, Jul 31, 2019 at 1:43 AM Roger Pau Monné  wrote:
>> On Wed, Jul 31, 2019 at 10:36:31AM +0200, Roger Pau Monné wrote:
>>> On Tue, Jul 30, 2019 at 10:55:24AM -0700, Roman Shaposhnik wrote:
 Sorry -- got a bit distracted yesterday. Attached is the log with only
 your latest patch attached. Interestingly enough the box booted fine
 without screen artifacts. So I guess we're getting closer...

 Thanks for all the help!
>>> That's quite weird, there's no functional changes between the
>>> previous patches and this one, the only difference is that this patch
>>> has more verbose output.
>>>
>>> Are you sure you didn't have any local patches on top of Xen that
>>> could explain this difference in behaviour?
>> FWIW, can you please try the plain patch again:
>>
>> https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01547.html
>>
>> And report back?
>>
>> I would like to get this committed ASAP if it does fix your issue.
> I'd like to say that it did -- but I tried it again just now and it
> still garbled screen and tons of:
>
> (XEN) printk: 26665 messages suppressed.
> (XEN) [VT-D]DMAR:[DMA Read] Request device [:00:02.0] fault addr
> 8e14c000, iommu reg = 82c0008de000
>
> I'm very much confused by what's going on, but it seems that's the
> case -- adding those debug print statements make the issue go away
>
> Here are the patches that are being applied:
>NOT WORKING:
> https://github.com/rvs/eve/blob/xen-bug/pkg/xen/01-iommu-mappings.patch
>
>WORKING: 
> https://github.com/rvs/eve/blob/a1291fcd4e669df2a63285afb5e8b4841f45c1c8/pkg/xen/01-iommu-mappings.patch
>
> At this point I'm really not sure what's going on.

Ok.  seeing as you've double checked this, the mystery deepens.

My bet is on the intel_iommu_lookup_page() call having side effects[1]. 
If you take out the debugging in the middle of the loop in
rmrr_identity_mapping(), does the problem reproduce again?

~Andrew

[1] Looking at the internals of addr_to_dma_page_maddr(), it does 100%
more memory allocation and higher-level PTE construction than looks wise
for what is supposed to be a getter.

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

Re: [Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

On Wed, Jul 31, 2019 at 2:46 AM Jan Beulich  wrote:
>
> On 31.07.2019 10:58, Andrew Cooper wrote:
> > On 31/07/2019 09:34, Jan Beulich wrote:
> >> On 30.07.2019 19:56, Roman Shaposhnik wrote:
> >>> On Fri, Jul 26, 2019 at 1:06 AM Jan Beulich  wrote:
>  On 23.07.2019 20:25, Roman Shaposhnik wrote:
> > Interestingly enough, adding iommu_inclusive_mapping=1 AND iommu=debug
> > booted the system just fine.
>  Btw (I've noticed this only now) - are you saying without "iommu=debug"
>  the box does _not_ boot fine, despite the other option?
> >>> Yes. But it made sense to me since iommu=debug (as per your
> >>> explanation) overwhelms the CPU and I guess adding
> >>> iommu_inclusive_mapping=1 avoids the code path that does it?
> >> I'm afraid I don't follow: My question was whether
> >> "iommu_inclusive_mapping=1" alone would not allow the box to boot.
> >> Without "iommu=debug" there's no excessive logging afaict, no
> >> matter what other IOMMU options you use.
> >
> > Without inclusive mappings, the system boots but the screen is junk and
> > there are DMA faults all over the place.  With inclusive mappings, it
> > all "works" fine.
> >
> > With debug enabled, the DMA faults are spat out to the console for a
> > short while, until an APIC error occurs and the system wedges completely.
>
> Right - the verbosity _with_ "iommu=debug" may be a problem. Hence me
> wondering whether the system indeed wouldn't boot _without_ that option.

Without iommu=debug (and any other option) the screen is garbled.

Thanks,
Roman.

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

Re: [Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

On Wed, Jul 31, 2019 at 1:43 AM Roger Pau Monné  wrote:
>
> On Wed, Jul 31, 2019 at 10:36:31AM +0200, Roger Pau Monné wrote:
> > On Tue, Jul 30, 2019 at 10:55:24AM -0700, Roman Shaposhnik wrote:
> > > Sorry -- got a bit distracted yesterday. Attached is the log with only
> > > your latest patch attached. Interestingly enough the box booted fine
> > > without screen artifacts. So I guess we're getting closer...
> > >
> > > Thanks for all the help!
> >
> > That's quite weird, there's no functional changes between the
> > previous patches and this one, the only difference is that this patch
> > has more verbose output.
> >
> > Are you sure you didn't have any local patches on top of Xen that
> > could explain this difference in behaviour?
>
> FWIW, can you please try the plain patch again:
>
> https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01547.html
>
> And report back?
>
> I would like to get this committed ASAP if it does fix your issue.

I'd like to say that it did -- but I tried it again just now and it
still garbled screen and tons of:

(XEN) printk: 26665 messages suppressed.
(XEN) [VT-D]DMAR:[DMA Read] Request device [:00:02.0] fault addr
8e14c000, iommu reg = 82c0008de000

I'm very much confused by what's going on, but it seems that's the
case -- adding those debug print statements make the issue go away

Here are the patches that are being applied:
   NOT WORKING:
https://github.com/rvs/eve/blob/xen-bug/pkg/xen/01-iommu-mappings.patch

   WORKING: 
https://github.com/rvs/eve/blob/a1291fcd4e669df2a63285afb5e8b4841f45c1c8/pkg/xen/01-iommu-mappings.patch

At this point I'm really not sure what's going on.

Thanks,
Roman.

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

Re: [Xen-devel] [BUG] After upgrade to Xen 4.12.0 iommu=no-igfx

On Wed, Jul 31, 2019 at 1:36 AM Roger Pau Monné  wrote:
>
> On Tue, Jul 30, 2019 at 10:55:24AM -0700, Roman Shaposhnik wrote:
> > Sorry -- got a bit distracted yesterday. Attached is the log with only
> > your latest patch attached. Interestingly enough the box booted fine
> > without screen artifacts. So I guess we're getting closer...
> >
> > Thanks for all the help!
>
> That's quite weird, there's no functional changes between the
> previous patches and this one, the only difference is that this patch
> has more verbose output.

That's super weird indeed. I'm re-trying your very first patch right
now to see if it may be a pilot error on my part (hope not).

> Are you sure you didn't have any local patches on top of Xen that
> could explain this difference in behaviour?

Pretty sure -- but let me push my branch in EVE and show to you all
(you will need Docker installed to build Xen the way EVE does).

But if you want to check for yourself:
   $ git clone https://github.com/rvs/eve.git -b xen-bug
   $ cd eve/pkg/xen
   $ docker build  --no-cache  .
   $ docker export $(docker create XXX a) | tar xvf - boot
   $ ls boot/xen.gz

Thanks,
Roman.

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

Re: [Xen-devel] [PATCH v2 0/2] Raspberry Pi 4 support


Hi,

On 7/31/19 1:05 PM, Julien Grall wrote:

On 29/07/2019 14:19, Stewart Hildebrand wrote:
This is a series to enable UART console for Raspberry Pi 4. Note that 
I'm relying on the firmware to initialize the UART (i.e. enable_uart=1 
in config.txt), since full UART initialization on this platform 
requires accessing some registers outside the range specified in the 
brcm,bcm2835-aux-uart node.


I have been able to get Xen+dom0+domUs booting. Tested with Xen 4.12 
and 4.13-unstable (b4c8a27d5b) and Linux 4.19.y (Raspberry Pi linux 
tree + a couple of patches). Please see [1] for build instructions and 
limitations.


New in v2:
* Drop early printk alias
* Set reg-shift and reg-io-width in the Xen driver
* Blacklist other aux peripherals in platform settings (spi1, spi2, 
and a couple of base aux registers)


Thanks,
Stewart Hildebrand
DornerWorks, Ltd

[1] https://github.com/dornerworks/xen-rpi4-builder

Stewart Hildebrand (2):
   ns16550: Add compatible string for Raspberry Pi 4


I have committed this patch...


   xen/arm: platform: Add Raspberry Pi platform
... this one need an answer regarding the impact on blacklist spi1 and 
spi2.


This patch is now merged as well. Thank you for adding support for the RPI4!

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] CODING_STYLE: clarify function argument indentation

On 31/07/2019 18:49, Volodymyr Babchuk wrote:
> Hi Andrew,
>
> Andrew Cooper writes:
>
>> On 31/07/2019 17:24, Volodymyr Babchuk wrote:
>>> There are coding style rules that are widely accepted by community,
>>> but newer were formalized in the document. Notable example is the
>>> question on how function arguments and parameters should be indented
>>> when they do not fit into one line.
>>>
>>> This question was raised multiple times lately, mostly because of
>>> ongoing efforts to create Xen coding style formatting tool and because
>>> of new community members, who are not aware of such unwritten rules.
>>>
>>> Actually, this rule is already implicitly defined in the document by
>>> defining emacs coding style: 'c-file-style: "BSD"'. In this mode emacs
>>> lines up function arguments under the first argument. Naturally, most
>>> of Xen code is written in this style.
>>>
>>> So, lets state the obvious and fix this rule explicitly.
>>>
>>> Signed-off-by: Volodymyr Babchuk 
>>> ---
>>>  CODING_STYLE | 14 ++
>>>  1 file changed, 14 insertions(+)
>>>
>>> diff --git a/CODING_STYLE b/CODING_STYLE
>>> index 6cc5b774cf..6479215a15 100644
>>> --- a/CODING_STYLE
>>> +++ b/CODING_STYLE
>>> @@ -53,6 +53,20 @@ Line Length
>>>  Lines should be less than 80 characters in length.  Long lines should
>>>  be split at sensible places and the trailing portions indented.
>>>
>>> +For multiline function declaration and call each new line should be
>>> +aligned with the first the parameter or argument. e.g.:
>>> +
>>> +void my_function_with_long_name(struct lengthy_struct_name *struct1,
>>> +struct lengthy_struct_name *struct2,
>>> +struct lengthy_struct_name *struct3);
>>> +
>>> +or
>>> +
>>> +function_with_so_many_params(wordy_parameter1, wordy_parameter2,
>>> + wordy_parameter3, wordy_parameter4);
>>> +
>>> +The same applies for macros.
>> For very wordy functions, or ones with silly quantities of parameters,
>> the following is also acceptable
>>
>> void my_function_with_long_and_silly_name(
>>  struct lengthy_struct_name *struct1, unsigned int womble, unsigned
>> int whatsit,
>>  struct lengthy_struct_name *struct2, bool yes, bool no, bool maybe,
>>  bool file_not_found, struct lengthy_struct_name *struct3, struct
>> lengthy_struct_name *struct4);
>>
>> which you will find in a few places throughout the code, because the
>> above doesn't waste enough vertical space to fit several functions in,
>> and push all the relevant details to the RHS.
> Excuse me, what it RHS?

Right Hand Side.

Sorry - I was being lazy when typing.

>
>> Per the above rules, the result would be this:
>>
>> void my_function_with_long_and_silly_name(struct lengthy_struct_name
>> *struct1,
>>  unsigned int womble,
>>  unsigned int whatsit,
>>  struct lengthy_struct_name
>> *struct2,
>>  bool yes, bool no, bool maybe,
>>  bool file_not_found,
>>  struct lengthy_struct_name
>> *struct3,
>>  struct lengthy_struct_name
>> *struct4);
>>
>> Of course, this is also a sign that maybe the function signature wants
>> changing anyway, but that may not be possible/sensible at the time.
>>
>> As with everything, the coding style is a set of guidelines which are
>> applicable to 98% of cases, but there are cases where aren't
>> appropriate, and common sense is the only reasonable deciding factor.
> I totally agree with you. Probably we should either add a generic clause
> like "This coding style rules may be violated if they produce weird
> results".

We should have a general clause (rather than specific ones), but I'd be
hesitant to word it like that.

How about:

These guidelines are expected to be applicable to all circumstances.  If
the result looks weird, consider whether this is the wisest way to solve
the problem in the first place, or whether an exception may be warranted.

The advantage here is if we see the same kind of exceptions being
requested repeatedly, then perhaps this is a hint that the coding style
should be modified.

> Or we can add clarification to this particular rule: "Do not break
> parameter definition to multiple lines. If parameters are too long,
> decrease indentation, but try to line them up. e.g.:
>
> void my_function_with_long_and_silly_name(
> struct lengthy_struct_name *struct1,
> unsigned int womble,
> unsigned int whatsit,
> struct lengthy_struct_name *struct2,
> bool yes, bool no, bool maybe,
> bool file_not_found,
> struct lengthy_struct_name *struct3,
> struct lengthy_struct_name *struct4);
> "
>
> What do you think?

The specific example I gave used exactly 4 spaces, consistent with the
rest of the style.

At the point that we are trying to reclaim space, reclaiming as much as
possible is the obvious move.

The above case is actually easy to spot in an automated fashion
(function declaration/call, open bracket, newline, indentation by one
block, subsequent

Re: [Xen-devel] [PATCH] CODING_STYLE: clarify function argument indentation


Hi Lars,

Lars Kurth writes:

>> On 31 Jul 2019, at 17:54, Viktor Mitin  wrote:
>> 
>> Hi All,
>> 
>> On Wed, Jul 31, 2019 at 7:45 PM Andrew Cooper  
>> wrote:
>>> 
>>> On 31/07/2019 17:24, Volodymyr Babchuk wrote:

[...]

> Ultimately we have to make some trade-offs as to what is more important:
> a) automatic style checking - which means "common sense" can't be formalised 
> and there will be boundary cases like the above
> b) reclaiming code review bandwidth through automation or going for a labour 
> intensive manual approach
I like the linux kernel approach.  checkpatch.pl produces errors, which are
"no go", but it also produces warnings for such boundary cases, for
maintainer/reviewer to decide.

> I suggest we discuss in tomorrow's community call how to approach
> this.
Good idea, I'll attend.

> I think the most important first step is to have a good view on the kind of 
> boundary cases that we may face
Then we need some volunteer who'll try to cover all corner cases.

-- 
Volodymyr Babchuk at EPAM
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] CODING_STYLE: clarify function argument indentation


Hi Andrew,

Andrew Cooper writes:

> On 31/07/2019 17:24, Volodymyr Babchuk wrote:
>> There are coding style rules that are widely accepted by community,
>> but newer were formalized in the document. Notable example is the
>> question on how function arguments and parameters should be indented
>> when they do not fit into one line.
>>
>> This question was raised multiple times lately, mostly because of
>> ongoing efforts to create Xen coding style formatting tool and because
>> of new community members, who are not aware of such unwritten rules.
>>
>> Actually, this rule is already implicitly defined in the document by
>> defining emacs coding style: 'c-file-style: "BSD"'. In this mode emacs
>> lines up function arguments under the first argument. Naturally, most
>> of Xen code is written in this style.
>>
>> So, lets state the obvious and fix this rule explicitly.
>>
>> Signed-off-by: Volodymyr Babchuk 
>> ---
>>  CODING_STYLE | 14 ++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/CODING_STYLE b/CODING_STYLE
>> index 6cc5b774cf..6479215a15 100644
>> --- a/CODING_STYLE
>> +++ b/CODING_STYLE
>> @@ -53,6 +53,20 @@ Line Length
>>  Lines should be less than 80 characters in length.  Long lines should
>>  be split at sensible places and the trailing portions indented.
>>
>> +For multiline function declaration and call each new line should be
>> +aligned with the first the parameter or argument. e.g.:
>> +
>> +void my_function_with_long_name(struct lengthy_struct_name *struct1,
>> +struct lengthy_struct_name *struct2,
>> +struct lengthy_struct_name *struct3);
>> +
>> +or
>> +
>> +function_with_so_many_params(wordy_parameter1, wordy_parameter2,
>> + wordy_parameter3, wordy_parameter4);
>> +
>> +The same applies for macros.
>
> For very wordy functions, or ones with silly quantities of parameters,
> the following is also acceptable
>
> void my_function_with_long_and_silly_name(
>  struct lengthy_struct_name *struct1, unsigned int womble, unsigned
> int whatsit,
>  struct lengthy_struct_name *struct2, bool yes, bool no, bool maybe,
>  bool file_not_found, struct lengthy_struct_name *struct3, struct
> lengthy_struct_name *struct4);
>
> which you will find in a few places throughout the code, because the
> above doesn't waste enough vertical space to fit several functions in,
> and push all the relevant details to the RHS.
Excuse me, what it RHS?

>
> Per the above rules, the result would be this:
>
> void my_function_with_long_and_silly_name(struct lengthy_struct_name
> *struct1,
>  unsigned int womble,
>  unsigned int whatsit,
>  struct lengthy_struct_name
> *struct2,
>  bool yes, bool no, bool maybe,
>  bool file_not_found,
>  struct lengthy_struct_name
> *struct3,
>  struct lengthy_struct_name
> *struct4);
>
> Of course, this is also a sign that maybe the function signature wants
> changing anyway, but that may not be possible/sensible at the time.
>
> As with everything, the coding style is a set of guidelines which are
> applicable to 98% of cases, but there are cases where aren't
> appropriate, and common sense is the only reasonable deciding factor.

I totally agree with you. Probably we should either add a generic clause
like "This coding style rules may be violated if they produce weird
results".

Or we can add clarification to this particular rule: "Do not break
parameter definition to multiple lines. If parameters are too long,
decrease indentation, but try to line them up. e.g.:

void my_function_with_long_and_silly_name(
struct lengthy_struct_name *struct1,
unsigned int womble,
unsigned int whatsit,
struct lengthy_struct_name *struct2,
bool yes, bool no, bool maybe,
bool file_not_found,
struct lengthy_struct_name *struct3,
struct lengthy_struct_name *struct4);
"

What do you think?

Problem is that document will blow up quickly if we'll try to cover all
corner cases. So personally I stick to "generic rules + common sense"
approach.

--
Volodymyr Babchuk at EPAM
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  2adc580bd59f5c3034fd6ecacd5748678373f17a
baseline version:
 xen  78cc87ce498bf621914c0554b83fac3ee00d

Last test of basis   139561  2019-07-31 11:00:34 Z0 days
Testing same since   139568  2019-07-31 15:00:46 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Brian Woods 
  George Dunlap 
  James Wang 
  Jan Beulich 
  Jin Nan Wang 
  Norbert Manthey 
  Paul Durrant 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 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 :

To xenbits.xen.org:/home/xen/git/xen.git
   78cc87..2adc580bd5  2adc580bd59f5c3034fd6ecacd5748678373f17a -> smoke

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

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

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-ovmf-amd64 18 guest-start/debianhvm.repeat fail 
REGR. vs. 139300
 test-amd64-i386-xl-qemuu-ovmf-amd64 18 guest-start/debianhvm.repeat fail REGR. 
vs. 139300

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 139300
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 139300
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 139300
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 139300
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 139300
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 qemuu3bd6cbbb181b6ae60a1d1f33ccd325b45f71aa2a
baseline version:
 qemuubf8b024372bf8abf5a9f40bfa65eeefad23ff988

Last test of basis   139300  2019-07-24 03:20:41 Z7 days
Failing since139335  2019-07-25 11:37:28 Z6 days9 attempts
Testing same since   139548  2019-07-31 03:12:15 Z0 days1 attempts


People who touched revisions under test:
  Alistair Francis 
  Andrey Shinkevich 
  Christian Borntraeger 
  Cornelia Huck 
  Damien Hedde 
  David Gibson 
  David

Re: [Xen-devel] [PATCH] CODING_STYLE: clarify function argument indentation

2019-07-31 Thread Lars Kurth



> On 31 Jul 2019, at 17:54, Viktor Mitin  wrote:
> 
> Hi All,
> 
> On Wed, Jul 31, 2019 at 7:45 PM Andrew Cooper  
> wrote:
>> 
>> On 31/07/2019 17:24, Volodymyr Babchuk wrote:
>>> There are coding style rules that are widely accepted by community,
>>> but newer were formalized in the document. Notable example is the
>>> question on how function arguments and parameters should be indented
>>> when they do not fit into one line.
>>> 
>>> This question was raised multiple times lately, mostly because of
>>> ongoing efforts to create Xen coding style formatting tool and because
>>> of new community members, who are not aware of such unwritten rules.
>>> 
>>> Actually, this rule is already implicitly defined in the document by
>>> defining emacs coding style: 'c-file-style: "BSD"'. In this mode emacs
>>> lines up function arguments under the first argument. Naturally, most
>>> of Xen code is written in this style.
>>> 
>>> So, lets state the obvious and fix this rule explicitly.
>>> 
>>> Signed-off-by: Volodymyr Babchuk 
>>> ---
>>> CODING_STYLE | 14 ++
>>> 1 file changed, 14 insertions(+)
>>> 
>>> diff --git a/CODING_STYLE b/CODING_STYLE
>>> index 6cc5b774cf..6479215a15 100644
>>> --- a/CODING_STYLE
>>> +++ b/CODING_STYLE
>>> @@ -53,6 +53,20 @@ Line Length
>>> Lines should be less than 80 characters in length.  Long lines should
>>> be split at sensible places and the trailing portions indented.
>>> 
>>> +For multiline function declaration and call each new line should be
>>> +aligned with the first the parameter or argument. e.g.:
>>> +
>>> +void my_function_with_long_name(struct lengthy_struct_name *struct1,
>>> +struct lengthy_struct_name *struct2,
>>> +struct lengthy_struct_name *struct3);
>>> +
>>> +or
>>> +
>>> +function_with_so_many_params(wordy_parameter1, wordy_parameter2,
>>> + wordy_parameter3, wordy_parameter4);
>>> +
>>> +The same applies for macros.
>> 
>> For very wordy functions, or ones with silly quantities of parameters,
>> the following is also acceptable
>> 
>> void my_function_with_long_and_silly_name(
>>struct lengthy_struct_name *struct1, unsigned int womble, unsigned
>> int whatsit,
>>struct lengthy_struct_name *struct2, bool yes, bool no, bool maybe,
>>bool file_not_found, struct lengthy_struct_name *struct3, struct
>> lengthy_struct_name *struct4);
>> 
>> which you will find in a few places throughout the code, because the
>> above doesn't waste enough vertical space to fit several functions in,
>> and push all the relevant details to the RHS.
>> 
>> Per the above rules, the result would be this:
>> 
>> void my_function_with_long_and_silly_name(struct lengthy_struct_name
>> *struct1,
>>  unsigned int womble,
>>  unsigned int whatsit,
>>  struct lengthy_struct_name
>> *struct2,
>>  bool yes, bool no, bool maybe,
>>  bool file_not_found,
>>  struct lengthy_struct_name
>> *struct3,
>>  struct lengthy_struct_name
>> *struct4);
>> 
>> Of course, this is also a sign that maybe the function signature wants
>> changing anyway, but that may not be possible/sensible at the time.
>> 
>> As with everything, the coding style is a set of guidelines which are
>> applicable to 98% of cases, but there are cases where aren't
>> appropriate, and common sense is the only reasonable deciding factor.
> 
> It might be hard to automate 'common sense' cases. It seems it would
> be easier to find a solution on how to avoid such 'common sense'
> cases.
> 
> One more open point with this rule is how to format the next case where:
> len(return type string)+len(function name)+len(any of parameter) > 79
> 
> For example:
> +some_long_return_type  my_function_with_long_name(struct
> lengthy_struct_name *struct1,
> +struct lengthy_struct_name *struct2,
> +struct lengthy_struct_name *struct3);
> 
> Thanks

Ultimately we have to make some trade-offs as to what is more important:
a) automatic style checking - which means "common sense" can't be formalised 
and there will be boundary cases like the above
b) reclaiming code review bandwidth through automation or going for a labour 
intensive manual approach

I suggest we discuss in tomorrow's community call how to approach this.
I think the most important first step is to have a good view on the kind of 
boundary cases that we may face

Lars 


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

Re: [Xen-devel] Xen Coding style and clang-format

On Wed, Jul 31, 2019 at 7:27 PM Lars Kurth  wrote:

> Viktor: thank you for spending time on this
>
> I added an item to community call tomorrow and CC'ed you in the invite. So I 
> think what we need to do is figure out a way on how to make the coding 
> standard enforceable by a coding standard checker such as proposed here. 
> AFAICT
> * It seems there are some undocumented coding standard rules, which are 
> essentially causing problems with the tool
> * In addition, the fact that the LLVM coding style is the baseline for the 
> checks may also create some problems with false standard violations
>
> My instinct would be to try and document any undocumented rules on a scratch 
> space (e.g. google doc), look at differences between Xen and LLVM formatting 
> style and then make decisions using a voting mechanism to avoid 
> bike-shedding. In some cases discussion may be necessary though
>
> It would be good if you could attend, but I think we can do without you, if 
> needed
>

Lars, thank you for the invitation. I will try to join the call.
Seems the topic is not a simple one, there are a lot of things to discuss it.

Thanks

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

[Xen-devel] [PATCH 1/3] automation: try to keep openSUSE Leap image a little smaller

Using `--no-recommends` when updating or installing commands should
prevent non strictly necessary packages to be installed.

doing a `clean -a` after installing all the packages, should, in
theory, free more space (as opposed to using just `clean`).

Signed-off-by: Dario Faggioli 
---
Cc: Doug Goldstein 
Cc: Wei Liu 
---
 automation/build/suse/opensuse-leap.dockerfile |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/automation/build/suse/opensuse-leap.dockerfile 
b/automation/build/suse/opensuse-leap.dockerfile
index 614a5c8405..eb70419002 100644
--- a/automation/build/suse/opensuse-leap.dockerfile
+++ b/automation/build/suse/opensuse-leap.dockerfile
@@ -7,8 +7,8 @@ ENV USER root
 RUN mkdir /build
 WORKDIR /build
 
-RUN zypper ref && zypper up -y
-RUN zypper install -y \
+RUN zypper ref && zypper up -y --no-recommends
+RUN zypper install -y --no-recommends \
 acpica \
 bc \
 bin86 \
@@ -64,4 +64,4 @@ RUN zypper install -y \
 xz-devel \
 zlib-devel \
 && \
-zypper clean
+zypper clean -a


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

[Xen-devel] [PATCH 2/3] automation: add openSUSE Tumbleweed CI image

openSUSE comes in two flavours: Leap, which is non-rolling, and released
annualy, and Tumbleweed, which is rolling.

Reasons why it makes sense to have both (despite both being openSUSE,
package lists in dockerfiles being quite similar, etc) are:
- Leap share a lot with SUSE Linux Enterprise. So, regressions on Leap,
  not only means regressions for all openSUSE Leap users, but also helps
  prevent/catch regressions on SLE;
- Tumbleweed often has the most bleeding-edge software, so it will help
  us prevent/catch regressions with newly released versions of
  libraries, compilers, etc (e.g., at the time of writing this commit,
  some build issues, with GCC9, where discovered while trying to build
  in a Tumbleweed image).

Note that, considering the rolling nature of Tumbleweed, the container
would need to be rebuilt (e.g., periodically), even if the docker file
does not change.

Signed-off-by: Dario Faggioli 
---
Cc: Doug Goldstein 
Cc: Wei Liu 
---
 .../build/suse/opensuse-tumbleweed.dockerfile  |   68 
 1 file changed, 68 insertions(+)
 create mode 100644 automation/build/suse/opensuse-tumbleweed.dockerfile

diff --git a/automation/build/suse/opensuse-tumbleweed.dockerfile 
b/automation/build/suse/opensuse-tumbleweed.dockerfile
new file mode 100644
index 00..2676a87c85
--- /dev/null
+++ b/automation/build/suse/opensuse-tumbleweed.dockerfile
@@ -0,0 +1,68 @@
+FROM opensuse/tumbleweed
+LABEL maintainer.name="The Xen Project" \
+  maintainer.email="xen-devel@lists.xenproject.org"
+
+ENV USER root
+
+RUN mkdir /build
+WORKDIR /build
+
+RUN zypper ref && zypper up -y --no-recommends
+RUN zypper install -y --no-recommends \
+acpica \
+bc \
+bin86 \
+bison \
+bzip2 \
+checkpolicy \
+clang \
+cmake \
+dev86 \
+discount \
+flex \
+gcc \
+gcc-c++ \
+gettext-tools \
+git \
+glib2-devel \
+glibc-devel \
+glibc-devel-32bit \
+gzip \
+hostname \
+libSDL2-devel \
+libaio-devel \
+libbz2-devel \
+libext2fs-devel \
+libgnutls-devel \
+libjpeg62-devel \
+libnl3-devel \
+libnuma-devel \
+libpixman-1-0-devel \
+libpng16-devel \
+libssh2-devel \
+libtasn1-devel \
+libuuid-devel \
+libyajl-devel \
+lzo-devel \
+make \
+nasm \
+ncurses-devel \
+ocaml \
+ocaml-findlib-devel \
+ocaml-ocamlbuild \
+ocaml-ocamldoc \
+pandoc \
+patch \
+pkg-config \
+python \
+python-devel \
+systemd-devel \
+tar \
+transfig \
+valgrind-devel \
+wget \
+which \
+xz-devel \
+zlib-devel \
+&& \
+zypper clean -a


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

[Xen-devel] [PATCH 3/3] automation: build in openSUSE Tumbleweed

Signed-off-by: Dario Faggioli 
---
Cc: Doug Goldstein 
Cc: Wei Liu 
---
 automation/gitlab-ci/build.yaml |   20 
 1 file changed, 20 insertions(+)

diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 1e61d30c85..94877f9d01 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -420,6 +420,26 @@ opensuse-leap-gcc-debug:
   variables:
 CONTAINER: suse:opensuse-leap
 
+opensuse-tumbleweed-clang:
+  extends: .clang-x86-64-build
+  variables:
+CONTAINER: suse:opensuse-tumbleweed
+
+opensuse-tumbleweed-clang-debug:
+  extends: .clang-x86-64-build-debug
+  variables:
+CONTAINER: suse:opensuse-tumbleweed
+
+opensuse-tumbleweed-gcc:
+  extends: .gcc-x86-64-build
+  variables:
+CONTAINER: suse:opensuse-tumbleweed
+
+opensuse-tumbleweed-gcc-debug:
+  extends: .gcc-x86-64-build-debug
+  variables:
+CONTAINER: suse:opensuse-tumbleweed
+
 # Arm builds
 
 debian-unstable-gcc-arm64:


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

[Xen-devel] [PATCH 0/3] automation: build Xen in openSUSE Tumbleweed

The openSUSE distribution comes in two flavours: Leap, which is
non-rolling, and released annualy, and Tumbleweed, which is rolling.

In general, they are quite similar, but the versions of the software
they ship can be significantly different. As it is easy to imagine,
Tumbleweed, being rolling, has much more recent and updated packages.

Actually, Tumbleweed often ships the most updated releases of various
software and projects, among many distribution around.

For instance, right now, Tumbleweed has gcc 9.1.1. Fedora Rawhide also
seems to have 9.1.1, while Fedora 30 has 9.0.1. Debian Unstable and
Ubuntu Disco both have 8.3.0, AFAICT.

Of course, it's not at all like "the more updated the better", or
anything like that! It's just that I see some value in having, as a part
of our CI:
- more diversity, in terms of versions of the packages/software we try
  to build Xen with and against;
- a "bleeding edge" environment, in order to catch, or at least be
  aware of, build issues with latest compilers and/or when linking
  against the latest version of this and that library.

Interestingly, in the past couple of days, a few build issues of Xen,
qemu-xen and ipxe (at the commit that we were checking out) with gcc
9.1.1 where discovered and fixed. And --at least as far as the ones
I've reported/fixed-- I found about them while building Xen in
openSUSE Tumbleweed (while working on this patch series :-D ).

Happy to hear what people think about all this. :-)

Oh, BTW, apart from adding the Tumbleweed dockerfile and CI jobs, the
series also does some minor tweaking of the already present openSUSE
Leap dockerfile (in patch 1).

Thanks and Regards.
---
Dario Faggioli (3):
  automation: try to keep openSUSE Leap image a little smaller
  automation: add openSUSE Tumbleweed CI image
  automation: build in openSUSE Tumbleweed

 automation/build/suse/opensuse-leap.dockerfile |6 +-
 .../build/suse/opensuse-tumbleweed.dockerfile  |   68 
 automation/gitlab-ci/build.yaml|   20 ++
 3 files changed, 91 insertions(+), 3 deletions(-)
 create mode 100644 automation/build/suse/opensuse-tumbleweed.dockerfile
--
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)

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

Re: [Xen-devel] [PATCH] CODING_STYLE: clarify function argument indentation

Hi All,

On Wed, Jul 31, 2019 at 7:45 PM Andrew Cooper  wrote:
>
> On 31/07/2019 17:24, Volodymyr Babchuk wrote:
> > There are coding style rules that are widely accepted by community,
> > but newer were formalized in the document. Notable example is the
> > question on how function arguments and parameters should be indented
> > when they do not fit into one line.
> >
> > This question was raised multiple times lately, mostly because of
> > ongoing efforts to create Xen coding style formatting tool and because
> > of new community members, who are not aware of such unwritten rules.
> >
> > Actually, this rule is already implicitly defined in the document by
> > defining emacs coding style: 'c-file-style: "BSD"'. In this mode emacs
> > lines up function arguments under the first argument. Naturally, most
> > of Xen code is written in this style.
> >
> > So, lets state the obvious and fix this rule explicitly.
> >
> > Signed-off-by: Volodymyr Babchuk 
> > ---
> >  CODING_STYLE | 14 ++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/CODING_STYLE b/CODING_STYLE
> > index 6cc5b774cf..6479215a15 100644
> > --- a/CODING_STYLE
> > +++ b/CODING_STYLE
> > @@ -53,6 +53,20 @@ Line Length
> >  Lines should be less than 80 characters in length.  Long lines should
> >  be split at sensible places and the trailing portions indented.
> >
> > +For multiline function declaration and call each new line should be
> > +aligned with the first the parameter or argument. e.g.:
> > +
> > +void my_function_with_long_name(struct lengthy_struct_name *struct1,
> > +struct lengthy_struct_name *struct2,
> > +struct lengthy_struct_name *struct3);
> > +
> > +or
> > +
> > +function_with_so_many_params(wordy_parameter1, wordy_parameter2,
> > + wordy_parameter3, wordy_parameter4);
> > +
> > +The same applies for macros.
>
> For very wordy functions, or ones with silly quantities of parameters,
> the following is also acceptable
>
> void my_function_with_long_and_silly_name(
> struct lengthy_struct_name *struct1, unsigned int womble, unsigned
> int whatsit,
> struct lengthy_struct_name *struct2, bool yes, bool no, bool maybe,
> bool file_not_found, struct lengthy_struct_name *struct3, struct
> lengthy_struct_name *struct4);
>
> which you will find in a few places throughout the code, because the
> above doesn't waste enough vertical space to fit several functions in,
> and push all the relevant details to the RHS.
>
> Per the above rules, the result would be this:
>
> void my_function_with_long_and_silly_name(struct lengthy_struct_name
> *struct1,
>   unsigned int womble,
>   unsigned int whatsit,
>   struct lengthy_struct_name
> *struct2,
>   bool yes, bool no, bool maybe,
>   bool file_not_found,
>   struct lengthy_struct_name
> *struct3,
>   struct lengthy_struct_name
> *struct4);
>
> Of course, this is also a sign that maybe the function signature wants
> changing anyway, but that may not be possible/sensible at the time.
>
> As with everything, the coding style is a set of guidelines which are
> applicable to 98% of cases, but there are cases where aren't
> appropriate, and common sense is the only reasonable deciding factor.

It might be hard to automate 'common sense' cases. It seems it would
be easier to find a solution on how to avoid such 'common sense'
cases.

One more open point with this rule is how to format the next case where:
len(return type string)+len(function name)+len(any of parameter) > 79

For example:
+some_long_return_type  my_function_with_long_name(struct
lengthy_struct_name *struct1,
+struct lengthy_struct_name *struct2,
+struct lengthy_struct_name *struct3);

Thanks

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

Re: [Xen-devel] [PATCH] CODING_STYLE: clarify function argument indentation

On 31/07/2019 17:24, Volodymyr Babchuk wrote:
> There are coding style rules that are widely accepted by community,
> but newer were formalized in the document. Notable example is the
> question on how function arguments and parameters should be indented
> when they do not fit into one line.
>
> This question was raised multiple times lately, mostly because of
> ongoing efforts to create Xen coding style formatting tool and because
> of new community members, who are not aware of such unwritten rules.
>
> Actually, this rule is already implicitly defined in the document by
> defining emacs coding style: 'c-file-style: "BSD"'. In this mode emacs
> lines up function arguments under the first argument. Naturally, most
> of Xen code is written in this style.
>
> So, lets state the obvious and fix this rule explicitly.
>
> Signed-off-by: Volodymyr Babchuk 
> ---
>  CODING_STYLE | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/CODING_STYLE b/CODING_STYLE
> index 6cc5b774cf..6479215a15 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -53,6 +53,20 @@ Line Length
>  Lines should be less than 80 characters in length.  Long lines should
>  be split at sensible places and the trailing portions indented.
>  
> +For multiline function declaration and call each new line should be
> +aligned with the first the parameter or argument. e.g.:
> +
> +void my_function_with_long_name(struct lengthy_struct_name *struct1,
> +struct lengthy_struct_name *struct2,
> +struct lengthy_struct_name *struct3);
> +
> +or
> +
> +function_with_so_many_params(wordy_parameter1, wordy_parameter2,
> + wordy_parameter3, wordy_parameter4);
> +
> +The same applies for macros.

For very wordy functions, or ones with silly quantities of parameters,
the following is also acceptable

void my_function_with_long_and_silly_name(
    struct lengthy_struct_name *struct1, unsigned int womble, unsigned
int whatsit,
    struct lengthy_struct_name *struct2, bool yes, bool no, bool maybe,
    bool file_not_found, struct lengthy_struct_name *struct3, struct
lengthy_struct_name *struct4);

which you will find in a few places throughout the code, because the
above doesn't waste enough vertical space to fit several functions in,
and push all the relevant details to the RHS.

Per the above rules, the result would be this:

void my_function_with_long_and_silly_name(struct lengthy_struct_name
*struct1,
  unsigned int womble,
  unsigned int whatsit,
  struct lengthy_struct_name
*struct2,
  bool yes, bool no, bool maybe,
  bool file_not_found,
  struct lengthy_struct_name
*struct3,
  struct lengthy_struct_name
*struct4);

Of course, this is also a sign that maybe the function signature wants
changing anyway, but that may not be possible/sensible at the time.

As with everything, the coding style is a set of guidelines which are
applicable to 98% of cases, but there are cases where aren't
appropriate, and common sense is the only reasonable deciding factor.

~Andrew

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

Re: [Xen-devel] Xen Coding style and clang-format

2019-07-31 Thread Lars Kurth



> On 31 Jul 2019, at 12:43, Viktor Mitin  wrote:
> 
> On Wed, Jul 31, 2019 at 2:25 PM Julien Grall  > wrote:
>> 
>> 
>> 
>> On 31/07/2019 12:16, Viktor Mitin wrote:
>>> On Mon, Jul 29, 2019 at 3:35 PM Julien Grall  wrote:
 On 7/29/19 1:21 PM, Viktor Mitin wrote:
> On Mon, Jul 29, 2019 at 1:49 PM Julien Grall  wrote:
>> On 7/29/19 10:13 AM, Viktor Mitin wrote:
>>> On Fri, Jul 26, 2019 at 3:50 PM Julien Grall  
>>> wrote:
 
 *
 
 -/* See linux
 Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt */
 +/* See linux
 + * 
 Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt */
 
 Multi-lines comment on Xen are using
 /*
 * Foo
 * Bar
 */
>>> 
>>> See my comment about clang-format supports only comments indentation 
>>> for now.
>> 
>> I saw it and I will reply here for simplicity. Having a automatic
>> checker that will do the wrong things is not ideal.
>> 
>> Imagine we decide to use this checker as a part of the commit process.
>> This means that the code will be modified to checker coding style and
>> not Xen one.
> 
> Well, you are right. Even more, unfortunately, there is no such coding
> style as 'bsd' in clang-format.
> So 'xen' clang-format style is based on the 'llvm' style.
 
 Do you have a pointer to the LLVM style?
>>> 
>>> Sure, see the next links:
>>> https://github.com/viktor-mitin/xen-clang-format-example/blob/master/.clang-format_llvm
>>> https://github.com/viktor-mitin/xen-clang-format-example/blob/master/.clang-format_xen
>> 
>> That's clang-format configuration not a write-up easily readable by human. 
>> It is
>> also does not say what will happen for the rest of the things not configured 
>> (if
>> there are any).
> 
> Here is Clang-Format Style Options documentation:
> https://clang.llvm.org/docs/ClangFormatStyleOptions.html 
> 
> 
> And LLVM Coding Standards documetation:
> https://llvm.org/docs/CodingStandards.html#introduction 
> 
> 
> Unfortunately, it seems it does not answer "what will happen for the
> rest of the things not configured (if there are any)"...
> 
> 
>> 
>>> 
 
 As I pointed out in a different thread, it woudl be easier to start from
 an existing coding style (LLVM, BSD...) and decide whether you want to
 fully adopt it or make changes.
 
 So someone needs to be pick one and look at the difference in style with
 Xen. It seems you already done that job as you tweak it for Xen. Do you
 have a write-up of the differences?
>>> 
>>> Yes, it is done exactly this way you mentioned. New 'xen' format style
>>> is based on 'llvm'.
>> 
>> Can you give a link to this write-up in a human readable way?
> 
> The summary I wrote in the original mail in this thread describes what
> was done based on 'llvm' coding style of clang-format.
> I'm putting it here with update: added BreakStringLiterals thing to be fixed.
> 
> Summary of the changes:
> - Added 3 new formatting styles to cover all the cases mentioned in
> Xen coding style document: Xen, Libxl, Linux;
> - Added list of the files and corresponding style name mappings;
> - Added indentation according to Xen coding style;
> - Added white space formatting according to Xen coding style;
> - Added bracing support exception for do/while loops;
> 
> Added to clang-format, however, probably this logic should be moved to
> python part (see known clang-format limitations above):
> - Braces should be omitted for blocks with a single statement. Note:
> these braces will be required by MISRA, for example, so it is probably
> worth adding such a requirement to the coding style.
> - Comments format requirements. Note: //-style comments are defined in
> C99 as well, and not just in the case of C++. C99 standard is 20-years
> old…
> 
> To be added:
> - Emacs local variables. Open points: Why to keep emacs local
> variables in Xen code? What about other editors' comments (vim)?
> - Warning to stderr in the case when ‘unfixable’ line/s detected.
> 
> To be fixed:
> - Max line length from 80 to 79 chars;
> - Disable // comments;
> - Set BreakStringLiterals to false (not explicitly covered by Xen
> coding style document for now)
> 
>> 
>>> 
 
 I am not sure why clang-format decided to format like that. Do you 
 know why?
>>> 
>>> The reason is that there are two strings in one line. It would not
>>> change it if it were
>>> not "arm,psci-1.0""\0", but "arm,psci-1.0\0".
>> 
>> I would like to see the exact part of the clang-format coding style
>> documentation that mention this requirements... The more that in an
>> example above (copied below for simplicity), there are two

[Xen-devel] [PATCH] CODING_STYLE: clarify function argument indentation

There are coding style rules that are widely accepted by community,
but newer were formalized in the document. Notable example is the
question on how function arguments and parameters should be indented
when they do not fit into one line.

This question was raised multiple times lately, mostly because of
ongoing efforts to create Xen coding style formatting tool and because
of new community members, who are not aware of such unwritten rules.

Actually, this rule is already implicitly defined in the document by
defining emacs coding style: 'c-file-style: "BSD"'. In this mode emacs
lines up function arguments under the first argument. Naturally, most
of Xen code is written in this style.

So, lets state the obvious and fix this rule explicitly.

Signed-off-by: Volodymyr Babchuk 
---
 CODING_STYLE | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/CODING_STYLE b/CODING_STYLE
index 6cc5b774cf..6479215a15 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -53,6 +53,20 @@ Line Length
 Lines should be less than 80 characters in length.  Long lines should
 be split at sensible places and the trailing portions indented.
 
+For multiline function declaration and call each new line should be
+aligned with the first the parameter or argument. e.g.:
+
+void my_function_with_long_name(struct lengthy_struct_name *struct1,
+struct lengthy_struct_name *struct2,
+struct lengthy_struct_name *struct3);
+
+or
+
+function_with_so_many_params(wordy_parameter1, wordy_parameter2,
+ wordy_parameter3, wordy_parameter4);
+
+The same applies for macros.
+
 User visible strings (e.g., printk() messages) should not be split so
 they can searched for more easily.
 
-- 
2.22.0

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

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

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf b3d00df69c78fa0e12986a7ff334689a76f4578a
baseline version:
 ovmf 3d34b5f32692c84bbc69ff34a9ea511bcb55e50a

Last test of basis   139533  2019-07-30 17:09:39 Z0 days
Testing same since   139550  2019-07-31 04:13:54 Z0 days1 attempts


People who touched revisions under test:
  Zhichao Gao 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   3d34b5f326..b3d00df69c  b3d00df69c78fa0e12986a7ff334689a76f4578a -> 
xen-tested-master

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

Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

Hi Jan,

On Mon, Jul 29, 2019 at 4:21 PM Jan Beulich  wrote:

>   - Line 67: I believe Jan request the space before label
> >>> Seems agreed not to add the spaces before label. Right?
> >>
> >> Certainly not, afaia. I will object to any written down rule disallowing
> >> leading blank(s) altogether. I will argue for making mandatory at least
> >> one blank of indentation.
> >
> > Coding style are a matter of taste. If everyone is going to say "I want
> > this in the coding style", then we are going to spend countless of hours
> > bike-shedding. This is not how we should use our already limited time.
>
> I agree with what you say in general, but not in this specific case: I've
> explained why the leading blank(s) are wanted here. This is not because of
> my taste, but because of helping with patch review.

I've checked all the styles supported by clang-format at the moment:
LLVM, Google, Chromium, Mozilla, WebKit, Microsoft. None of them uses
spaces before labels. It seems Linux coding style does not use it as
well. Please see the questions below:

How all those projects live without this issue?
What is the reason not to fix 'diff -p'? Is it not possible at all?
Could you please share more details about the background of the issue
and examples?

Thanks

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

[Xen-devel] [libvirt test] 139551: tolerable all pass - PUSHED

flight 139551 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139551/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 139516
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 139516
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  b0ecc0a04cfcfc706e252d3960f7f10db45c9186
baseline version:
 libvirt  fed58d83c60ff1c20292856bec006577788b7494

Last test of basis   139516  2019-07-30 04:18:48 Z1 days
Testing same since   139551  2019-07-31 04:18:54 Z0 days1 attempts


People who touched revisions under test:
  Daniel P. Berrangé 
  Eric Blake 
  Jiri Denemark 
  Ján Tomko 
  Vitaly Kuznetsov 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-arm64-arm64-libvirt-qcow2   pass
 test-armhf-armhf-libvirt-raw pass
 test-amd64-amd64-libvirt-vhd 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 :

To xenbits.xen.org:/home/xen/git/libvirt.git
   fed58d83c6..b0ecc0a04c  b0ecc0a04cfcfc706e252d3960f7f10db45c9186 -> 
xen-tested-master

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

[Xen-devel] [linux-4.19 test] 139544: regressions - FAIL

flight 139544 linux-4.19 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/139544/

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-examine  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit1   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 linux64f4694072aa4ac23eb9ad2feeb0a178d2a054da
baseline version:
 linux84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d

Last test of basis   129313  2018-11-02 05:39:08 Z  271 days
Failing since129412  2018-11-04 14:10:15 Z  269 days  180 attempts
Testing same since   139458  2019-07-28 22:09:13 Z2 days4 attempts


2364 people touched revisions under test,
not listing them all

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

Re: [Xen-devel] [RFC] Generating Go bindings for libxl

2019-07-31 Thread George Dunlap

On 7/30/19 7:39 PM, Tamas K Lengyel wrote:
> On Tue, Jul 30, 2019 at 9:49 AM George Dunlap  
> wrote:
>>
>> On 7/30/19 2:48 PM, Tamas K Lengyel wrote:
>>> On Tue, Jul 30, 2019 at 7:32 AM Nicholas Rosbrook
>>>  wrote:

 Hello,

 As a follow up to the presentation that Brendan Kerrigan and I gave at Xen
 summit earlier this month, "Client Virtualization Toolstack in Go", I 
 would like to open
 a discussion around the development of Go bindings for libxl. George 
 Dunlap,
 Nicolas Belouin and I have had some discussion off-line already.

 So far, these are the topics of discussion:
>>>
>>> Hi Nicholas,
>>> to add to the list of topics I just want to mention that perhaps it
>>> may be beneficial to consider parts of the go bindings not go to libxl
>>> at all. I have been digging through libxl for the past couple months
>>> and it's asynchronous callback system is damn near impossible to
>>> follow and I just can't shake the feeling that it would be a lot
>>> easier to follow if it was in go.
>>
>> So I don't think we're ever going to switch to golang being our primary
>> toolstack language, because calling it from other languages isn't really
>> an option.  That means that implementing things like domain creation in
>> Go mean duplicating functionality in two places, which is
>> extraordinarily expensive from a software-engineering perspective.
>>
>> FWIW I think the asynchronous callback system just needs better
>> documentation.  It always takes me a little bit to get my bearings again
>> once I have to change that code, but once I do, everything is
>> consistent.  And as I understand it, the external interface was written
>> primarily with libvirt in mind, so it would probably be difficult to
>> change it while remaining compatible.
>>
>>> Not to mention the performance
>>> issues with the built-in garbage collector
>>
>> What performance issues were you seeing with libxl's garbage collector?
>> I thought it just kept a list of pointers and freed them at the very end.
> 
> I didn't investigate too closely but on some occasions a considerable
> amount of the execution time was being spent in there according to
> callgrind. After everything was finished in a domain creation xl would
> just "hang" in there for a while before actually exiting. It was not
> very consistent and recompiling libxl sometimes sped things up.
> Haven't run into it since I've upgraded to debian buster and a newer
> gcc.

Is it possible this was actually doing the "async" parts of long-running
operations?  When no async callback is provided, the very last thing
that happens before a return is to  sleep and wait for the next thing to
happen, then call the next thing in the async chain.

 -George

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

[Xen-devel] Fedora 30 DomU - pygrub always boots the second menu option

2019-07-31 Thread Steven Haigh

There's a ton of changes to grub in Fedora 30 Most of them causing 
pain.


When booting using pygrub, the presented menu always has the second 
option selected.


The contents of /etc/default/grub is as follows:
GRUB_TIMEOUT=1
GRUB_DEFAULT=0
GRUB_DISABLE_SUBMENU=true
GRUB_TERMINAL_OUTPUT="console"
GRUB_CMDLINE_LINUX="audit=0 selinux=0 console=hvc0"
GRUB_DISABLE_RECOVERY="true"
GRUB_ENABLE_BLSCFG=false

I have attached the generated grub.cfg created via:
grub2-mkconfig -o /boot/grub/grub.cfg

BLSCFG is a whole new clusterf**k of problems that became default in 
Fedora 30 that cause many problems - but first things first...

Steven Haigh

 net...@crc.id.au  https://www.crc.id.au
 +613 9001 6090    +614 1293 5897


#
# DO NOT EDIT THIS FILE
#
# It is automatically generated by grub2-mkconfig using templates
# from /etc/grub.d and settings from /etc/default/grub
#

### BEGIN /etc/grub.d/00_header ###
set pager=1

if [ -f ${config_directory}/grubenv ]; then
  load_env -f ${config_directory}/grubenv
elif [ -s $prefix/grubenv ]; then
  load_env
fi
if [ "${next_entry}" ] ; then
   set default="${next_entry}"
   set next_entry=
   save_env next_entry
   set boot_once=true
else
   set default="0"
fi

if [ x"${feature_menuentry_id}" = xy ]; then
  menuentry_id_option="--id"
else
  menuentry_id_option=""
fi

export menuentry_id_option

if [ "${prev_saved_entry}" ]; then
  set saved_entry="${prev_saved_entry}"
  save_env saved_entry
  set prev_saved_entry=
  save_env prev_saved_entry
  set boot_once=true
fi

function savedefault {
  if [ -z "${boot_once}" ]; then
saved_entry="${chosen}"
save_env saved_entry
  fi
}

function load_video {
  if [ x$feature_all_video_module = xy ]; then
insmod all_video
  else
insmod efi_gop
insmod efi_uga
insmod ieee1275_fb
insmod vbe
insmod vga
insmod video_bochs
insmod video_cirrus
  fi
}

terminal_output console
if [ x$feature_timeout_style = xy ] ; then
  set timeout_style=menu
  set timeout=1
# Fallback normal timeout code in case the timeout_style feature is
# unavailable.
else
  set timeout=1
fi
### END /etc/grub.d/00_header ###

### BEGIN /etc/grub.d/01_users ###
if [ -f ${prefix}/user.cfg ]; then
  source ${prefix}/user.cfg
  if [ -n "${GRUB2_PASSWORD}" ]; then
set superusers="root"
export superusers
password_pbkdf2 root ${GRUB2_PASSWORD}
  fi
fi
### END /etc/grub.d/01_users ###

### BEGIN /etc/grub.d/08_fallback_counting ###
insmod increment
# Check if boot_counter exists and boot_success=0 to activate this behaviour.
if [ -n "${boot_counter}" -a "${boot_success}" = "0" ]; then
  # if countdown has ended, choose to boot rollback deployment,
  # i.e. default=1 on OSTree-based systems.
  if  [ "${boot_counter}" = "0" -o "${boot_counter}" = "-1" ]; then
set default=1
set boot_counter=-1
  # otherwise decrement boot_counter
  else
decrement boot_counter
  fi
  save_env boot_counter
fi
### END /etc/grub.d/08_fallback_counting ###

### BEGIN /etc/grub.d/10_linux ###
menuentry 'Fedora (5.1.20-300.fc30.x86_64) 30 (Thirty)' --class fedora --class 
gnu-linux --class gnu --class os --unrestricted $menuentry_id_option 
'gnulinux-5.1.20-300.fc30.x86_64-advanced-e2f94071-1c3b-4b45-b6fb-22e3f952d4ae' 
{
load_video
set gfxpayload=keep
insmod gzio
insmod part_msdos
insmod ext2
if [ x$feature_platform_search_hint = xy ]; then
  search --no-floppy --fs-uuid --set=root  
e2f94071-1c3b-4b45-b6fb-22e3f952d4ae
else
  search --no-floppy --fs-uuid --set=root 
e2f94071-1c3b-4b45-b6fb-22e3f952d4ae
fi
linux   /boot/vmlinuz-5.1.20-300.fc30.x86_64 
root=UUID=e2f94071-1c3b-4b45-b6fb-22e3f952d4ae ro audit=0 selinux=0 
console=hvc0 
initrd  /boot/initramfs-5.1.20-300.fc30.x86_64.img
}
menuentry 'Fedora (5.1.18-300.fc30.x86_64) 30 (Thirty)' --class fedora --class 
gnu-linux --class gnu --class os --unrestricted $menuentry_id_option 
'gnulinux-5.1.18-300.fc30.x86_64-advanced-e2f94071-1c3b-4b45-b6fb-22e3f952d4ae' 
{
load_video
set gfxpayload=keep
insmod gzio
insmod part_msdos
insmod ext2
if [ x$feature_platform_search_hint = xy ]; then
  search --no-floppy --fs-uuid --set=root  
e2f94071-1c3b-4b45-b6fb-22e3f952d4ae
else
  search --no-floppy --fs-uuid --set=root 
e2f94071-1c3b-4b45-b6fb-22e3f952d4ae
fi
linux   /boot/vmlinuz-5.1.18-300.fc30.x86_64 
root=UUID=e2f94071-1c3b-4b45-b6fb-22e3f952d4ae ro audit=0 selinux=0 
console=hvc0 
initrd  /boot/initramfs-5.1.18-300.fc30.x86_64.img
}
menuentry 'Fedora (0-rescue-46e72612de204d5d8d6a9fe68e255ba3) 30 (Thirty)' 
--class fedora --class gnu-linux --class gnu --class os --unrestricted 
$menuentry_id_option 
'gnulinux-0-rescue-46e72612de204d5d8d6a9fe68e255ba3-advanced-e2f94071-1c3b-4b45-b6fb-22e3f952d4ae'
 {
load_video
insmod gzio
insmod part_msdos
insmod ext2

Re: [Xen-devel] [RFC] Generating Go bindings for libxl

2019-07-31 Thread George Dunlap

On 7/30/19 10:52 PM, Nicholas Rosbrook wrote:
>> All that said, the first question I think is, what the generated code
>> needs to look like.  Then, if c-for-go can be configured to do that,
>> then we can consider it; otherwise, making our own generator from the
>> IDL will be the only option.
> 
> Writing a custom Go code generator means that all C symbols used need
> to be known at generation time. E.g., the Go code generator needs to know
> the signature of libxl_domain_create_new for C.libxl_domain_create_new(...)
> to work. Currently, such knowledge is gained by parsing C code, which makes
> sense given the nature of cgo.

I return to the question I stated before.  At the moment, your bindings
have the following call chain:

* DomainInfo(), hand-crafted.  Calls domainInfo().
* domainInfo(), automaticall generated.  Calls C.libxl_domain_info().

The in-tree bindings have the following call chain:

* DomainInfo(), hand-crafted.  Calls C.libxl_domain_info().

Since DomainInfo() is hand-crafted in both cases, what's the advantage
of having domainInfo() at all?

> AFAICT, the IDL describes how to generate C types
> and boiler-plate functions like libxl__dispose(). How would the IDL 
> alone be able to 
> generate valid Go code without significant expansion?

So just to clarify terminology: The IDL is the description language
itself, which at the moment only contains information about the libxl
structures.  We have generators for various C bits of libxl which read
the IDL and spit out boilerplate C.  The idea would be that we write a
new generator for Go which reads the IDL and spits out boilerplate Go.

If you look at gentypes.py, you'll see that it's not doing anything
fancy; it's very much just spitting out strings based on simple rules.
I think a large amount of the boilerplate Go code (the C <-> Go
marshalling code in particular) can be done in the same way.

>> Out of curiosity, have you looked at the existing in-tree bindings?  Any
>> particular opinions?
> 
> Yes, my process started by using the existing bindings.
> 
> One thing is that they do not conform to basic go standards. For
> example: naming conventions, naked returns are used everywhere, and I find
> it strange that there is an exported Context variable. But, obviously those 
> are
> very minor things and easy to change. See [1] for general information on this.

I looked at the thing about naked returns, and didn't really understand
it; but anyway I'm happy to have things modified to be more Go-like.  I
definitely "speak" Go with a funny accent.

The exported Ctx variable is probably vestigial; I don't think I'd argue
against removing it.

Can I say -- I've been going open-source for so long, that I feel almost
unsafe when nobody reviews my stuff.  Most of this code was written by
me and reviewed by nobody (since I was the only person interested); it's
good to have someone else take a critical look at it.

> I also thought it looked very tedious to do by hand, and would be hard to 
> extend
> in the future. Hence the search for a cgo generator.

Right; and the intention was always to begin to do it by hand to see how
we wanted it to look, and then write a generator to do the tedious work
by reading the IDL.

>> There are two major differences I note.
>>
>> First, is that in your version, there seems to be two layers: libxl.go
>> is generated by c-for-go, and contains simple function calls; e.g.:
>> domainInfo(), which takes a *Ctx as an argument and calls
>> C.libxl_domain_info.  Then you have libxl_wrappers.go, which is written
>> manually, defining DomainInfo as a  method on Ctx, and calls domainInfo().
>>
>> So you're writing the "idiomatic Go" part by hand anyway; I don't really
>> see why having a hand-written Go function call an automatically
>> generated Go function to call a C function is better than having a
>> hand-written Go function call a C function directly.
> 
> I'm sure you would agree that writing all of that cgo code by hand was a PITA.

Writing the .toC() and .toGo() methods is certainly a pain, and wants a
generator.  I didn't find writing:

func (Ctx *Context) DomainInfo(Id Domid) (di *Dominfo, err error) {
err = Ctx.CheckOpen()
if err != nil {
return
}

var cdi C.libxl_dominfo
C.libxl_dominfo_init()
defer C.libxl_dominfo_dispose()

ret := C.libxl_domain_info(Ctx.ctx, , C.uint32_t(Id))

if ret != 0 {
err = Error(-ret)
return
}

di = cdi.toGo()

return
}

to be terribly much more work than writing something like:

func (c *Context) DomainInfo(domid DomID) (*DomInfo, error) {
di := DomInfo{}

if ret := domainInfo(c.Ctx, , uint32(domid)); ret != 0 {
return nil, fmt.Errorf("unable to retrieve domain info: %v", 
ret)
}
di.Deref()

return , nil
}

If we "more discipline to have defer'd dispose/free calls", the code
will probably look

[Xen-devel] Fedora 30 and BLSCFG changes equals non-booting DomUs.

2019-07-31 Thread Steven Haigh

Fedora 30 implemented Boot Loader Specification (BLS) by default for 
all newly installed, and any upgraded systems.


This causes hell booting a DomU that is *not* configured as HVM - thus 
fails when not using the bootloader from within the guest.


pygrub will always fail to boot these VMs.

Links:

Fedora change page:
https://fedoraproject.org/wiki/Changes/BootLoaderSpecByDefault

Main Fedora BZ with lots of issues:
https://bugzilla.redhat.com/show_bug.cgi?id=1652806

My bug report on new kernels not appearing in generated grub.cfg files:
https://bugzilla.redhat.com/show_bug.cgi?id=1703700

So far, the only workaround is to install the 'grubby-depreciated' 
package, set 'GRUB_ENABLE_BLSCFG=false' in /etc/default/grub, then 
manually re-create the grub.cfg file via: grub2-mkconfig -o 
/boot/grub/grub.cfg


Upon a newer kernel being installed, it may or may not appear in the 
grub.cfg configuration - even with the above changes. As such, numerous 
kernel upgrades later and your installed VM might not boot at all.


In numerous systems, I run grub2-mkconfig in /etc/rc.d/rc.local to 
avoid a completely broken VM. Not ideal.


So, to start the discussion, with none of this currently being sent 
upstream, this is a Fedora-ism. How to handle BLS enabled guests?


It also seems to be a Fedora problem with respect to kernel updates 
still causing problems - but that's another issue.


Steven Haigh

 net...@crc.id.au  https://www.crc.id.au
 +613 9001 6090    +614 1293 5897




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

Re: [Xen-devel] [PATCH v2 0/2] Raspberry Pi 4 support

2019-07-31 Thread Stewart Hildebrand

On Wednesday, July 31, 2019 8:32 AM, Andre Przywara  
wrote:
>On Mon, 29 Jul 2019 09:19:18 -0400
>Stewart Hildebrand  wrote:
>
>Hi,
>
>> This is a series to enable UART console for Raspberry Pi 4. Note that I'm 
>> relying on the firmware to initialize the UART (i.e. enable_uart=1 in 
>> config.txt), since full UART initialization on this platform requires 
>> accessing some registers outside the range specified in the 
>> brcm,bcm2835-aux-uart node.
>>
>> I have been able to get Xen+dom0+domUs booting. Tested with Xen 4.12 and 
>> 4.13-unstable (b4c8a27d5b)
>
>Mmmh, did that really work for you? I needed the next commit in staging as 
>well:
>commit ead6b9f78355e8d366e0c80c4a73fa7fbd6d26cc
>Author: Andrii Anisov 
>Date:   Thu Jul 18 16:22:20 2019 +0300
>
>xen/arm: cpuerrata: Align a virtual address before unmap
>
>Otherwise Xen would crash before even considering Dom0.

Hmm... Yes. I did not have Andrii's patch included in my tests. I tested in 
4.13-unstable (b4c8a27d5b) and on 4.12 (604ee1116d3e), it booted into Xen+dom0. 
Now that I'm thinking about it, the domU test may have only been on 4.12, not 
on 4.13-unstable.

>With Andrii's patch, your two patches and Stefano's resmem series I was able 
>to at least run Xen till it was looking for Dom0, from U-
>Boot, with ATF (providing PSCI).

I'm not using ATF/U-Boot, and I've limited system RAM to total_mem=1024 in 
config.txt (even though I have the 4GB model) due to some DMA/driver issues (SD 
card, PCIe, etc) with the 64-bit RPi kernel. I also did not have Stefano's 
reserved-memory series applied in any test.

The following logs are a from couple of test runs from 4.13-unstable on Sun Jul 
28:

(XEN) RAM:  - 3b3f
(XEN) 
(XEN) MODULE[0]: 2eff5f00 - 2e49 Device Tree 
(XEN) MODULE[1]: 0048 - 0140 Kernel  
(XEN)  RESVD[0]:  - 1000
...
(XEN) Allocating 1:1 mappings totalling 512MB for dom0:
(XEN) BANK[0] 0x001000-0x002800 (384MB)
(XEN) BANK[1] 0x003000-0x003800 (128MB)

Curiously, in another test with total_mem=1536, Xen happened to allocate memory 
for dom0 in a different way. I am noticing that the RPi firmware carved out a 
hole in RAM for me in this case.

(XEN) RAM:  - 3b3f
(XEN) RAM: 4000 - 5fff
(XEN) 
(XEN) MODULE[0]: 2eff5f00 - 2e55 Device Tree 
(XEN) MODULE[1]: 0048 - 0140 Kernel  
(XEN)  RESVD[0]:  - 1000
...
(XEN) Allocating 1:1 mappings totalling 512MB for dom0:
(XEN) BANK[0] 0x001000-0x002000 (256MB)
(XEN) BANK[1] 0x004000-0x005000 (256MB)

Stew

>There seems to be some hiccup in the reserved-memory code in U-Boot, where 
>U-Boot tries to use the already region, but that's an
>independent matter.
>
>Cheers,
>Andre.
>
>> and Linux 4.19.y (Raspberry Pi linux tree + a couple of patches). Please see 
>> [1] for build instructions and limitations.
>>
>> New in v2:
>> * Drop early printk alias
>> * Set reg-shift and reg-io-width in the Xen driver
>> * Blacklist other aux peripherals in platform settings (spi1, spi2, and a 
>> couple of base aux registers)
>>
>> Thanks,
>> Stewart Hildebrand
>> DornerWorks, Ltd
>>
>> [1] https://github.com/dornerworks/xen-rpi4-builder
>>
>> Stewart Hildebrand (2):
>>   ns16550: Add compatible string for Raspberry Pi 4
>>   xen/arm: platform: Add Raspberry Pi platform
>>
>>  xen/arch/arm/platforms/Makefile|  1 +
>>  xen/arch/arm/platforms/brcm-raspberry-pi.c | 55 ++
>>  xen/drivers/char/ns16550.c |  7 +++
>>  3 files changed, 63 insertions(+)
>>  create mode 100644 xen/arch/arm/platforms/brcm-raspberry-pi.c
>>


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

Re: [Xen-devel] [PATCH 5/7] xen/arm: traps: Avoid BUG_ON() in do_trap_brk()

On 30/07/2019 18:00, Stefano Stabellini wrote:
> On Tue, 30 Jul 2019, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 7/29/19 11:02 PM, Stefano Stabellini wrote:
>>> On Tue, 23 Jul 2019, Julien Grall wrote:
 At the moment, do_trap_brk() is using a BUG_ON() to check the hardware
 has been correctly configured during boot.

 Any error when configuring the hardware could result to a guest 'brk'
 trapping in the hypervisor and crash it.

 This is pretty harsh to kill Xen when actually killing the guest would
 be enough as misconfiguring this trap would not lead to exposing
 sensitive data. Replace the BUG_ON() with crashing the guest.

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

 diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
 index 132686ee0f..ef37ca6bde 100644
 --- a/xen/arch/arm/traps.c
 +++ b/xen/arch/arm/traps.c
 @@ -1304,10 +1304,15 @@ int do_bug_frame(const struct cpu_user_regs *regs,
 vaddr_t pc)
   #ifdef CONFIG_ARM_64
   static void do_trap_brk(struct cpu_user_regs *regs, const union hsr hsr)
   {
 -/* HCR_EL2.TGE and MDCR_EL2.TDE are not set so we never receive
 - * software breakpoint exception for EL1 and EL0 here.
 +/*
 + * HCR_EL2.TGE and MDCR_EL2.TDR are currently not set. So we should
 + * never receive software breakpoing exception for EL1 and EL0 here.
*/
 -BUG_ON(!hyp_mode(regs));
 +if ( !hyp_mode(regs) )
 +{
 +domain_crash(current->domain);
 +return;
 +}
>>> This is a good change to have. I am wondering if it would make sense to
>>> also printk some debug messages, maybe even show_execution_state. If so,
>>> we need to be careful that it's only done in debug builds, or it is rate
>>> limited. What do you think?
>> Messages are already printed by domain_crash(...). But I don't see the 
>> concern
>> about non-debug build or even not rate limiting... If the domain crash, then
>> it will not cause anymore print and therefore you can't spam the console 
>> here.
> Ah yes, you are quite right

I still have a series pending to force people to put a useful printk()
in, because there is nothing more infuriating than to find a
domain_crash() with no clarifying context.

It should be a gprintk(XENLOG_ERR, and will eventually be folded into
domain_crash()'s prototype.

~Andrew

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

Re: [Xen-devel] [PATCH v2 2/2] xen/arm: platform: Add Raspberry Pi platform


Hi,

On 31/07/2019 14:55, Stewart Hildebrand wrote:

On Wednesday, July 31, 2019 8:04 AM, Julien Grall  wrote:

Hi Stewart,


Hi Julien


On 29/07/2019 14:19, Stewart Hildebrand wrote:

The aux peripherals (uart1, spi1, and spi2) share an IRQ and a page of
memory. For debugging, it is helpful to use the aux UART in Xen. In
this case, Xen would try to assign spi1 and spi2 to dom0, but this
results in an error since the shared IRQ was already assigned to Xen.
Blacklist aux devices other than the UART to prevent mapping the shared
IRQ and memory range to dom0.


Reading the commit message, it is unclear what's the impact on blacklist spi1
and spi2. Could you expand it?


Yes, good thinking. What do you think about the following (the first paragraph 
is unchanged, just copied for completeness):

"The aux peripherals (uart1, spi1, and spi2) share an IRQ and a page of
memory. For debugging, it is helpful to use the aux UART in Xen. In
this case, Xen would try to assign spi1 and spi2 to dom0, but this
results in an error since the shared IRQ was already assigned to Xen.
Blacklist aux devices other than the UART to prevent mapping the shared
IRQ and memory range to dom0.

Blacklisting spi1 and spi2 unfortunately makes those peripherals
unavailable for use in the system. Future work could include forwarding
the IRQ for spi1 and spi2, and trap and mediate access to the memory
range for spi1 and spi2."


Ok, so they are not critical to boot Dom0. Good :).

Hopefully they will learn the lesson for the next generation!



Would you like me to re-send the patch, or can the message be updated on commit?


No, I will update the commit message and commit it later on today.

With the new commit message:

Acked-by: Julien Grall 

Cheers,

--
Julien Grall

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

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

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  78cc87ce498bf621914c0554b83fac3ee00d
baseline version:
 xen  1585ed3c702e680ae492d852c8cff62cf300df99

Last test of basis   139529  2019-07-30 14:01:43 Z1 days
Testing same since   139561  2019-07-31 11:00:34 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 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 :

To xenbits.xen.org:/home/xen/git/xen.git
   1585ed3c70..78cc87  78cc87ce498bf621914c0554b83fac3ee00d -> smoke

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

Re: [Xen-devel] [PATCH v4 2/2] xen/arm: merge make_timer_node and make_timer_domU_node


Hi Volodymyr,

On 31/07/2019 14:41, Volodymyr Babchuk wrote:

Viktor Mitin writes:

On Wed, Jul 31, 2019 at 3:33 PM Volodymyr Babchuk
 wrote:

So, previously this code copied "compatible" property from platform
device tree. Please note, that theoretically it would be neither
"arm,armv8-timer" not "arm,armv7-timer". Now you are setting one of the
two values. I'm not sure if this is right thing to do in the first
place. Probably we need comment from Julien. But this change should be
reflected in the commit message.


Well, it is done, because Julien preferred domU variant as more simple one.
Actually I have checked that both variats works well, but kept domU case.

My concern is that you are changing function behavior in
non-backward compatible way. Yes, it is working on your platform. But
what about others?


There are only official three compatible existing for the arch timer:
   - arm,armv7-timer
   - arm,armv8-timer
   - arm,cortex-a15-timer

The latter should always have also arm,armv7-timer. Any OS running on Xen should 
not assume that a specific property should be there. If it is not the case, then 
fix your OS.


I am also discarding any other property compatible as they are probably 
out-of-tree and therefore not supported.


For 64-bit domain, you can only ever run on Armv8 platform so there are no 
change here.


For 32-bit domain, you can run on either Armv8 and Armv7 platform. It is a grey 
area where you should pass a different property depending on the platform you 
are. Libxl is always passing armv7 so I would prefer to keep like that.


The main difference with this patch will be for 32-bit dom0. They will always 
see Armv7 compatible even when running on Armv8 platform.


Xen 32-bit on Armv8 platform is not supported. Running 32-bit dom0 on Xen 64-bit 
is very unlikely.


So I don't have any major concerns with this change. This has the advantage to 
uniformize the way arch timer is exposed to all the guests.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v4 2/2] xen/arm: merge make_timer_node and make_timer_domU_node


Hi Viktor,

I am going to exceptionally top-post.

There are rules that are widely accepted for the coding style yet they are not 
written in CODING_STYLE. Rather than keeping reminding us how everything is 
unwritten, it would be more beneficial if you try to help us making better.


Meanwhile, I see limited value to waste both your time arguing on it. Volodymyr 
is an experienced contributor of Xen Project and I trust him to point out what 
is widely accepted.


Cheers,

On 31/07/2019 14:41, Volodymyr Babchuk wrote:


Viktor Mitin writes:


On Wed, Jul 31, 2019 at 3:33 PM Volodymyr Babchuk
 wrote:




Viktor Mitin writes:


Merged make_timer_node and make_timer_domU_node into one function
make_timer_node.

It is widely accepted to write commit messages in imperative mood,
e.g. "merge" instead of "merged"


Kept the domU version for the compatible as it is simpler.
Kept the hw version for the clock as it is relevant for the both cases.

... or "keep" instead of "kept"


Well, again, there is no such rule in the coding style document.

Yeah, but this is widely accepted style. It is good to have all commit
messages in the same style, isn't?


Suggested-by: Julien Grall 
Signed-off-by: Viktor Mitin 
---
v4 updates:
updated "Kept the domU version for the compatible as it is simpler"

  xen/arch/arm/domain_build.c | 109 +---
  1 file changed, 39 insertions(+), 70 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d04a1c3aec..4d7c3411a6 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -964,8 +964,12 @@ static int __init make_gic_node(const struct domain *d, 
void *fdt,

  static int __init make_timer_node(const struct kernel_info *kinfo)
  {
+int res;
  void *fdt = kinfo->fdt;
-

In the previous patch you added this empty string, now you are deleting
it.


Why not? Do not remember why did it, probably it was more convenient
at that moment.
Anyway, why not?

Because patches should not include unnecessary changes. You are spending
reviewer's mental resources by introducing unneeded changes and then
undoing them in the next patch.




+unsigned int irq[MAX_TIMER_PPI];

MAX_TIMER_PPI equals to 4, but looks like you are using only first 3
items of the array.


Yes. This is because MAX_TIMER_PPI has been defined, and this
particular example is taken from time.c

Yes, but code in time.c uses all four IRQs. In your case you just wasting
extra space on stack.




+gic_interrupt_t intrs[3];
+u32 clock_frequency;
+bool clock_valid;

Do you really need to move those declarations?


Not really, it has appeared as a result of many code edit iterations.
As I mentioned previously, those patches are changed several times already,
so the final version has another order of the local variables. Why not?

Because patches should do only necessary things. If you for some reason
want to tidy up variable declaration, please do this in the separate patch.


  static const struct dt_device_match timer_ids[] __initconst =
  {
  DT_MATCH_COMPATIBLE("arm,armv7-timer"),
@@ -973,15 +977,6 @@ static int __init make_timer_node(const struct kernel_info 
*kinfo)
  { /* sentinel */ },
  };
  struct dt_device_node *dev;
-u32 len;
-const void *compatible;
-int res;
-unsigned int irq;
-gic_interrupt_t intrs[3];
-u32 clock_frequency;
-bool clock_valid;
-
-dt_dprintk("Create timer node\n");

  dev = dt_find_matching_node(NULL, timer_ids);
  if ( !dev )
@@ -990,35 +985,49 @@ static int __init make_timer_node(const struct 
kernel_info *kinfo)
  return -FDT_ERR_XEN(ENOENT);
  }

-compatible = dt_get_property(dev, "compatible", );
-if ( !compatible )
-{
-dprintk(XENLOG_ERR, "Can't find compatible property for timer node\n");
-return -FDT_ERR_XEN(ENOENT);
-}
-
  res = fdt_begin_node(fdt, "timer");
  if ( res )
  return res;

-res = fdt_property(fdt, "compatible", compatible, len);
-if ( res )
-return res;
+if ( !is_64bit_domain(kinfo->d) )
+{
+res = fdt_property_string(fdt, "compatible", "arm,armv7-timer");
+if ( res )
+return res;
+}
+else
+{
+res = fdt_property_string(fdt, "compatible", "arm,armv8-timer");
+if ( res )
+return res;
+}

So, previously this code copied "compatible" property from platform
device tree. Please note, that theoretically it would be neither
"arm,armv8-timer" not "arm,armv7-timer". Now you are setting one of the
two values. I'm not sure if this is right thing to do in the first
place. Probably we need comment from Julien. But this change should be
reflected in the commit message.


Well, it is done, because Julien preferred domU variant as more simple one.
Actually I have checked that both variats works well, but kept domU case.

My concern is that you are changing function

Re: [Xen-devel] [PATCH v4 2/2] xen/arm: merge make_timer_node and make_timer_domU_node


Hi,

NIT: s/merge/consolidate/

On 31/07/2019 11:28, Viktor Mitin wrote:

Merged make_timer_node and make_timer_domU_node into one function
make_timer_node.

Kept the domU version for the compatible as it is simpler.
Kept the hw version for the clock as it is relevant for the both cases.


The commit message needs a bit of rewording:
 - It is not clear why they the two functions are merged
 - This needs more word around so the commit message looks like a coherent text.



Suggested-by: Julien Grall 
Signed-off-by: Viktor Mitin 
---
v4 updates:
updated "Kept the domU version for the compatible as it is simpler"

  xen/arch/arm/domain_build.c | 109 +---
  1 file changed, 39 insertions(+), 70 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d04a1c3aec..4d7c3411a6 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -964,8 +964,12 @@ static int __init make_gic_node(const struct domain *d, 
void *fdt,
  
  static int __init make_timer_node(const struct kernel_info *kinfo)

  {
+int res;
  void *fdt = kinfo->fdt;
-


Please avoid to add code that you drop in a patch later.


+unsigned int irq[MAX_TIMER_PPI];
+gic_interrupt_t intrs[3];
+u32 clock_frequency;
+bool clock_valid;


This is not related to this patch and only increase the complexity of the 
review. If you want to do reshuffling then it should be a separate patch.


But then, I see you real value of the re-ordering here.


  static const struct dt_device_match timer_ids[] __initconst =
  {
  DT_MATCH_COMPATIBLE("arm,armv7-timer"),
@@ -973,15 +977,6 @@ static int __init make_timer_node(const struct kernel_info 
*kinfo)
  { /* sentinel */ },
  };
  struct dt_device_node *dev;
-u32 len;
-const void *compatible;
-int res;
-unsigned int irq;
-gic_interrupt_t intrs[3];
-u32 clock_frequency;
-bool clock_valid;
-
-dt_dprintk("Create timer node\n");


Why is it dropped?

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] Intel TXT: add reviewer, move to Odd Fixes state

2019-07-31 Thread Lars Kurth

> On 31 Jul 2019, at 12:52, Julien Grall  wrote:
> 
>> To move forward:
>> * There should be a policy discussion
> 
> How should I raise it? Do you want a patch again contribution-guidelines?

I think we should start with an e-mail thread with an appropriate title on 
xen-devel@ (CCing committers@) outlining 
* What the proposal is and why it is important
  - How we document it is secondary and I am happy to pick this up after there 
is agreement 
* How it would be implemented - e.g. if the proposal was to reject e-mails with 
a disclaimer, we need to have a mechanism that does this reliably and also 
informs senders why a mail was not posted. We wouldn't want xen-devel@ to 
become a black hole, where stuff from some people gets lost without

It then would have to go through a vote as normal. You may want to have a chat 
to Ian Jackson on IRC: he has some opinions and experience that is applicable

I just agreed with Ian, that there will be a similar discussion related to the 
2 step process to change maintainers via unsupported status, which also was 
highlighted in this thread. Although this can probably just be a patch to 
MAINTAINERS

Regards
Lars
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 1/2] xen/arm: extend fdt_property_interrupts


Hi,

You should have enough characters in the title to explain what you are 
extending. Something like:


xen/arm: Extend fdt_property_interrupts to support domU

On 31/07/2019 11:28, Viktor Mitin wrote:

Extend fdt_property_interrupts to deal with other domain than the hwdom.

The prototype of fdt_property_interrupts() has been modified
to support both hwdom and domU in one function.

This is a preparatory for the patch "xen/arm: merge make_timer_node and
make_timer_domU_node". Original goal is to merge make_timer_node and
make_timer_domU_node functions. See discussion in e-mail, the Message-ID is:
<20190620103805.927-1-viktor.mitin...@gmail.com>


The commit message should not point to discussion but summarize it. If you want 
to point to the discussion, then please do it after ---.


Also, this is a bit risky to write down the title of a patch that does not 
preceded it (or been committed). Image we decide to rename it... Instead, it is 
common to say "A follow-up patch will need to create the interrupts for either 
dom0 or domU".




Note: there is no functional changes introduced by this patch.


You are expanding the function to this is technically a functional changes.



Suggested-by: Julien Grall 
Signed-off-by: Viktor Mitin 
---
  xen/arch/arm/domain_build.c | 22 +-
  1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 4c8404155a..d04a1c3aec 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -621,17 +621,19 @@ static void __init set_interrupt(gic_interrupt_t 
interrupt,
   *  "interrupts": contains the list of interrupts
   *  "interrupt-parent": link to the GIC
   */
-static int __init fdt_property_interrupts(void *fdt, gic_interrupt_t *intr,
-  unsigned num_irq)
+static int __init fdt_property_interrupts(const struct kernel_info *kinfo,
+gic_interrupt_t *intr, unsigned num_irq)


Please align the section line with the first argument.


  {
  int res;
+uint32_t phandle = is_hardware_domain(kinfo->d) ?
+   dt_interrupt_controller->phandle : GUEST_PHANDLE_GIC;
  
-res = fdt_property(fdt, "interrupts", intr, sizeof (intr[0]) * num_irq);

+res = fdt_property(kinfo->fdt, "interrupts",
+   intr, sizeof (intr[0]) * num_irq);
  if ( res )
  return res;
  
-res = fdt_property_cell(fdt, "interrupt-parent",

-dt_interrupt_controller->phandle);
+res = fdt_property_cell(kinfo->fdt, "interrupt-parent", phandle);
  
  return res;

  }
@@ -733,7 +735,7 @@ static int __init make_hypervisor_node(struct domain *d,
   *  TODO: Handle properly the cpumask;
   */
  set_interrupt(intr, d->arch.evtchn_irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
-res = fdt_property_interrupts(fdt, , 1);
+res = fdt_property_interrupts(kinfo, , 1);
  if ( res )
  return res;
  
@@ -960,8 +962,10 @@ static int __init make_gic_node(const struct domain *d, void *fdt,

  return res;
  }
  
-static int __init make_timer_node(const struct domain *d, void *fdt)

+static int __init make_timer_node(const struct kernel_info *kinfo)


This change is still not explained in the commit message.


  {
+void *fdt = kinfo->fdt;
+


You add the newline here but drop it in the next patch. In general, it is 
strongly recommended to not add code this is removed in the same series unless 
there are a reason to.



  static const struct dt_device_match timer_ids[] __initconst =
  {
  DT_MATCH_COMPATIBLE("arm,armv7-timer"),
@@ -1016,7 +1020,7 @@ static int __init make_timer_node(const struct domain *d, 
void *fdt)
  dt_dprintk("  Virt interrupt %u\n", irq);
  set_interrupt(intrs[2], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
  
-res = fdt_property_interrupts(fdt, intrs, 3);

+res = fdt_property_interrupts(kinfo, intrs, 3);
  if ( res )
  return res;
  
@@ -1377,7 +1381,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,

  if ( device_get_class(node) == DEVICE_GIC )
  return make_gic_node(d, kinfo->fdt, node);
  if ( dt_match_node(timer_matches, node) )
-return make_timer_node(d, kinfo->fdt);
+return make_timer_node(kinfo);
  
  /* Skip nodes used by Xen */

  if ( dt_device_used_by(node) == DOMID_XEN )



Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 2/2] xen/arm: platform: Add Raspberry Pi platform

2019-07-31 Thread Stewart Hildebrand

On Wednesday, July 31, 2019 8:04 AM, Julien Grall  wrote:
>Hi Stewart,

Hi Julien

>On 29/07/2019 14:19, Stewart Hildebrand wrote:
>> The aux peripherals (uart1, spi1, and spi2) share an IRQ and a page of
>> memory. For debugging, it is helpful to use the aux UART in Xen. In
>> this case, Xen would try to assign spi1 and spi2 to dom0, but this
>> results in an error since the shared IRQ was already assigned to Xen.
>> Blacklist aux devices other than the UART to prevent mapping the shared
>> IRQ and memory range to dom0.
>
>Reading the commit message, it is unclear what's the impact on blacklist spi1
>and spi2. Could you expand it?

Yes, good thinking. What do you think about the following (the first paragraph 
is unchanged, just copied for completeness):

"The aux peripherals (uart1, spi1, and spi2) share an IRQ and a page of
memory. For debugging, it is helpful to use the aux UART in Xen. In
this case, Xen would try to assign spi1 and spi2 to dom0, but this
results in an error since the shared IRQ was already assigned to Xen.
Blacklist aux devices other than the UART to prevent mapping the shared
IRQ and memory range to dom0.

Blacklisting spi1 and spi2 unfortunately makes those peripherals
unavailable for use in the system. Future work could include forwarding
the IRQ for spi1 and spi2, and trap and mediate access to the memory
range for spi1 and spi2."

Would you like me to re-send the patch, or can the message be updated on commit?

Stew

>The rest of the patch looks good.
>
>Cheers,
>
>--
>Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v5] Speculative mitigation facilities report wrong status

On 31/07/2019 14:33, Jin Nan Wang wrote:
> Booting with spec-ctrl=0 results in Xen printing "None MD_CLEAR".
>
> (XEN)   Support for HVM VMs: None MD_CLEAR
> (XEN)   Support for PV VMs: None MD_CLEAR
>
> Add a check about X86_FEATURE_MD_CLEAR to avoid to print "None".
>
> Signed-off-by: James Wang 

Reviewed-by: Andrew Cooper 

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

[Xen-devel] [PATCH v5] Speculative mitigation facilities report wrong status

Booting with spec-ctrl=0 results in Xen printing "None MD_CLEAR".

(XEN)   Support for HVM VMs: None MD_CLEAR
(XEN)   Support for PV VMs: None MD_CLEAR

Add a check about X86_FEATURE_MD_CLEAR to avoid to print "None".

Signed-off-by: James Wang 
---
 xen/arch/x86/spec_ctrl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index cada9a058e..468a847598 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -366,6 +366,7 @@ static void __init print_details(enum ind_thunk thunk, 
uint64_t caps)
 printk("  Support for HVM VMs:%s%s%s%s%s\n",
(boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
 boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ||
+boot_cpu_has(X86_FEATURE_MD_CLEAR)   ||
 opt_eager_fpu)   ? ""   : " 
None",
boot_cpu_has(X86_FEATURE_SC_MSR_HVM)  ? " MSR_SPEC_CTRL" : "",
boot_cpu_has(X86_FEATURE_SC_RSB_HVM)  ? " RSB"   : "",
@@ -377,6 +378,7 @@ static void __init print_details(enum ind_thunk thunk, 
uint64_t caps)
 printk("  Support for PV VMs:%s%s%s%s%s\n",
(boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
 boot_cpu_has(X86_FEATURE_SC_RSB_PV) ||
+boot_cpu_has(X86_FEATURE_MD_CLEAR)  ||
 opt_eager_fpu)   ? ""   : " 
None",
boot_cpu_has(X86_FEATURE_SC_MSR_PV)   ? " MSR_SPEC_CTRL" : "",
boot_cpu_has(X86_FEATURE_SC_RSB_PV)   ? " RSB"   : "",
-- 
2.22.0


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

[Xen-devel] [PATCH] Speculative mitigation facilities report wrong status V4

Booting with spec-ctrl=0 results in Xen printing "None MD_CLEAR".

(XEN)   Support for HVM VMs: None MD_CLEAR
(XEN)   Support for PV VMs: None MD_CLEAR

Add a check about X86_FEATURE_MD_CLEAR to avoid to print "None".

Signed-off-by: James Wang 
---
 xen/arch/x86/spec_ctrl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index cada9a058e..468a847598 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -366,6 +366,7 @@ static void __init print_details(enum ind_thunk thunk, 
uint64_t caps)
 printk("  Support for HVM VMs:%s%s%s%s%s\n",
(boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
 boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ||
+boot_cpu_has(X86_FEATURE_MD_CLEAR)   ||
 opt_eager_fpu)   ? ""   : " 
None",
boot_cpu_has(X86_FEATURE_SC_MSR_HVM)  ? " MSR_SPEC_CTRL" : "",
boot_cpu_has(X86_FEATURE_SC_RSB_HVM)  ? " RSB"   : "",
@@ -377,6 +378,7 @@ static void __init print_details(enum ind_thunk thunk, 
uint64_t caps)
 printk("  Support for PV VMs:%s%s%s%s%s\n",
(boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
 boot_cpu_has(X86_FEATURE_SC_RSB_PV) ||
+boot_cpu_has(X86_FEATURE_MD_CLEAR)  ||
 opt_eager_fpu)   ? ""   : " 
None",
boot_cpu_has(X86_FEATURE_SC_MSR_PV)   ? " MSR_SPEC_CTRL" : "",
boot_cpu_has(X86_FEATURE_SC_RSB_PV)   ? " RSB"   : "",
-- 
2.22.0


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

Re: [Xen-devel] [PATCH v4 2/2] xen/arm: merge make_timer_node and make_timer_domU_node


Viktor Mitin writes:

> On Wed, Jul 31, 2019 at 3:33 PM Volodymyr Babchuk
>  wrote:
>>
>>
>>
>> Viktor Mitin writes:
>>
>> > Merged make_timer_node and make_timer_domU_node into one function
>> > make_timer_node.
>> It is widely accepted to write commit messages in imperative mood,
>> e.g. "merge" instead of "merged"
>>
>> > Kept the domU version for the compatible as it is simpler.
>> > Kept the hw version for the clock as it is relevant for the both cases.
>> ... or "keep" instead of "kept"
>
> Well, again, there is no such rule in the coding style document.
Yeah, but this is widely accepted style. It is good to have all commit
messages in the same style, isn't?

>> > Suggested-by: Julien Grall 
>> > Signed-off-by: Viktor Mitin 
>> > ---
>> > v4 updates:
>> >updated "Kept the domU version for the compatible as it is simpler"
>> >
>> >  xen/arch/arm/domain_build.c | 109 +---
>> >  1 file changed, 39 insertions(+), 70 deletions(-)
>> >
>> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> > index d04a1c3aec..4d7c3411a6 100644
>> > --- a/xen/arch/arm/domain_build.c
>> > +++ b/xen/arch/arm/domain_build.c
>> > @@ -964,8 +964,12 @@ static int __init make_gic_node(const struct domain 
>> > *d, void *fdt,
>> >
>> >  static int __init make_timer_node(const struct kernel_info *kinfo)
>> >  {
>> > +int res;
>> >  void *fdt = kinfo->fdt;
>> > -
>> In the previous patch you added this empty string, now you are deleting
>> it.
>
> Why not? Do not remember why did it, probably it was more convenient
> at that moment.
> Anyway, why not?
Because patches should not include unnecessary changes. You are spending
reviewer's mental resources by introducing unneeded changes and then
undoing them in the next patch.

>>
>> > +unsigned int irq[MAX_TIMER_PPI];
>> MAX_TIMER_PPI equals to 4, but looks like you are using only first 3
>> items of the array.
>
> Yes. This is because MAX_TIMER_PPI has been defined, and this
> particular example is taken from time.c
Yes, but code in time.c uses all four IRQs. In your case you just wasting
extra space on stack.

>>
>> > +gic_interrupt_t intrs[3];
>> > +u32 clock_frequency;
>> > +bool clock_valid;
>> Do you really need to move those declarations?
>
> Not really, it has appeared as a result of many code edit iterations.
> As I mentioned previously, those patches are changed several times already,
> so the final version has another order of the local variables. Why not?
Because patches should do only necessary things. If you for some reason
want to tidy up variable declaration, please do this in the separate patch.

>> >  static const struct dt_device_match timer_ids[] __initconst =
>> >  {
>> >  DT_MATCH_COMPATIBLE("arm,armv7-timer"),
>> > @@ -973,15 +977,6 @@ static int __init make_timer_node(const struct 
>> > kernel_info *kinfo)
>> >  { /* sentinel */ },
>> >  };
>> >  struct dt_device_node *dev;
>> > -u32 len;
>> > -const void *compatible;
>> > -int res;
>> > -unsigned int irq;
>> > -gic_interrupt_t intrs[3];
>> > -u32 clock_frequency;
>> > -bool clock_valid;
>> > -
>> > -dt_dprintk("Create timer node\n");
>> >
>> >  dev = dt_find_matching_node(NULL, timer_ids);
>> >  if ( !dev )
>> > @@ -990,35 +985,49 @@ static int __init make_timer_node(const struct 
>> > kernel_info *kinfo)
>> >  return -FDT_ERR_XEN(ENOENT);
>> >  }
>> >
>> > -compatible = dt_get_property(dev, "compatible", );
>> > -if ( !compatible )
>> > -{
>> > -dprintk(XENLOG_ERR, "Can't find compatible property for timer 
>> > node\n");
>> > -return -FDT_ERR_XEN(ENOENT);
>> > -}
>> > -
>> >  res = fdt_begin_node(fdt, "timer");
>> >  if ( res )
>> >  return res;
>> >
>> > -res = fdt_property(fdt, "compatible", compatible, len);
>> > -if ( res )
>> > -return res;
>> > +if ( !is_64bit_domain(kinfo->d) )
>> > +{
>> > +res = fdt_property_string(fdt, "compatible", "arm,armv7-timer");
>> > +if ( res )
>> > +return res;
>> > +}
>> > +else
>> > +{
>> > +res = fdt_property_string(fdt, "compatible", "arm,armv8-timer");
>> > +if ( res )
>> > +return res;
>> > +}
>> So, previously this code copied "compatible" property from platform
>> device tree. Please note, that theoretically it would be neither
>> "arm,armv8-timer" not "arm,armv7-timer". Now you are setting one of the
>> two values. I'm not sure if this is right thing to do in the first
>> place. Probably we need comment from Julien. But this change should be
>> reflected in the commit message.
>
> Well, it is done, because Julien preferred domU variant as more simple one.
> Actually I have checked that both variats works well, but kept domU case.
My concern is that you are changing function behavior in
non-backward compatible way. Yes, it is working on your platform. But
what

Re: [Xen-devel] [PATCH v3 1/2] x86/ubsan: Don't perform alignment checking on supporting compilers

On 26/07/2019 08:33, Jan Beulich wrote:
 On 27.06.19 at 20:56,  wrote:
>> GCC 5 introduced -fsanitize=alignment which is enabled by default by
>> CONFIG_UBSAN.  This trips a load of wont-fix cases in the ACPI tables and the
>> hypercall page and stubs writing logic.
>>
>> It also causes the native Xen boot to crash before the console is set up, for
>> an as-yet unidentified reason (most likley a wont-fix case earlier on boot).
>>
>> Disable alignment sanitisation on compilers which would try using it.
>>
>> Signed-off-by: Andrew Cooper 
> Reviewed-by: Jan Beulich 
>
> I'm sorry for the delay - it was only now that I've been told how
> to access the mails still delivered to my old mailbox between me
> leaving the office that day and the switch of mailboxes actually
> having happened.

TBH, I'd completely forgotten about this series.  Thanks for the reminder.

~Andrew

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

[Xen-devel] [PATCH] Speculative mitigation facilities report wrong status v3

Add a check about X86_FEATURE_MD_CLEAR to avoid to print "None".

Signed-off-by: James Wang 
---
 xen/arch/x86/spec_ctrl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index cada9a058e..468a847598 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -366,6 +366,7 @@ static void __init print_details(enum ind_thunk thunk, 
uint64_t caps)
 printk("  Support for HVM VMs:%s%s%s%s%s\n",
(boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
 boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ||
+boot_cpu_has(X86_FEATURE_MD_CLEAR)   ||
 opt_eager_fpu)   ? ""   : " 
None",
boot_cpu_has(X86_FEATURE_SC_MSR_HVM)  ? " MSR_SPEC_CTRL" : "",
boot_cpu_has(X86_FEATURE_SC_RSB_HVM)  ? " RSB"   : "",
@@ -377,6 +378,7 @@ static void __init print_details(enum ind_thunk thunk, 
uint64_t caps)
 printk("  Support for PV VMs:%s%s%s%s%s\n",
(boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
 boot_cpu_has(X86_FEATURE_SC_RSB_PV) ||
+boot_cpu_has(X86_FEATURE_MD_CLEAR)  ||
 opt_eager_fpu)   ? ""   : " 
None",
boot_cpu_has(X86_FEATURE_SC_MSR_PV)   ? " MSR_SPEC_CTRL" : "",
boot_cpu_has(X86_FEATURE_SC_RSB_PV)   ? " RSB"   : "",
-- 
2.22.0


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

Re: [Xen-devel] [PATCH v3 07/10] xen/nodemask: Drop nodes_{setall, clear}() and improve the initialisers

On 31/07/2019 14:12, Jan Beulich wrote:
> On 31.07.2019 14:49, Andrew Cooper wrote:
>> I don't see a way to avoid expanding node twice, but given that its wrapper 
>> is in ALL_CAPS and obviously a macro.
>>
>> Furthermore, experimenting with a deliberate attempt to provoke this, I got
>>
>> numa.c: In function ‘numa_initmem_init’:
>>
>> /local/xen.git/xen/include/xen/nodemask.h:90:10: error: nonconstant array 
>> index in initializer
>>
>>   [(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG)  \
>>
>>    ^
>>
>> numa.c:274:23: note: in expansion of macro ‘NODEMASK_OF’
>>
>>   node_online_map = NODEMASK_OF(foo++);
>>
>>     ^~~
>>
>> /local/xen.git/xen/include/xen/nodemask.h:90:10: note: (near initialization 
>> for ‘(anonymous).bits’)
>>
>>   [(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG)  \
>>
>>    ^
>>
>> numa.c:274:23: note: in expansion of macro ‘NODEMASK_OF’
>>
>>   node_online_map = NODEMASK_OF(foo++);
>>
>>     ^~~
>>
>> from GCC 6.3, which I think covers everything we need, and will prevent side 
>> effects from double expansion in practice.
> Ah, right. With this my R-b applies to the change as is (with the
> additional adjustments folded in that you've pointed out).

I've actually tweaked it a fraction more to not bifurcate NODEMASK_OF()
in an ifdef, which means we'll get diagnostics like that out of the
compiler even when I haven't hacked NODES_SHIFT to 7 locally.

However, it now touches ARM code so I'll post a full v4 series with this
and later issues in the series addressed.

~Andrew

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

Re: [Xen-devel] [PATCH v3 07/10] xen/nodemask: Drop nodes_{setall, clear}() and improve the initialisers

2019-07-31 Thread Jan Beulich

On 31.07.2019 14:49, Andrew Cooper wrote:
> I don't see a way to avoid expanding node twice, but given that its wrapper 
> is in ALL_CAPS and obviously a macro.
> 
> Furthermore, experimenting with a deliberate attempt to provoke this, I got
> 
> numa.c: In function ‘numa_initmem_init’:
> 
> /local/xen.git/xen/include/xen/nodemask.h:90:10: error: nonconstant array 
> index in initializer
> 
>   [(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG)  \
> 
>    ^
> 
> numa.c:274:23: note: in expansion of macro ‘NODEMASK_OF’
> 
>   node_online_map = NODEMASK_OF(foo++);
> 
>     ^~~
> 
> /local/xen.git/xen/include/xen/nodemask.h:90:10: note: (near initialization 
> for ‘(anonymous).bits’)
> 
>   [(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG)  \
> 
>    ^
> 
> numa.c:274:23: note: in expansion of macro ‘NODEMASK_OF’
> 
>   node_online_map = NODEMASK_OF(foo++);
> 
>     ^~~
> 
> from GCC 6.3, which I think covers everything we need, and will prevent side 
> effects from double expansion in practice.

Ah, right. With this my R-b applies to the change as is (with the
additional adjustments folded in that you've pointed out).

Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] Speculative mitigation facilities report wrong status v2

On 31/07/2019 14:09, Jin Nan Wang wrote:

This wants at least a minimal aspect of description from your first
patch, i.e. that booting with spec-ctrl=0 results in Xen printing "None
MD_CLEAR".

> Add a check about X86_FEATURE_MD_CLEAR to avoid to print "None".

Needs a SoB tag.

The actual code change is correct.

~Andrew

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

Re: [Xen-devel] [PATCH v2 07/10] vm_event: Add vm_event_ng interface

2019-07-31 Thread Jan Beulich

On 31.07.2019 14:53, Petre Ovidiu PIRCALABU wrote:
> On Thu, 2019-07-18 at 14:44 +, Jan Beulich wrote:
>> On 18.07.2019 15:59, Petre Ovidiu PIRCALABU wrote:
>>> Before using xenforeignmemory_map_resource I investigated several
>>> different approaches:
>>> - Allocate the memory in hypervisor and xc_map_foreign_pages
>>> (doesn't
>>> work because you cannot "foreignmap" pages of your own domain.
>>> - Allocate the memory in guest, and map the allocated pages in XEN.
>>> To
>>> my knowledge there is no such API in linux to do this and the
>>> monitor
>>> application, as an userspace program, is not aware of the actual
>>> gfns
>>> for an allocated memory area.
>>>
>>> So, at this point the most promising solution is allocating the
>>> memory
>>> in XEN, sharing it with ID using share_xen_page_with_guest, and
>>> mapping
>>> it with xenforeignmemory_map_resource (with the flag
>>> XENMEM_rsrc_acq_caller_owned set)
>>
>> Which is fine - that's why Paul had introduced the generic interface.
>>
>>> To my understanding the cleanup sequence from
>>> gnttab_unpopulate_status_frames basically boils down to:
>>> 1. guest_physmap_remove_page
>>> 2. if ( test_and_clear_bit(_PGC_allocated, >count_info) )
>>>  put_page(pg);
>>> 3. free_xenheap_page
>>
>> You're missing the crucial part of undoing step 2 if you find
>> that there are still references left to the page.
>>
>> And then, because of your use of vzalloc(), you can't use
>> free_xenheap_pages(), as that takes a (direct map) linear address
>> as input. It has to be free_domheap_pages() in your case.
>>
>>> My current implementation uses vzalloc instead of
>>> alloc_xenheap_pages
>>> and that's why I assumed vunmap and free_domheap_pages will suffice
>>> (I
>>> would have called vfree directly, but the temporary linked list
>>> that is
>>> used to hold the page references causes free_domheap_pages to
>>> crash)
>>>
>>> Do I also have to call guest_physmap_remove_page and put_page?
>>> (steps
>>> 1. and 2.)
>>
>> guest_physmap_remove_page() needs to be called for translated page-
>> owning domains if the page was ever added to their physmap. As long
>> as you avoid adding, you also don't need to remove. I don't recall
>> though whether a translated domain can access a resource without it
>> having a representation in its GFN space.
>>
> I've traced the GFN value for the shared MFN and it's invalid. It's set
> to INVALID_M2P_ENTRY in share_xen_page_with_guest, but I was expecting
> it to be set to valid value later on (e.g. xenmem_add_to_physmap).
> Am I missing something?

By "traced" do you mean "observed" (e.g. by way of logging) or "gone
through the code to verify it can't ever become valid"? In the latter
case you'd have proven what you want/need. Thinking about it though,
seeing how xenmem_add_to_physmap_one() works and assuming you indeed
don't add the page by other means, it looks to me as if you've done
more than is needed, as I've said already that you need to remove the
page only if it had been added.

Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH] Speculative mitigation facilities report wrong status v2

Add a check about X86_FEATURE_MD_CLEAR to avoid to print "None".
---
 xen/arch/x86/spec_ctrl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index cada9a058e..468a847598 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -366,6 +366,7 @@ static void __init print_details(enum ind_thunk thunk, 
uint64_t caps)
 printk("  Support for HVM VMs:%s%s%s%s%s\n",
(boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
 boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ||
+boot_cpu_has(X86_FEATURE_MD_CLEAR)   ||
 opt_eager_fpu)   ? ""   : " 
None",
boot_cpu_has(X86_FEATURE_SC_MSR_HVM)  ? " MSR_SPEC_CTRL" : "",
boot_cpu_has(X86_FEATURE_SC_RSB_HVM)  ? " RSB"   : "",
@@ -377,6 +378,7 @@ static void __init print_details(enum ind_thunk thunk, 
uint64_t caps)
 printk("  Support for PV VMs:%s%s%s%s%s\n",
(boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
 boot_cpu_has(X86_FEATURE_SC_RSB_PV) ||
+boot_cpu_has(X86_FEATURE_MD_CLEAR)  ||
 opt_eager_fpu)   ? ""   : " 
None",
boot_cpu_has(X86_FEATURE_SC_MSR_PV)   ? " MSR_SPEC_CTRL" : "",
boot_cpu_has(X86_FEATURE_SC_RSB_PV)   ? " RSB"   : "",
-- 
2.22.0


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

Re: [Xen-devel] [PATCH v2 07/10] vm_event: Add vm_event_ng interface

2019-07-31 Thread Petre Ovidiu PIRCALABU

On Thu, 2019-07-18 at 14:44 +, Jan Beulich wrote:
> On 18.07.2019 15:59, Petre Ovidiu PIRCALABU wrote:
> > Before using xenforeignmemory_map_resource I investigated several
> > different approaches:
> > - Allocate the memory in hypervisor and xc_map_foreign_pages
> > (doesn't
> > work because you cannot "foreignmap" pages of your own domain.
> > - Allocate the memory in guest, and map the allocated pages in XEN.
> > To
> > my knowledge there is no such API in linux to do this and the
> > monitor
> > application, as an userspace program, is not aware of the actual
> > gfns
> > for an allocated memory area.
> > 
> > So, at this point the most promising solution is allocating the
> > memory
> > in XEN, sharing it with ID using share_xen_page_with_guest, and
> > mapping
> > it with xenforeignmemory_map_resource (with the flag
> > XENMEM_rsrc_acq_caller_owned set)
> 
> Which is fine - that's why Paul had introduced the generic interface.
> 
> > To my understanding the cleanup sequence from
> > gnttab_unpopulate_status_frames basically boils down to:
> > 1. guest_physmap_remove_page
> > 2. if ( test_and_clear_bit(_PGC_allocated, >count_info) )
> > put_page(pg);
> > 3. free_xenheap_page
> 
> You're missing the crucial part of undoing step 2 if you find
> that there are still references left to the page.
> 
> And then, because of your use of vzalloc(), you can't use
> free_xenheap_pages(), as that takes a (direct map) linear address
> as input. It has to be free_domheap_pages() in your case.
> 
> > My current implementation uses vzalloc instead of
> > alloc_xenheap_pages
> > and that's why I assumed vunmap and free_domheap_pages will suffice
> > (I
> > would have called vfree directly, but the temporary linked list
> > that is
> > used to hold the page references causes free_domheap_pages to
> > crash)
> > 
> > Do I also have to call guest_physmap_remove_page and put_page?
> > (steps
> > 1. and 2.)
> 
> guest_physmap_remove_page() needs to be called for translated page-
> owning domains if the page was ever added to their physmap. As long
> as you avoid adding, you also don't need to remove. I don't recall
> though whether a translated domain can access a resource without it
> having a representation in its GFN space.
> 
I've traced the GFN value for the shared MFN and it's invalid. It's set
to INVALID_M2P_ENTRY in share_xen_page_with_guest, but I was expecting
it to be set to valid value later on (e.g. xenmem_add_to_physmap). 
Am I missing something? 

Many thanks,
Petre

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

Re: [Xen-devel] [PATCH v4 1/2] xen/arm: extend fdt_property_interrupts


Hi,

I am only going to comment on process. I will look at the rest later on.

On 31/07/2019 13:28, Viktor Mitin wrote:

Hi Volodymyr,

On Wed, Jul 31, 2019 at 3:11 PM Volodymyr Babchuk
 wrote:



Hi Viktor,

It is recommended (and probably required, but I can't find exact place
in the rules) to include cover letter if you are sending more that one
patch in series. This will ease up review process, because reviewer will
know what to expect in the series.

 > There is no such requirement, only recommendation.


It is a strong recommendation: "If you need to send more than one patches (which 
normally means you're sending a patch series with cover letter),".



I did not put it since this is simple short patch series and both
patches in this series have been discussed previously, so it is known
what it is about.


For a first, if you don't have a cover letter then the threading in e-mail 
client would look weird:

   [PATCH v4 1/2] xen/arm: extend fdt_property_interrupts
  |-> [PATCH v4 2/2] xen/arm: merge make_timer_node and make_timer_domU_node

I tend to hid anything within the thread so I have only one title. For the title 
it is not clear to me what's the purpose of the e-mail.


The cover letter is also used to keep a summary of what was discussed and the 
overall goal. It does not matter if it is just a few lines. This is also a good 
place to have a discussion of the overall series (i.e not specific to a patch).


Lastly, you may have new reviewers that haven't followed the previous 
discussion. You have also reviewers like me which receive a few hundreds e-mails 
per week (just counting my inbox so e-mail I am CCed to). While I have a good 
memory, I can't possibly remember everything single e-mails.


So the cover letter is a good place to explain what changes have been done 
between series. You can also do that per-patch.


Speaking about changelog, I would highly recommend to keep all the changelog 
from v1. This gives us an idea what happen over the review.





Viktor Mitin writes:


Extend fdt_property_interrupts to deal with other domain than the hwdom.

The prototype of fdt_property_interrupts() has been modified
to support both hwdom and domU in one function.

This is a preparatory for the patch "xen/arm: merge make_timer_node and
make_timer_domU_node". Original goal is to merge make_timer_node and
make_timer_domU_node functions. See discussion in e-mail, the Message-ID is:
<20190620103805.927-1-viktor.mitin...@gmail.com>

Note: there is no functional changes introduced by this patch.

This is not completely true, because you change the way how phandle is
retrieved. Also, earlier you said that "fdt_property_interrupts() has
been modified to support both hwdom and domU in one function". This is
the functional change.


Phandle is retreved the same way:
in case of  hwdom it is dt_interrupt_controller->phandle
in case of domU it is GUEST_PHANDLE_GIC.
Don't see any functional change here.

What is 'functional change' in your opinion?


The function is now extended so technically it has changed. "No functional 
change" is usually used when the code is consolidated/reworked.








Suggested-by: Julien Grall 
Signed-off-by: Viktor Mitin 
---
  xen/arch/arm/domain_build.c | 22 +-
  1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 4c8404155a..d04a1c3aec 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -621,17 +621,19 @@ static void __init set_interrupt(gic_interrupt_t 
interrupt,
   *  "interrupts": contains the list of interrupts
   *  "interrupt-parent": link to the GIC
   */
-static int __init fdt_property_interrupts(void *fdt, gic_interrupt_t *intr,
-  unsigned num_irq)
+static int __init fdt_property_interrupts(const struct kernel_info *kinfo,
+gic_interrupt_t *intr, unsigned num_irq)

As I said earlier, this formatting contradicts with the coding style.


There is no such coding style requirement (not explicitly documented).
Even more, the original code formatted the same way.


What original code? This is formatted with all the parameters aligned. So can 
you please do it.





  {
  int res;
+uint32_t phandle = is_hardware_domain(kinfo->d) ?
+   dt_interrupt_controller->phandle : GUEST_PHANDLE_GIC;

-res = fdt_property(fdt, "interrupts", intr, sizeof (intr[0]) * num_irq);
+res = fdt_property(kinfo->fdt, "interrupts",
+   intr, sizeof (intr[0]) * num_irq);
  if ( res )
  return res;

-res = fdt_property_cell(fdt, "interrupt-parent",
-dt_interrupt_controller->phandle);
+res = fdt_property_cell(kinfo->fdt, "interrupt-parent", phandle);

  return res;
  }
@@ -733,7 +735,7 @@ static int __init make_hypervisor_node(struct domain *d,
   *  TODO: Handle properly the cpumask;
   */

Re: [Xen-devel] [PATCH] Intel TXT: add reviewer, move to Odd Fixes state

2019-07-31 Thread Hawrylko, Lukasz

I am waiting for another mail address dedicated for mailing lists that
has the disclaimer disabled. This is an official way in Intel to do
that. I don't know when it will be ready, but I expect that this process
can take few days.

From my perspective we can wait until I will have that mail address, so
I can submit this patch in "blessed" way. Sorry for this confusion.

Lukasz

On Wed, 2019-07-31 at 12:52 +0100, Julien Grall wrote:
> Hi Lars,
> 
> On 30/07/2019 12:22, Lars Kurth wrote:
> > 
> > > On 30 Jul 2019, at 11:08, George Dunlap <
> > > george.dun...@citrix.com
> > >  
> > >  > > george.dun...@citrix.com
> > > >> wrote:
> > > 
> > > On 7/30/19 10:54 AM, Julien Grall wrote:
> > > > Hi Jan,
> > > > 
> > > > On 30/07/2019 10:05, Jan Beulich wrote:
> > > > > On 30.07.2019 10:54, Julien Grall wrote:
> > > > > > On 7/30/19 9:29 AM, Jan Beulich wrote:
> > > > > > > On 30.07.2019 08:56, Lukasz Hawrylko wrote:
> > 
> > +full committers list and Juergen
> > 
> > OK. We should have a separate discussion related to disclaimers:
> > make a formal 
> > decision and afterwards document it in the contribution workflow. I
> > agree that 
> > this makes sense, and this has been raised by Julien in the past
> > privately 
> > related to questions on xen-devel@. It then turned out that Arm
> > folks from China 
> > have consistently used disclaimers on contributions to mini-os and
> > unikraft. 
> > This has stopped now, which is to Julien's credit. I suggested than
> > that Julien 
> > should raise this issue formally as a policy change, which never
> > happened.
> > 
> > I do not believe that we should block any patches from being applied
> > due to 
> > disclaimers in absence of an agreed policy. Contributors sign a DCO
> > and that may 
> > well override a disclaimer (we do not have access to the legal
> > advice that Greg 
> > KH refers to). If there was a serious legal issue, the LF would have
> > contacted 
> > all of its projects. And I also could not find any public reference
> > to such an 
> > issue. This definitely something where the Advisory Board should
> > have some input.
> > 
> > And in particular this patch also contains no code and should not be
> > blocked on 
> > these grounds.
> 
> I originally objected on this patch because the disclaimer issue was
> pointed out 
> 3 versions ago and still not addressed. This then went on the
> discussion with 
> Jan about the disclaimer.
> 
> While reviewer only means you are CC to e-mails, I would at least
> expect them to 
> understand the process and be able to address comments.
> 
> > @Lukasz: please take note of this issue for the next time round. It
> > should be 
> > easy enough to disable the disclaimer when sending to certain lists
> 
> It is not easy enough as you may think ;). At Arm we have to go
> through a 
> different SMTP server so we bypass exchange.
> 
> > To move forward:
> > * There should be a policy discussion
> 
> How should I raise it? Do you want a patch again contribution-
> guidelines?
> 
> > * There should be AB input
> > * The outcome should be documented in 
> > https://xenproject.org/help/contribution-guidelines/
> >  and the git contribution 
> > workflow
> 
> Cheers,
> 
> 


smime.p7s
Description: S/MIME cryptographic signature


Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial 
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | 
Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). If you are not the intended recipient, please 
contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 07/10] xen/nodemask: Drop nodes_{setall, clear}() and improve the initialisers

On 30/07/2019 10:44, Jan Beulich wrote:

> On 29.07.2019 14:12, Andrew Cooper wrote:
>> There is no need to use runtime variable-length clearing when MAX_NUMNODES is
>> known to the compiler.  Drop these functions and use the initialisers 
>> instead.
> The only slight concern I have with this is that it further locks
> down the maximum remaining to be a compile time constant. But this
> is not an objection, just a remark.

The maximum number of nodes I'm aware of at all is 10, and we currently default 
to 64.

I don't think it is likely that we'll get to a point where a runtime nodesize 
is a realistic consideration that we would want to take.

>
>> @@ -67,7 +65,34 @@ typedef struct { DECLARE_BITMAP(bits, MAX_NUMNODES); } 
>> nodemask_t;
>>   
>>   #define nodemask_bits(src) ((src)->bits)
>>   
>> -extern nodemask_t _unused_nodemask_arg_;
>> +#define NODEMASK_LAST_WORD BITMAP_LAST_WORD_MASK(MAX_NUMNODES)
>> +
>> +#define NODEMASK_NONE   \
>> +((nodemask_t) {{\
>> +[0 ... BITS_TO_LONGS(MAX_NUMNODES) - 1] = 0 \
>> +}})
>> +
>> +#if MAX_NUMNODES <= BITS_PER_LONG
>> +
>> +#define NODEMASK_ALL  ((nodemask_t) {{ NODEMASK_LAST_WORD }})
>> +#define NODEMASK_OF(node) ((nodemask_t) {{ 1UL << (node) }})
>> +
>> +#else /* MAX_NUMNODES > BITS_PER_LONG */
>> +
>> +#define NODEMASK_ALL\
>> +((nodemask_t) {{\
>> +[0 ... BITS_TO_LONGS(MAX_NUMNODES) - 2] = ~0UL, \
>> +[BITS_TO_LONGS(MAX_NUMNODES) - 1] = NODEMASK_LAST_WORD  \
>> +}})
>> +
>> +#define NODEMASK_OF(node)   \
>> +({  \
>> +nodemask_t m = NODES_NONE;  \
>> +m.bits[(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG);   \
> I think you will want to avoid the double evaluation of "node"
> here. With this taken care of
> Reviewed-by: Jan Beulich 

I'm afraid this is a bit more complicated after I spotted another opencoding of 
NODEMASK_OF().

diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c

index 00b64c3322..24af9bc471 100644

--- a/xen/arch/arm/smpboot.c

+++ b/xen/arch/arm/smpboot.c

@@ -46,7 +46,7 @@ struct cpuinfo_arm cpu_data[NR_CPUS];

 register_t __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = MPIDR_INVALID };

 

 /* Fake one node for now. See also include/asm-arm/numa.h */

-nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };

+nodemask_t __read_mostly node_online_map = NODEMASK_OF(0);

 

 /* Xen stack for bringing up the first CPU. */

 static unsigned char __initdata cpu0_boot_stack[STACK_SIZE]

diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c

index 7473f83b7b..9a55c013e5 100644

--- a/xen/arch/x86/numa.c

+++ b/xen/arch/x86/numa.c

@@ -47,7 +47,7 @@ nodeid_t apicid_to_node[MAX_LOCAL_APIC] = {

 };

 cpumask_t node_to_cpumask[MAX_NUMNODES] __read_mostly;

 

-nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };

+nodemask_t __read_mostly node_online_map = NODEMASK_OF(0);

 

 bool numa_off;

 s8 acpi_numa = 0;

diff --git a/xen/include/xen/nodemask.h b/xen/include/xen/nodemask.h

index 9933fec5c4..c474dca3f0 100644

--- a/xen/include/xen/nodemask.h

+++ b/xen/include/xen/nodemask.h

@@ -86,11 +86,9 @@ typedef struct { DECLARE_BITMAP(bits, MAX_NUMNODES); } 
nodemask_t;

 }})

 

 #define NODEMASK_OF(node)   \

-({  \

-    nodemask_t m = NODES_NONE;  \

-    m.bits[(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG);   \

-    m;  \

-})

+((nodemask_t) {{    \

+    [(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG)  \

+}})

 

 #endif /* MAX_NUMNODES */

 

and to be used as a static initialiser, NODEMASK_OF() needs to be an ICE and 
can't use ({}) .

I don't see a way to avoid expanding node twice, but given that its wrapper is 
in ALL_CAPS and obviously a macro.

Furthermore, experimenting with a deliberate attempt to provoke this, I got 

numa.c: In function ‘numa_initmem_init’:

/local/xen.git/xen/include/xen/nodemask.h:90:10: error: nonconstant array index 
in initializer

 [(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG)  \

  ^

numa.c:274:23: note: in expansion of macro ‘NODEMASK_OF’

 node_online_map = NODEMASK_OF(foo++);

   ^~~

/local/xen.git/xen/include/xen/nodemask.h:90:10: note: (near initialization for 
‘(anonymous).bits’)

 [(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG)  \

Re: [Xen-devel] [PATCH v4 2/2] xen/arm: merge make_timer_node and make_timer_domU_node

On Wed, Jul 31, 2019 at 3:33 PM Volodymyr Babchuk
 wrote:
>
>
>
> Viktor Mitin writes:
>
> > Merged make_timer_node and make_timer_domU_node into one function
> > make_timer_node.
> It is widely accepted to write commit messages in imperative mood,
> e.g. "merge" instead of "merged"
>
> > Kept the domU version for the compatible as it is simpler.
> > Kept the hw version for the clock as it is relevant for the both cases.
> ... or "keep" instead of "kept"

Well, again, there is no such rule in the coding style document.

> > Suggested-by: Julien Grall 
> > Signed-off-by: Viktor Mitin 
> > ---
> > v4 updates:
> >updated "Kept the domU version for the compatible as it is simpler"
> >
> >  xen/arch/arm/domain_build.c | 109 +---
> >  1 file changed, 39 insertions(+), 70 deletions(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index d04a1c3aec..4d7c3411a6 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -964,8 +964,12 @@ static int __init make_gic_node(const struct domain 
> > *d, void *fdt,
> >
> >  static int __init make_timer_node(const struct kernel_info *kinfo)
> >  {
> > +int res;
> >  void *fdt = kinfo->fdt;
> > -
> In the previous patch you added this empty string, now you are deleting
> it.

Why not? Do not remember why did it, probably it was more convenient
at that moment.
Anyway, why not?

>
> > +unsigned int irq[MAX_TIMER_PPI];
> MAX_TIMER_PPI equals to 4, but looks like you are using only first 3
> items of the array.

Yes. This is because MAX_TIMER_PPI has been defined, and this
particular example is taken from time.c

>
> > +gic_interrupt_t intrs[3];
> > +u32 clock_frequency;
> > +bool clock_valid;
> Do you really need to move those declarations?

Not really, it has appeared as a result of many code edit iterations.
As I mentioned previously, those patches are changed several times already,
so the final version has another order of the local variables. Why not?

> >  static const struct dt_device_match timer_ids[] __initconst =
> >  {
> >  DT_MATCH_COMPATIBLE("arm,armv7-timer"),
> > @@ -973,15 +977,6 @@ static int __init make_timer_node(const struct 
> > kernel_info *kinfo)
> >  { /* sentinel */ },
> >  };
> >  struct dt_device_node *dev;
> > -u32 len;
> > -const void *compatible;
> > -int res;
> > -unsigned int irq;
> > -gic_interrupt_t intrs[3];
> > -u32 clock_frequency;
> > -bool clock_valid;
> > -
> > -dt_dprintk("Create timer node\n");
> >
> >  dev = dt_find_matching_node(NULL, timer_ids);
> >  if ( !dev )
> > @@ -990,35 +985,49 @@ static int __init make_timer_node(const struct 
> > kernel_info *kinfo)
> >  return -FDT_ERR_XEN(ENOENT);
> >  }
> >
> > -compatible = dt_get_property(dev, "compatible", );
> > -if ( !compatible )
> > -{
> > -dprintk(XENLOG_ERR, "Can't find compatible property for timer 
> > node\n");
> > -return -FDT_ERR_XEN(ENOENT);
> > -}
> > -
> >  res = fdt_begin_node(fdt, "timer");
> >  if ( res )
> >  return res;
> >
> > -res = fdt_property(fdt, "compatible", compatible, len);
> > -if ( res )
> > -return res;
> > +if ( !is_64bit_domain(kinfo->d) )
> > +{
> > +res = fdt_property_string(fdt, "compatible", "arm,armv7-timer");
> > +if ( res )
> > +return res;
> > +}
> > +else
> > +{
> > +res = fdt_property_string(fdt, "compatible", "arm,armv8-timer");
> > +if ( res )
> > +return res;
> > +}
> So, previously this code copied "compatible" property from platform
> device tree. Please note, that theoretically it would be neither
> "arm,armv8-timer" not "arm,armv7-timer". Now you are setting one of the
> two values. I'm not sure if this is right thing to do in the first
> place. Probably we need comment from Julien. But this change should be
> reflected in the commit message.

Well, it is done, because Julien preferred domU variant as more simple one.
Actually I have checked that both variats works well, but kept domU case.

It is in the commit message:
"Kept the domU version for the compatible as it is simpler."
>
>
> >  /* The timer IRQ is emulated by Xen. It always exposes an active-low
> >   * level-sensitive interrupt */
> I'm not demanding this, but you can fix this comment in the next
> version. It does not conforms to the coding style. Also, it is partially
> misplaced now.

The format of this comment has not been changed by me.
Why do you think that it is misplaced now?

> > +if ( is_hardware_domain(kinfo->d) )
> > +{
> > +irq[TIMER_PHYS_SECURE_PPI] = timer_get_irq(TIMER_PHYS_SECURE_PPI);
> > +irq[TIMER_PHYS_NONSECURE_PPI] =
> > +
> > timer_get_irq(TIMER_PHYS_NONSECURE_PPI);
> > +irq[TIMER_VIRT_PPI] = timer_get_irq(TIMER_VIRT_PPI);
> > +

Re: [Xen-devel] [PATCH v4 2/2] xen/arm: merge make_timer_node and make_timer_domU_node



Viktor Mitin writes:

> Merged make_timer_node and make_timer_domU_node into one function
> make_timer_node.
It is widely accepted to write commit messages in imperative mood,
e.g. "merge" instead of "merged"

> Kept the domU version for the compatible as it is simpler.
> Kept the hw version for the clock as it is relevant for the both cases.
... or "keep" instead of "kept"

> Suggested-by: Julien Grall 
> Signed-off-by: Viktor Mitin 
> ---
> v4 updates:
>updated "Kept the domU version for the compatible as it is simpler"
>
>  xen/arch/arm/domain_build.c | 109 +---
>  1 file changed, 39 insertions(+), 70 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index d04a1c3aec..4d7c3411a6 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -964,8 +964,12 @@ static int __init make_gic_node(const struct domain *d, 
> void *fdt,
>
>  static int __init make_timer_node(const struct kernel_info *kinfo)
>  {
> +int res;
>  void *fdt = kinfo->fdt;
> -
In the previous patch you added this empty string, now you are deleting
it.

> +unsigned int irq[MAX_TIMER_PPI];
MAX_TIMER_PPI equals to 4, but looks like you are using only first 3
items of the array.

> +gic_interrupt_t intrs[3];
> +u32 clock_frequency;
> +bool clock_valid;
Do you really need to move those declarations?

>  static const struct dt_device_match timer_ids[] __initconst =
>  {
>  DT_MATCH_COMPATIBLE("arm,armv7-timer"),
> @@ -973,15 +977,6 @@ static int __init make_timer_node(const struct 
> kernel_info *kinfo)
>  { /* sentinel */ },
>  };
>  struct dt_device_node *dev;
> -u32 len;
> -const void *compatible;
> -int res;
> -unsigned int irq;
> -gic_interrupt_t intrs[3];
> -u32 clock_frequency;
> -bool clock_valid;
> -
> -dt_dprintk("Create timer node\n");
>
>  dev = dt_find_matching_node(NULL, timer_ids);
>  if ( !dev )
> @@ -990,35 +985,49 @@ static int __init make_timer_node(const struct 
> kernel_info *kinfo)
>  return -FDT_ERR_XEN(ENOENT);
>  }
>
> -compatible = dt_get_property(dev, "compatible", );
> -if ( !compatible )
> -{
> -dprintk(XENLOG_ERR, "Can't find compatible property for timer 
> node\n");
> -return -FDT_ERR_XEN(ENOENT);
> -}
> -
>  res = fdt_begin_node(fdt, "timer");
>  if ( res )
>  return res;
>
> -res = fdt_property(fdt, "compatible", compatible, len);
> -if ( res )
> -return res;
> +if ( !is_64bit_domain(kinfo->d) )
> +{
> +res = fdt_property_string(fdt, "compatible", "arm,armv7-timer");
> +if ( res )
> +return res;
> +}
> +else
> +{
> +res = fdt_property_string(fdt, "compatible", "arm,armv8-timer");
> +if ( res )
> +return res;
> +}
So, previously this code copied "compatible" property from platform
device tree. Please note, that theoretically it would be neither
"arm,armv8-timer" not "arm,armv7-timer". Now you are setting one of the
two values. I'm not sure if this is right thing to do in the first
place. Probably we need comment from Julien. But this change should be
reflected in the commit message.


>  /* The timer IRQ is emulated by Xen. It always exposes an active-low
>   * level-sensitive interrupt */
I'm not demanding this, but you can fix this comment in the next
version. It does not conforms to the coding style. Also, it is partially
misplaced now.

> +if ( is_hardware_domain(kinfo->d) )
> +{
> +irq[TIMER_PHYS_SECURE_PPI] = timer_get_irq(TIMER_PHYS_SECURE_PPI);
> +irq[TIMER_PHYS_NONSECURE_PPI] =
> +timer_get_irq(TIMER_PHYS_NONSECURE_PPI);
> +irq[TIMER_VIRT_PPI] = timer_get_irq(TIMER_VIRT_PPI);
> +}
> +else
> +{
> +irq[TIMER_PHYS_SECURE_PPI] = GUEST_TIMER_PHYS_S_PPI;
> +irq[TIMER_PHYS_NONSECURE_PPI] = GUEST_TIMER_PHYS_NS_PPI;
> +irq[TIMER_VIRT_PPI] = GUEST_TIMER_VIRT_PPI;
> +}
>
> -irq = timer_get_irq(TIMER_PHYS_SECURE_PPI);
> -dt_dprintk("  Secure interrupt %u\n", irq);
> -set_interrupt(intrs[0], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
> +dt_dprintk("  Secure interrupt %u\n", irq[TIMER_PHYS_SECURE_PPI]);
> +set_interrupt(intrs[0], irq[TIMER_PHYS_SECURE_PPI],
> + 0xf, DT_IRQ_TYPE_LEVEL_LOW);
Strange formatting. As I said earlier, 0xf should be aligned with intrs[0].

> -irq = timer_get_irq(TIMER_PHYS_NONSECURE_PPI);
> -dt_dprintk("  Non secure interrupt %u\n", irq);
> -set_interrupt(intrs[1], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
> +dt_dprintk("  Non secure interrupt %u\n", irq[TIMER_PHYS_NONSECURE_PPI]);
> +set_interrupt(intrs[1], irq[TIMER_PHYS_NONSECURE_PPI],
> + 0xf, DT_IRQ_TYPE_LEVEL_LOW);
The same about formatting.

>
> -irq = timer_get_irq(TIMER_VIRT_PPI);
> -

Re: [Xen-devel] [PATCH v2 0/2] Raspberry Pi 4 support

2019-07-31 Thread Andre Przywara

On Mon, 29 Jul 2019 09:19:18 -0400
Stewart Hildebrand  wrote:

Hi,

> This is a series to enable UART console for Raspberry Pi 4. Note that I'm 
> relying on the firmware to initialize the UART (i.e. enable_uart=1 in 
> config.txt), since full UART initialization on this platform requires 
> accessing some registers outside the range specified in the 
> brcm,bcm2835-aux-uart node.
> 
> I have been able to get Xen+dom0+domUs booting. Tested with Xen 4.12 and 
> 4.13-unstable (b4c8a27d5b)

Mmmh, did that really work for you? I needed the next commit in staging as well:
commit ead6b9f78355e8d366e0c80c4a73fa7fbd6d26cc
Author: Andrii Anisov 
Date:   Thu Jul 18 16:22:20 2019 +0300

xen/arm: cpuerrata: Align a virtual address before unmap

Otherwise Xen would crash before even considering Dom0.

With Andrii's patch, your two patches and Stefano's resmem series I was able to 
at least run Xen till it was looking for Dom0, from U-Boot, with ATF (providing 
PSCI).
There seems to be some hiccup in the reserved-memory code in U-Boot, where 
U-Boot tries to use the already region, but that's an independent matter.

Cheers,
Andre.

> and Linux 4.19.y (Raspberry Pi linux tree + a couple of patches). Please see 
> [1] for build instructions and limitations.
> 
> New in v2:
> * Drop early printk alias
> * Set reg-shift and reg-io-width in the Xen driver
> * Blacklist other aux peripherals in platform settings (spi1, spi2, and a 
> couple of base aux registers)
> 
> Thanks,
> Stewart Hildebrand
> DornerWorks, Ltd
> 
> [1] https://github.com/dornerworks/xen-rpi4-builder
> 
> Stewart Hildebrand (2):
>   ns16550: Add compatible string for Raspberry Pi 4
>   xen/arm: platform: Add Raspberry Pi platform
> 
>  xen/arch/arm/platforms/Makefile|  1 +
>  xen/arch/arm/platforms/brcm-raspberry-pi.c | 55 ++
>  xen/drivers/char/ns16550.c |  7 +++
>  3 files changed, 63 insertions(+)
>  create mode 100644 xen/arch/arm/platforms/brcm-raspberry-pi.c
> 


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

Re: [Xen-devel] [PATCH v4 1/2] xen/arm: extend fdt_property_interrupts

Hi Volodymyr,

On Wed, Jul 31, 2019 at 3:11 PM Volodymyr Babchuk
 wrote:
>
>
> Hi Viktor,
>
> It is recommended (and probably required, but I can't find exact place
> in the rules) to include cover letter if you are sending more that one
> patch in series. This will ease up review process, because reviewer will
> know what to expect in the series.

There is no such requirement, only recommendation.
I did not put it since this is simple short patch series and both
patches in this series have been discussed previously, so it is known
what it is about.

> Viktor Mitin writes:
>
> > Extend fdt_property_interrupts to deal with other domain than the hwdom.
> >
> > The prototype of fdt_property_interrupts() has been modified
> > to support both hwdom and domU in one function.
> >
> > This is a preparatory for the patch "xen/arm: merge make_timer_node and
> > make_timer_domU_node". Original goal is to merge make_timer_node and
> > make_timer_domU_node functions. See discussion in e-mail, the Message-ID is:
> > <20190620103805.927-1-viktor.mitin...@gmail.com>
> >
> > Note: there is no functional changes introduced by this patch.
> This is not completely true, because you change the way how phandle is
> retrieved. Also, earlier you said that "fdt_property_interrupts() has
> been modified to support both hwdom and domU in one function". This is
> the functional change.

Phandle is retreved the same way:
in case of  hwdom it is dt_interrupt_controller->phandle
in case of domU it is GUEST_PHANDLE_GIC.
Don't see any functional change here.

What is 'functional change' in your opinion?

>
> >
> > Suggested-by: Julien Grall 
> > Signed-off-by: Viktor Mitin 
> > ---
> >  xen/arch/arm/domain_build.c | 22 +-
> >  1 file changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 4c8404155a..d04a1c3aec 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -621,17 +621,19 @@ static void __init set_interrupt(gic_interrupt_t 
> > interrupt,
> >   *  "interrupts": contains the list of interrupts
> >   *  "interrupt-parent": link to the GIC
> >   */
> > -static int __init fdt_property_interrupts(void *fdt, gic_interrupt_t *intr,
> > -  unsigned num_irq)
> > +static int __init fdt_property_interrupts(const struct kernel_info *kinfo,
> > +gic_interrupt_t *intr, unsigned num_irq)
> As I said earlier, this formatting contradicts with the coding style.

There is no such coding style requirement (not explicitly documented).
Even more, the original code formatted the same way.

> >  {
> >  int res;
> > +uint32_t phandle = is_hardware_domain(kinfo->d) ?
> > +   dt_interrupt_controller->phandle : 
> > GUEST_PHANDLE_GIC;
> >
> > -res = fdt_property(fdt, "interrupts", intr, sizeof (intr[0]) * 
> > num_irq);
> > +res = fdt_property(kinfo->fdt, "interrupts",
> > +   intr, sizeof (intr[0]) * num_irq);
> >  if ( res )
> >  return res;
> >
> > -res = fdt_property_cell(fdt, "interrupt-parent",
> > -dt_interrupt_controller->phandle);
> > +res = fdt_property_cell(kinfo->fdt, "interrupt-parent", phandle);
> >
> >  return res;
> >  }
> > @@ -733,7 +735,7 @@ static int __init make_hypervisor_node(struct domain *d,
> >   *  TODO: Handle properly the cpumask;
> >   */
> >  set_interrupt(intr, d->arch.evtchn_irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
> > -res = fdt_property_interrupts(fdt, , 1);
> > +res = fdt_property_interrupts(kinfo, , 1);
> >  if ( res )
> >  return res;
> >
> > @@ -960,8 +962,10 @@ static int __init make_gic_node(const struct domain 
> > *d, void *fdt,
> >  return res;
> >  }
> >
> > -static int __init make_timer_node(const struct domain *d, void *fdt)
> > +static int __init make_timer_node(const struct kernel_info *kinfo)
> >  {
> > +void *fdt = kinfo->fdt;
> > +
> No need for empty line there.

Why not? Is there any reason or pointers to the documents?

Thanks

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

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

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-pvshim   18 guest-localmigrate/x10   fail REGR. vs. 139484

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-examine 11 examine-serial/bootloaderfail  like 139442
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 139484
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 139484
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 139484
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 139484
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 139484
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 139484
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 139484
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 139484
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 139484
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 xen  1585ed3c702e680ae492d852c8cff62cf300df99
baseline version:
 xen

Re: [Xen-devel] [PATCH 3/7] xen/arm: Rework psr_mode_is_32bit()


Hi Stefano,

On 29/07/2019 22:52, Stefano Stabellini wrote:

On Fri, 26 Jul 2019, Volodymyr Babchuk wrote:

Julien Grall writes:


Hi,

On 26/07/2019 13:31, Volodymyr Babchuk wrote:


Julien Grall writes:


psr_mode_is_32bit() prototype does not match the rest of the helpers for
the process state. Looking at the callers, most of them will access
struct cpu_user_regs just for calling psr_mode_is_32bit().

The macro is now reworked to take a struct cpu_user_regs in parameter.
At the same time take the opportunity to switch to a static inline
helper.

I'm a bit concerned about naming now. As psr_mode_is_32bit() is now have
no psr parameter, and ARM ARM uses term "state" instead of "mode", maybe
it is worth to rename this helper to something like "is_32bit_state"?


It really depends how you see it. The bit is part of the "mode" field,
so technically we are checking whether the mode corresponds to a
32-bit one or not. This is also inline with the rest of the helpers
within this header.

I would be willing to consider renaming the helper to regs_mode_is_32bit().

I'm fine with this name.


The patch is fine by me, as is, or with the name changed to
regs_mode_is_32bit. (I didn't commit it.)


I am thinking to get the renaming separately. So I can also rework the other 
helpers.




Either way:

Reviewed-by: Stefano Stabellini 


So I will commit as is and add in my todo list renaming.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v4 1/2] xen/arm: extend fdt_property_interrupts


Hi Viktor,

It is recommended (and probably required, but I can't find exact place
in the rules) to include cover letter if you are sending more that one
patch in series. This will ease up review process, because reviewer will
know what to expect in the series.

Viktor Mitin writes:

> Extend fdt_property_interrupts to deal with other domain than the hwdom.
>
> The prototype of fdt_property_interrupts() has been modified
> to support both hwdom and domU in one function.
>
> This is a preparatory for the patch "xen/arm: merge make_timer_node and
> make_timer_domU_node". Original goal is to merge make_timer_node and
> make_timer_domU_node functions. See discussion in e-mail, the Message-ID is:
> <20190620103805.927-1-viktor.mitin...@gmail.com>
>
> Note: there is no functional changes introduced by this patch.
This is not completely true, because you change the way how phandle is
retrieved. Also, earlier you said that "fdt_property_interrupts() has
been modified to support both hwdom and domU in one function". This is
the functional change.

>
> Suggested-by: Julien Grall 
> Signed-off-by: Viktor Mitin 
> ---
>  xen/arch/arm/domain_build.c | 22 +-
>  1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 4c8404155a..d04a1c3aec 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -621,17 +621,19 @@ static void __init set_interrupt(gic_interrupt_t 
> interrupt,
>   *  "interrupts": contains the list of interrupts
>   *  "interrupt-parent": link to the GIC
>   */
> -static int __init fdt_property_interrupts(void *fdt, gic_interrupt_t *intr,
> -  unsigned num_irq)
> +static int __init fdt_property_interrupts(const struct kernel_info *kinfo,
> +gic_interrupt_t *intr, unsigned num_irq)
As I said earlier, this formatting contradicts with the coding style.

>  {
>  int res;
> +uint32_t phandle = is_hardware_domain(kinfo->d) ?
> +   dt_interrupt_controller->phandle : GUEST_PHANDLE_GIC;
>
> -res = fdt_property(fdt, "interrupts", intr, sizeof (intr[0]) * num_irq);
> +res = fdt_property(kinfo->fdt, "interrupts",
> +   intr, sizeof (intr[0]) * num_irq);
>  if ( res )
>  return res;
>
> -res = fdt_property_cell(fdt, "interrupt-parent",
> -dt_interrupt_controller->phandle);
> +res = fdt_property_cell(kinfo->fdt, "interrupt-parent", phandle);
>
>  return res;
>  }
> @@ -733,7 +735,7 @@ static int __init make_hypervisor_node(struct domain *d,
>   *  TODO: Handle properly the cpumask;
>   */
>  set_interrupt(intr, d->arch.evtchn_irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
> -res = fdt_property_interrupts(fdt, , 1);
> +res = fdt_property_interrupts(kinfo, , 1);
>  if ( res )
>  return res;
>
> @@ -960,8 +962,10 @@ static int __init make_gic_node(const struct domain *d, 
> void *fdt,
>  return res;
>  }
>
> -static int __init make_timer_node(const struct domain *d, void *fdt)
> +static int __init make_timer_node(const struct kernel_info *kinfo)
>  {
> +void *fdt = kinfo->fdt;
> +
No need for empty line there.

>  static const struct dt_device_match timer_ids[] __initconst =
>  {
>  DT_MATCH_COMPATIBLE("arm,armv7-timer"),
> @@ -1016,7 +1020,7 @@ static int __init make_timer_node(const struct domain 
> *d, void *fdt)
>  dt_dprintk("  Virt interrupt %u\n", irq);
>  set_interrupt(intrs[2], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
>
> -res = fdt_property_interrupts(fdt, intrs, 3);
> +res = fdt_property_interrupts(kinfo, intrs, 3);
>  if ( res )
>  return res;
>
> @@ -1377,7 +1381,7 @@ static int __init handle_node(struct domain *d, struct 
> kernel_info *kinfo,
>  if ( device_get_class(node) == DEVICE_GIC )
>  return make_gic_node(d, kinfo->fdt, node);
>  if ( dt_match_node(timer_matches, node) )
> -return make_timer_node(d, kinfo->fdt);
> +return make_timer_node(kinfo);
>
>  /* Skip nodes used by Xen */
>  if ( dt_device_used_by(node) == DOMID_XEN )


--
Volodymyr Babchuk at EPAM
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4] xen/doc: Improve Dom0-less documentation


Hi Viktor,

On 31/07/2019 09:57, Viktor Mitin wrote:

On Wed, Jul 31, 2019 at 11:40 AM Julien Grall  wrote:

I can switch the memory property back to hexadecimal on commit. But I
would like to understand why the value has changed before doing that.


Ok, let's keep hexadecimal.
128Mb is ok in this example, I use 512Mb for my tests,
so while testing this example I changed it and didn't restore.

This should be left untouched, as you mentioned.
memory = <0x0 0x2>;


Acked-by: Julien Grall 

I will commit it soon. Thank you for the patch.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 0/2] Raspberry Pi 4 support


Hi Stewart,

On 29/07/2019 14:19, Stewart Hildebrand wrote:

This is a series to enable UART console for Raspberry Pi 4. Note that I'm 
relying on the firmware to initialize the UART (i.e. enable_uart=1 in 
config.txt), since full UART initialization on this platform requires accessing 
some registers outside the range specified in the brcm,bcm2835-aux-uart node.

I have been able to get Xen+dom0+domUs booting. Tested with Xen 4.12 and 
4.13-unstable (b4c8a27d5b) and Linux 4.19.y (Raspberry Pi linux tree + a couple 
of patches). Please see [1] for build instructions and limitations.

New in v2:
* Drop early printk alias
* Set reg-shift and reg-io-width in the Xen driver
* Blacklist other aux peripherals in platform settings (spi1, spi2, and a 
couple of base aux registers)

Thanks,
Stewart Hildebrand
DornerWorks, Ltd

[1] https://github.com/dornerworks/xen-rpi4-builder

Stewart Hildebrand (2):
   ns16550: Add compatible string for Raspberry Pi 4


I have committed this patch...


   xen/arm: platform: Add Raspberry Pi platform

... this one need an answer regarding the impact on blacklist spi1 and spi2.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 2/2] xen/arm: platform: Add Raspberry Pi platform


Hi Stewart,

On 29/07/2019 14:19, Stewart Hildebrand wrote:

The aux peripherals (uart1, spi1, and spi2) share an IRQ and a page of
memory. For debugging, it is helpful to use the aux UART in Xen. In
this case, Xen would try to assign spi1 and spi2 to dom0, but this
results in an error since the shared IRQ was already assigned to Xen.
Blacklist aux devices other than the UART to prevent mapping the shared
IRQ and memory range to dom0.


Reading the commit message, it is unclear what's the impact on blacklist spi1 
and spi2. Could you expand it?


The rest of the patch looks good.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 1/2] ns16550: Add compatible string for Raspberry Pi 4

Hi,

On 29/07/2019 17:06, Andre Przywara wrote:

On Mon, 29 Jul 2019 09:19:19 -0400
Stewart Hildebrand wrote:

Hi,

Per the BCM2835 peripherals datasheet [1] page 10:
"The UART core is build to emulate 16550 behaviour ... The implemented
UART is not a 16650 compatible UART However as far as possible the
first 8 control and status registers are laid out like a 16550 UART. Al
16550 register bits which are not supported can be written but will be
ignored and read back as 0. All control bits for simple UART receive/
transmit operations are available."

Additionally, Linux uses the 8250/16550 driver for the aux UART [2].

Unfortunately the brcm,bcm2835-aux-uart device tree binding doesn't
have the reg-shift and reg-io-width properties [3]. Thus, the reg-shift
and reg-io-width properties are inherent properties of this UART.

Thanks to Andre Przywara for contributing the reg-shift and
reg-io-width setting snippet.

In my testing, I have relied on enable_uart=1 being set in config.txt,
a configuration file read by the Raspberry Pi's firmware. With
enable_uart=1, the firmware performs UART initialization.

[1]
https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2835/BCM2835-ARM-Peripherals.pdf
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/tty/serial/8250/8250_bcm2835aux.c
[3]
https://www.kernel.org/doc/Documentation/devicetree/bindings/serial/brcm,bcm2835-aux-uart.txt

Signed-off-by: Stewart Hildebrand

Reviewed-by: Andre Przywara
Tested-by: Andre Przywara

Acked-by: Julien Grall

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] Intel TXT: add reviewer, move to Odd Fixes state


Hi Lars,

On 30/07/2019 12:22, Lars Kurth wrote:



On 30 Jul 2019, at 11:08, George Dunlap > wrote:


On 7/30/19 10:54 AM, Julien Grall wrote:

Hi Jan,

On 30/07/2019 10:05, Jan Beulich wrote:

On 30.07.2019 10:54, Julien Grall wrote:

On 7/30/19 9:29 AM, Jan Beulich wrote:

On 30.07.2019 08:56, Lukasz Hawrylko wrote:


+full committers list and Juergen

OK. We should have a separate discussion related to disclaimers: make a formal 
decision and afterwards document it in the contribution workflow. I agree that 
this makes sense, and this has been raised by Julien in the past privately 
related to questions on xen-devel@. It then turned out that Arm folks from China 
have consistently used disclaimers on contributions to mini-os and unikraft. 
This has stopped now, which is to Julien's credit. I suggested than that Julien 
should raise this issue formally as a policy change, which never happened.


I do not believe that we should block any patches from being applied due to 
disclaimers in absence of an agreed policy. Contributors sign a DCO and that may 
well override a disclaimer (we do not have access to the legal advice that Greg 
KH refers to). If there was a serious legal issue, the LF would have contacted 
all of its projects. And I also could not find any public reference to such an 
issue. This definitely something where the Advisory Board should have some input.


And in particular this patch also contains no code and should not be blocked on 
these grounds.


I originally objected on this patch because the disclaimer issue was pointed out 
3 versions ago and still not addressed. This then went on the discussion with 
Jan about the disclaimer.


While reviewer only means you are CC to e-mails, I would at least expect them to 
understand the process and be able to address comments.




@Lukasz: please take note of this issue for the next time round. It should be 
easy enough to disable the disclaimer when sending to certain lists


It is not easy enough as you may think ;). At Arm we have to go through a 
different SMTP server so we bypass exchange.




To move forward:
* There should be a policy discussion


How should I raise it? Do you want a patch again contribution-guidelines?


* There should be AB input
* The outcome should be documented in 
https://xenproject.org/help/contribution-guidelines/ and the git contribution 
workflow


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC] XCP-ng subproject proposal

2019-07-31 Thread Wei Liu

+1 from me.

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

Re: [Xen-devel] Xen Coding style and clang-format

On Wed, Jul 31, 2019 at 2:25 PM Julien Grall  wrote:
>
>
>
> On 31/07/2019 12:16, Viktor Mitin wrote:
> > On Mon, Jul 29, 2019 at 3:35 PM Julien Grall  wrote:
> >> On 7/29/19 1:21 PM, Viktor Mitin wrote:
> >>> On Mon, Jul 29, 2019 at 1:49 PM Julien Grall  wrote:
>  On 7/29/19 10:13 AM, Viktor Mitin wrote:
> > On Fri, Jul 26, 2019 at 3:50 PM Julien Grall  
> > wrote:
> >>
> >> *
> >>
> >> -/* See linux
> >> Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt */
> >> +/* See linux
> >> + * 
> >> Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt */
> >>
> >> Multi-lines comment on Xen are using
> >> /*
> >>  * Foo
> >>  * Bar
> >>  */
> >
> > See my comment about clang-format supports only comments indentation 
> > for now.
> 
>  I saw it and I will reply here for simplicity. Having a automatic
>  checker that will do the wrong things is not ideal.
> 
>  Imagine we decide to use this checker as a part of the commit process.
>  This means that the code will be modified to checker coding style and
>  not Xen one.
> >>>
> >>> Well, you are right. Even more, unfortunately, there is no such coding
> >>> style as 'bsd' in clang-format.
> >>> So 'xen' clang-format style is based on the 'llvm' style.
> >>
> >> Do you have a pointer to the LLVM style?
> >
> > Sure, see the next links:
> > https://github.com/viktor-mitin/xen-clang-format-example/blob/master/.clang-format_llvm
> > https://github.com/viktor-mitin/xen-clang-format-example/blob/master/.clang-format_xen
>
> That's clang-format configuration not a write-up easily readable by human. It 
> is
> also does not say what will happen for the rest of the things not configured 
> (if
> there are any).

Here is Clang-Format Style Options documentation:
https://clang.llvm.org/docs/ClangFormatStyleOptions.html

And LLVM Coding Standards documetation:
 https://llvm.org/docs/CodingStandards.html#introduction

Unfortunately, it seems it does not answer "what will happen for the
rest of the things not configured (if there are any)"...


>
> >
> >>
> >> As I pointed out in a different thread, it woudl be easier to start from
> >> an existing coding style (LLVM, BSD...) and decide whether you want to
> >> fully adopt it or make changes.
> >>
> >> So someone needs to be pick one and look at the difference in style with
> >> Xen. It seems you already done that job as you tweak it for Xen. Do you
> >> have a write-up of the differences?
> >
> > Yes, it is done exactly this way you mentioned. New 'xen' format style
> > is based on 'llvm'.
>
> Can you give a link to this write-up in a human readable way?

The summary I wrote in the original mail in this thread describes what
was done based on 'llvm' coding style of clang-format.
I'm putting it here with update: added BreakStringLiterals thing to be fixed.

Summary of the changes:
- Added 3 new formatting styles to cover all the cases mentioned in
Xen coding style document: Xen, Libxl, Linux;
- Added list of the files and corresponding style name mappings;
- Added indentation according to Xen coding style;
- Added white space formatting according to Xen coding style;
- Added bracing support exception for do/while loops;

Added to clang-format, however, probably this logic should be moved to
python part (see known clang-format limitations above):
- Braces should be omitted for blocks with a single statement. Note:
these braces will be required by MISRA, for example, so it is probably
worth adding such a requirement to the coding style.
- Comments format requirements. Note: //-style comments are defined in
C99 as well, and not just in the case of C++. C99 standard is 20-years
old…

To be added:
- Emacs local variables. Open points: Why to keep emacs local
variables in Xen code? What about other editors' comments (vim)?
- Warning to stderr in the case when ‘unfixable’ line/s detected.

To be fixed:
- Max line length from 80 to 79 chars;
- Disable // comments;
- Set BreakStringLiterals to false (not explicitly covered by Xen
coding style document for now)

>
> >
> >>
> >> I am not sure why clang-format decided to format like that. Do you 
> >> know why?
> >
> > The reason is that there are two strings in one line. It would not
> > change it if it were
> > not "arm,psci-1.0""\0", but "arm,psci-1.0\0".
> 
>  I would like to see the exact part of the clang-format coding style
>  documentation that mention this requirements... The more that in an
>  example above (copied below for simplicity), there are two strings in on
>  line.
> >>>
> >>> The closest found seems BinPackParameters BinPackArguments,
> >>> however, it is about function calls according to manual...
> >>
> >> Above, you mention the work is based on the LLVM coding style. Is there
> >> anything in that coding style about the string?
> >
> > Well, not much. See

Re: [Xen-devel] [RFC 5/6] arm64: call enter_hypervisor_head only when it is needed

2019-07-31 Thread Andre Przywara

On Wed, 31 Jul 2019 12:02:20 +0100
Julien Grall  wrote:

Hi,

> On 30/07/2019 18:35, Andrii Anisov wrote:
> > 
> > On 26.07.19 13:59, Julien Grall wrote:  
> >> Hi,
> >>
> >> On 26/07/2019 11:37, Andrii Anisov wrote:  
> >>> From: Andrii Anisov 
> >>>
> >>> On ARM64 we know exactly if trap happened from hypervisor or guest, so
> >>> we do not need to take that decision. This reduces a condition for
> >>> all enter_hypervisor_head calls and the function call for traps from
> >>> the hypervisor mode.  
> >>
> >> One condition lost but ...  
> > 
> > ...In the hot path (actually at any trap).  
> 
> Everything is in the hot path here, yet there are a lot of other branches. So 
> why this branch in particular?
> 
> As I have mentioned a few times before, there are a difference between the 
> theory and the practice. In theory, removing a branch looks nice. But in 
> practice this may not be the case.
> 
> In this particular case, I don't believe this is going to have a real impact 
> on 
> the performance.
> 
> The PSTATE has been saved a few instructions before in cpu_user_regs, so there
> are high chance the value will still be in the L1 cache.

I agree on this, and second the idea of *not* micro-optimising code just for 
the sake of it. If you have numbers that back this up, it would be a different 
story.

> The compiler may also decide to do the direct branch when not in guest_mode. 
> A 
> trap from the hypervisor is mostly for interrupts. So there are chance this 
> is 
> not going to have a real impact on the overall of the interrupt handling.
> 
> If you are really worry of the impact of branch then there are a few more 
> important places (with a greater benefits) to look:
>  1) It seems the compiler use a jump table for the switch case in 
> do_trap_guest_sync(), so it will result to multiple direct branch everytime.

I don't think it's worth to "fix" this issue. The compiler has done this for a 
reason, and I would guess it figured that this is cheaper than other ways of 
solving this. If you are really paranoid about this, I would try to compile 
this with tuning for your particular core (-mtune), so that the compiler can 
throw in more micro-architectural knowledge about the cost of certain 
instructions.

>  2) Indirect branch have a non-negligible cost compare to direct branch. 
> This is a lot used in the interrupt code (see gic_hw_ops->read_irq()). All of 
> them are known at boot time, so they could be replace with direct branch. x86 
> recently introduced alternative_call() for this purpose. This could be 
> re-used 
> on Arm.

This is indeed something I was always worried about: It looks cheap and elegant 
in the C source code, but is potentially expensive on hardware. The particular 
snippet is:
...
  249024:   d5033fdfisb
  249028:   f9401e80ldr x0, [x20, #56]
  24902c:   f9407801ldr x1, [x0, #240]
  249030:   2a1303e0mov w0, w19
  249034:   d63f0020blr x1
...
In case of an interrupt, the first load will probably miss the cache, and the 
CPU is stuck now, because due to the dependencies it can't do much else. It 
can't even predict the branch and speculatively execute anything, because the 
destination address is yet another dependent load away.
This might not matter for little cores like A53s, because they wouldn't 
speculate anyway. But better cores (A72, for instance) would most likely 
benefit from an optimisation in this area.

Cheers,
Andre.

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

Re: [Xen-devel] [PATCH v2] x86/HVM: correct hvmemul_map_linear_addr() for multi-page case

2019-07-31 Thread Alexandru Stefan ISAILA



On 13.09.2018 13:12, Jan Beulich wrote:
> The function does two translations in one go for a single guest access.
> Any failure of the first translation step (guest linear -> guest
> physical), resulting in #PF, ought to take precedence over any failure
> of the second step (guest physical -> host physical). Bail out of the
> loop early solely when translation produces HVMTRANS_bad_linear_to_gfn,
> and record the most relevant of perhaps multiple different errors
> otherwise. (The choice of ZERO_BLOCK_PTR as sentinel is arbitrary.)
> 
> Signed-off-by: Jan Beulich 

This is useful for adding new functionality to hvmemul_map_linear_addr()

Reviewed-by: Alexandru Isaila 

> ---
> v2: Add comment (mapping table) and adjust update_map_err()
>  accordingly.
> 
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -532,6 +532,36 @@ static int hvmemul_do_mmio_addr(paddr_t
>   }
>   
>   /*
> + * Intended mapping, implemented without table lookup:
> + *
> + * -
> + * | \ new |   |   |   |   |
> + * |   \   | OKAY  | NULL  | RETRY | UNHND |
> + * | err \ |   |   |   |   |
> + * -
> + * | OKAY  | OKAY  | NULL  | RETRY | UNHND |
> + * -
> + * | NULL  | NULL  | NULL  | RETRY | UNHND |
> + * -
> + * | RETRY | RETRY | RETRY | RETRY | UNHND |
> + * -
> + * | UNHND | UNHND | UNHND | UNHND | UNHND |
> + * -
> + */
> +static void *update_map_err(void *err, void *new)
> +{
> +if ( err == ZERO_BLOCK_PTR || err == ERR_PTR(~X86EMUL_OKAY) ||
> + new == ERR_PTR(~X86EMUL_UNHANDLEABLE) )
> +return new;
> +
> +if ( new == ERR_PTR(~X86EMUL_OKAY) ||
> + err == ERR_PTR(~X86EMUL_UNHANDLEABLE) )
> +return err;
> +
> +return err ?: new;
> +}
> +
> +/*
>* Map the frame(s) covering an individual linear access, for writeable
>* access.  May return NULL for MMIO, or ERR_PTR(~X86EMUL_*) for other 
> errors
>* including ERR_PTR(~X86EMUL_OKAY) for write-discard mappings.
> @@ -544,7 +574,7 @@ static void *hvmemul_map_linear_addr(
>   struct hvm_emulate_ctxt *hvmemul_ctxt)
>   {
>   struct vcpu *curr = current;
> -void *err, *mapping;
> +void *err = ZERO_BLOCK_PTR, *mapping;
>   unsigned int nr_frames = ((linear + bytes - !!bytes) >> PAGE_SHIFT) -
>   (linear >> PAGE_SHIFT) + 1;
>   unsigned int i;
> @@ -600,27 +630,28 @@ static void *hvmemul_map_linear_addr(
>   goto out;
>   
>   case HVMTRANS_bad_gfn_to_mfn:
> -err = NULL;
> -goto out;
> +err = update_map_err(err, NULL);
> +continue;
>   
>   case HVMTRANS_gfn_paged_out:
>   case HVMTRANS_gfn_shared:
> -err = ERR_PTR(~X86EMUL_RETRY);
> -goto out;
> +err = update_map_err(err, ERR_PTR(~X86EMUL_RETRY));
> +continue;
>   
>   default:
> -goto unhandleable;
> +err = update_map_err(err, ERR_PTR(~X86EMUL_UNHANDLEABLE));
> +continue;
>   }
>   
>   *mfn++ = page_to_mfn(page);
>   
>   if ( p2m_is_discard_write(p2mt) )
> -{
> -err = ERR_PTR(~X86EMUL_OKAY);
> -goto out;
> -}
> +err = update_map_err(err, ERR_PTR(~X86EMUL_OKAY));
>   }
>   
> +if ( err != ZERO_BLOCK_PTR )
> +goto out;
> +
>   /* Entire access within a single frame? */
>   if ( nr_frames == 1 )
>   mapping = map_domain_page(hvmemul_ctxt->mfn[0]);
> @@ -639,6 +670,7 @@ static void *hvmemul_map_linear_addr(
>   return mapping + (linear & ~PAGE_MASK);
>   
>unhandleable:
> +ASSERT(err == ZERO_BLOCK_PTR);
>   err = ERR_PTR(~X86EMUL_UNHANDLEABLE);
>   
>out:
> 
> 
> 
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
> 
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen Coding style and clang-format




On 31/07/2019 12:16, Viktor Mitin wrote:

On Mon, Jul 29, 2019 at 3:35 PM Julien Grall  wrote:

On 7/29/19 1:21 PM, Viktor Mitin wrote:

On Mon, Jul 29, 2019 at 1:49 PM Julien Grall  wrote:

On 7/29/19 10:13 AM, Viktor Mitin wrote:

On Fri, Jul 26, 2019 at 3:50 PM Julien Grall  wrote:


*

-/* See linux
Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt */
+/* See linux
+ * Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt */

Multi-lines comment on Xen are using
/*
 * Foo
 * Bar
 */


See my comment about clang-format supports only comments indentation for now.


I saw it and I will reply here for simplicity. Having a automatic
checker that will do the wrong things is not ideal.

Imagine we decide to use this checker as a part of the commit process.
This means that the code will be modified to checker coding style and
not Xen one.


Well, you are right. Even more, unfortunately, there is no such coding
style as 'bsd' in clang-format.
So 'xen' clang-format style is based on the 'llvm' style.


Do you have a pointer to the LLVM style?


Sure, see the next links:
https://github.com/viktor-mitin/xen-clang-format-example/blob/master/.clang-format_llvm
https://github.com/viktor-mitin/xen-clang-format-example/blob/master/.clang-format_xen


That's clang-format configuration not a write-up easily readable by human. It is 
also does not say what will happen for the rest of the things not configured (if 
there are any).






As I pointed out in a different thread, it woudl be easier to start from
an existing coding style (LLVM, BSD...) and decide whether you want to
fully adopt it or make changes.

So someone needs to be pick one and look at the difference in style with
Xen. It seems you already done that job as you tweak it for Xen. Do you
have a write-up of the differences?


Yes, it is done exactly this way you mentioned. New 'xen' format style
is based on 'llvm'.


Can you give a link to this write-up in a human readable way?






I am not sure why clang-format decided to format like that. Do you know why?


The reason is that there are two strings in one line. It would not
change it if it were
not "arm,psci-1.0""\0", but "arm,psci-1.0\0".


I would like to see the exact part of the clang-format coding style
documentation that mention this requirements... The more that in an
example above (copied below for simplicity), there are two strings in on
line.


The closest found seems BinPackParameters BinPackArguments,
however, it is about function calls according to manual...


Above, you mention the work is based on the LLVM coding style. Is there
anything in that coding style about the string?


Well, not much. See clang-format configurator mentioned above.
However, there is a useful clang BreakStringLiterals option.
It should be turned off to follow your suggestion not to break string
literal for grep use case.


I am not speaking about clang-format itself but the LLVM coding style. I assume 
there is a human readable coding style for LLVM, right? If so, is there any 
section in it about string?










   >> +D11PRINT("Allocated %#" PRIpaddr "-%#" PRIpaddr






*

-clock_valid = dt_property_read_u32(dev, "clock-frequency",
-   _frequency);
+clock_valid =
+dt_property_read_u32(dev, "clock-frequency", _frequency);

I am not sure why clang-format decide to format like that. The current version
is definitely valid.


The current version is not valid as it takes 81 chars, while 79 is
allowed according to coding style.


Really? I looked at the code and this is 62 characters... It would be 81
characters if "_frequency);" were on the same line. But see how it
is split in 2 lines.


You are right, there are two lines. So it needs to find out how to
configure or implement such a feature to ignore such cases.


What's the LLVM coding style policy about this?


Well, LLVM formats it as shown above. All the other 'native'
clang-format styles behave similarly.


This does not answer to my question. You pointed me how clang-format is 
configured, not how the behavior of clang format for this particular case and 
the developer documentation related to this.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] Xen Coding style and clang-format

Hi Julien,

On Mon, Jul 29, 2019 at 3:35 PM Julien Grall  wrote:
>
> Hi Viktor,
>
> On 7/29/19 1:21 PM, Viktor Mitin wrote:
> > On Mon, Jul 29, 2019 at 1:49 PM Julien Grall  wrote:
> >> On 7/29/19 10:13 AM, Viktor Mitin wrote:
> >>> On Fri, Jul 26, 2019 at 3:50 PM Julien Grall  wrote:
> >>
> >>> It is unknown how difficult it would be to implement that with
> >>> clang-format, however, it can be analyzed.
> >>
> >> ...  but the goal of this discussion is to understand the limitations of
> >> Clang-format and whether a Coding Style change may be easier.
> >
> > It is not so easy to do, so it may take a time to investigate every the 
> > case.
>
> There must be a documentation for clang-format to explain the default
> coding style and way to tweak it, right? Could we get a pointer to it?

https://clang.llvm.org/docs/ClangFormat.html
Even more, there is 'clang-format configurator' which allows
experimenting with clang-format options online:
https://zed0.co.uk/clang-format-configurator/

> 
>  *
> 
>  -/* See linux
>  Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt */
>  +/* See linux
>  + * 
>  Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt */
> 
>  Multi-lines comment on Xen are using
>  /*
>  * Foo
>  * Bar
>  */
> >>>
> >>> See my comment about clang-format supports only comments indentation for 
> >>> now.
> >>
> >> I saw it and I will reply here for simplicity. Having a automatic
> >> checker that will do the wrong things is not ideal.
> >>
> >> Imagine we decide to use this checker as a part of the commit process.
> >> This means that the code will be modified to checker coding style and
> >> not Xen one.
> >
> > Well, you are right. Even more, unfortunately, there is no such coding
> > style as 'bsd' in clang-format.
> > So 'xen' clang-format style is based on the 'llvm' style.
>
> Do you have a pointer to the LLVM style?

Sure, see the next links:
https://github.com/viktor-mitin/xen-clang-format-example/blob/master/.clang-format_llvm
https://github.com/viktor-mitin/xen-clang-format-example/blob/master/.clang-format_xen

>
> As I pointed out in a different thread, it woudl be easier to start from
> an existing coding style (LLVM, BSD...) and decide whether you want to
> fully adopt it or make changes.
>
> So someone needs to be pick one and look at the difference in style with
> Xen. It seems you already done that job as you tweak it for Xen. Do you
> have a write-up of the differences?

Yes, it is done exactly this way you mentioned. New 'xen' format style
is based on 'llvm'.

>
>  I am not sure why clang-format decided to format like that. Do you know 
>  why?
> >>>
> >>> The reason is that there are two strings in one line. It would not
> >>> change it if it were
> >>> not "arm,psci-1.0""\0", but "arm,psci-1.0\0".
> >>
> >> I would like to see the exact part of the clang-format coding style
> >> documentation that mention this requirements... The more that in an
> >> example above (copied below for simplicity), there are two strings in on
> >> line.
> >
> > The closest found seems BinPackParameters BinPackArguments,
> > however, it is about function calls according to manual...
>
> Above, you mention the work is based on the LLVM coding style. Is there
> anything in that coding style about the string?

Well, not much. See clang-format configurator mentioned above.
However, there is a useful clang BreakStringLiterals option.
It should be turned off to follow your suggestion not to break string
literal for grep use case.

>
> >
> >>
> >>   >> +D11PRINT("Allocated %#" PRIpaddr "-%#" PRIpaddr
> >>
> >>
> >>>
> 
>  *
> 
>  -clock_valid = dt_property_read_u32(dev, "clock-frequency",
>  -   _frequency);
>  +clock_valid =
>  +dt_property_read_u32(dev, "clock-frequency", _frequency);
> 
>  I am not sure why clang-format decide to format like that. The current 
>  version
>  is definitely valid.
> >>>
> >>> The current version is not valid as it takes 81 chars, while 79 is
> >>> allowed according to coding style.
> >>
> >> Really? I looked at the code and this is 62 characters... It would be 81
> >> characters if "_frequency);" were on the same line. But see how it
> >> is split in 2 lines.
> >
> > You are right, there are two lines. So it needs to find out how to
> > configure or implement such a feature to ignore such cases.
>
> What's the LLVM coding style policy about this?

Well, LLVM formats it as shown above. All the other 'native'
clang-format styles behave similarly.

Thanks

>
> Cheers,
>
> --
> Julien Grall

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

[Xen-devel] XEN is not bringing

2019-07-31 Thread Raushan Kumar

Hi team,


Could you please suggest me why kernel gets stuck at

[..]

"

(XEN) *** Serial input to DOM0 (type 'CTRL-a' three times to switch input)
(XEN) Freed 336kB init memory.
"
[..]


Please find the below log :-
U-Boot 2015.04 (Jul 23 2019 - 12:58:16)

CPU: Renesas Electronics R8A7795 rev 2.0
Board: H3ULCB
I2C:   ready
DRAM:  3.9 GiB
MMC:   sh-sdhi: 0, sh-sdhi: 1
In:serial
Out:   serial
Err:   serial
Net:   Board Net Initialization Failed
No ethernet found.
Hit any key to stop autoboot:  0
=> ext2load mmc 0:2 0x4808 /boot/xen.uImage
951696 bytes read in 49 ms (18.5 MiB/s)
=> ext2load mmc 0:2 0x4800 /boot/xen.dtb
61568 bytes read in 11 ms (5.3 MiB/s)
=> ext2load mmc 0:2 0x7a00 /boot/Image
15782400 bytes read in 662 ms (22.7 MiB/s)
=> bootm 0x4808 - 0x4800
## Booting kernel from Legacy Image at 4808 ...
   Image Name:   XEN
   Image Type:   AArch64 Linux Kernel Image (uncompressed)
   Data Size:951632 Bytes = 929.3 KiB
   Load Address: 7808
   Entry Point:  7808
   Verifying Checksum ... OK
## Flattened Device Tree blob at 4800
   Booting using the fdt blob at 0x4800
   Loading Kernel Image ... OK
   Using Device Tree in place at 4800, end 4801207f

Starting kernel ...

- UART enabled -
- CPU  booting -
- Current EL 0008 -
- Xen starting at EL2 -
- Zero BSS -
- Setting up control registers -
- Turning on paging -
- Ready -
(XEN) Checking for initrd in /chosen
(XEN) RAM: 4800 - 7fff
(XEN) RAM: 0005 - 00053fff
(XEN) RAM: 0006 - 00063fff
(XEN) RAM: 0007 - 00073fff
(XEN)
(XEN) MODULE[0]: 4800 - 4801 Device Tree
(XEN) MODULE[1]: 7a00 - 7c00 Kernel
(XEN)  RESVD[0]: 4800 - 4801
(XEN)
(XEN)
(XEN) Command line: dom0_mem=752M console=dtuart dtuart=serial0 dom0_max_vcpus=4
(XEN) PFN compression on bits 19...19
(XEN) Domain heap initialised
(XEN) Booting using Device Tree
(XEN) Platform: Generic System
(XEN) Looking for dtuart at "serial0", options ""
 Xen 4.13-unstable
(XEN) Xen version 4.13-unstable (aarch64-poky-linux-gcc (Linaro GCC 
5.2-2015.11-2) 5.2.1 20151005) debug=y  Tue Jul 30 12:23:51 IST 209
(XEN) Latest ChangeSet: Fri Jul 19 13:51:24 2019 +0200 git:66d11b9-dirty
(XEN) build-id: ed9351ffb11d753817e362e4bd6d189b3a2de317
(XEN) Processor: 411fd073: "ARM Limited", variant: 0x1, part 0xd07, rev 0x3
(XEN) 64-bit Execution:
(XEN)   Processor Features:  
(XEN) Exception Levels: EL3:64+32 EL2:64+32 EL1:64+32 EL0:64+32
(XEN) Extensions: FloatingPoint AdvancedSIMD
(XEN)   Debug Features: 10305106 
(XEN)   Auxiliary Features:  
(XEN)   Memory Model Features: 1124 
(XEN)   ISA Features:  00011120 
(XEN) 32-bit Execution:
(XEN)   Processor Features: 0131:00011011
(XEN) Instruction Sets: AArch32 A32 Thumb Thumb-2 Jazelle
(XEN) Extensions: GenericTimer Security
(XEN)   Debug Features: 03010066
(XEN)   Auxiliary Features: 
(XEN)   Memory Model Features: 10201105 4000 0126 02102211
(XEN)  ISA Features: 02101110 13112111 21232042 01112131 00011142 00011121
(XEN) Using SMC Calling Convention v1.0
(XEN) Using PSCI v1.0
(XEN) SMP: Allowing 8 CPUs
(XEN) Generic Timer IRQ: phys=30 hyp=26 virt=27 Freq: 8333 KHz
(XEN) GICv2 initialization:
(XEN) gic_dist_addr=f101
(XEN) gic_cpu_addr=f102
(XEN) gic_hyp_addr=f104
(XEN) gic_vcpu_addr=f106
(XEN) gic_maintenance_irq=25
(XEN) GICv2: Adjusting CPU interface base to 0xf102f000
(XEN) GICv2: 512 lines, 8 cpus, secure (IID 0200043b).
(XEN) XSM Framework v1.0.0 initialized
(XEN) Initialising XSM SILO mode
(XEN) Using scheduler: SMP Credit Scheduler rev2 (credit2)
(XEN) Initializing Credit2 scheduler
(XEN)  load_precision_shift: 18
(XEN)  load_window_shift: 30
(XEN)  underload_balance_tolerance: 0
(XEN)  overload_balance_tolerance: -3
(XEN)  runqueues arrangement: socket
(XEN)  cap enforcement granularity: 10ms
(XEN) load tracking window length 1073741824 ns
(XEN) Adding cpu 0 to runqueue 0
(XEN)  First cpu on runqueue, activating
(XEN) Allocated console ring of 64 KiB.
(XEN) CPU0: Guest atomics will try 10 times before pausing the domain
(XEN) Bringing up CPU1
- CPU 0001 booting -
- Current EL 0008 -
- Xen starting at EL2 -
- Setting up control registers -
- Turning on paging -
- Ready -
(XEN) Adding cpu 1 to runqueue 0
(XEN) CPU1: Guest atomics will try 13 times before pausing the domain
(XEN) CPU 1 booted.
(XEN) Bringing up CPU2
- CPU 0002 booting -
- Current EL 0008 -
- Xen starting at EL2 -
- Setting up control registers -
- Turning on paging -
- Ready -
(XEN) Adding cpu 2 to runqueue 0
(XEN) CPU2: Guest atomics will try 9 times before pausing the domain
(XEN) CPU 2 booted.

Re: [Xen-devel] [PATCH] x86/boot: Fix build dependenices for reloc.c

2019-07-31 Thread Jan Beulich

On 30.07.2019 19:07, Andrew Cooper wrote:
> c/s 201f852eaf added start_info.h and kconfig.h to reloc.c, but only updated
> start_info.h in RELOC_DEPS.
> 
> This causes reloc.c to not be regenerated when Kconfig changes.  It is most
> noticeable when enabling CONFIG_PVH and finding the resulting binary crash
> early with:
> 
>(d9) (XEN)
>(d9) (XEN) 
>(d9) (XEN) Panic on CPU 0:
>(d9) (XEN) Magic value is wrong: c2c2c2c2
>(d9) (XEN) 
>(d9) (XEN)
>(d9) (XEN) Reboot in five seconds...
>(XEN) d9v0 Triple fault - invoking HVM shutdown action 1
> 
> Reported-by: Paul Durrant 
> Signed-off-by: Andrew Cooper 

Acked-by: Jan Beulich 
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] Speculative mitigation facilities report wrong status

On 31/07/2019 11:45, Jin Nan Wang wrote:
> Hi folks,
>
> On 7/31/19 5:44 PM, Andrew Cooper wrote:
>> The check for reporting MD_CLEAR must stay as X86_FEATURE_MD_CLEAR,
>> because this is a property in microcode which no controls, and nothing
>> further to virtualise at Xen's level.
> There are two solution, which one would you like?
>
> solution1: make sure set X86_FEATURE_SC_VERW_PV/HVM, under
> X86_FEATURE_MD_CLEAR exist.

No - this is not a solution.  This causes Xen to ignore spec-ctrl=0 and
use VERW itself when it was instructed not to.

The only bug here is in the printed output.  In your example, with
MDS-capable microcode and spec-ctrl=0 on the command line, the correct
output should be

(XEN)   Support for HVM VMs: MD_CLEAR
(XEN)   Support for PV VMs: MD_CLEAR

The actual behaviour of Xen is correct, but the printed message is
confusing.

~Andrew

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

Re: [Xen-devel] [RFC 5/6] arm64: call enter_hypervisor_head only when it is needed


Hi Andrii,

On 30/07/2019 18:35, Andrii Anisov wrote:


On 26.07.19 13:59, Julien Grall wrote:

Hi,

On 26/07/2019 11:37, Andrii Anisov wrote:

From: Andrii Anisov 

On ARM64 we know exactly if trap happened from hypervisor or guest, so
we do not need to take that decision. This reduces a condition for
all enter_hypervisor_head calls and the function call for traps from
the hypervisor mode.


One condition lost but ...


...In the hot path (actually at any trap).


Everything is in the hot path here, yet there are a lot of other branches. So 
why this branch in particular?


As I have mentioned a few times before, there are a difference between the 
theory and the practice. In theory, removing a branch looks nice. But in 
practice this may not be the case.


In this particular case, I don't believe this is going to have a real impact on 
the performance.


The PSTATE has been saved a few instructions before in cpu_user_regs, so there
are high chance the value will still be in the L1 cache.

The compiler may also decide to do the direct branch when not in guest_mode. A 
trap from the hypervisor is mostly for interrupts. So there are chance this is 
not going to have a real impact on the overall of the interrupt handling.


If you are really worry of the impact of branch then there are a few more 
important places (with a greater benefits) to look:
1) It seems the compiler use a jump table for the switch case in 
do_trap_guest_sync(), so it will result to multiple direct branch everytime.
2) Indirect branch have a non-negligible cost compare to direct branch. 
This is a lot used in the interrupt code (see gic_hw_ops->read_irq()). All of 
them are known at boot time, so they could be replace with direct branch. x86 
recently introduced alternative_call() for this purpose. This could be re-used 
on Arm.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] x86/boot: Fix build dependenices for reloc.c

2019-07-31 Thread Roger Pau Monné

On Wed, Jul 31, 2019 at 10:57:36AM +0100, Andrew Cooper wrote:
> On 31/07/2019 09:47, Roger Pau Monné wrote:
> > On Tue, Jul 30, 2019 at 06:07:54PM +0100, Andrew Cooper wrote:
> >> c/s 201f852eaf added start_info.h and kconfig.h to reloc.c, but only 
> >> updated
> >> start_info.h in RELOC_DEPS.
> >>
> >> This causes reloc.c to not be regenerated when Kconfig changes.  It is most
> >> noticeable when enabling CONFIG_PVH and finding the resulting binary crash
> >> early with:
> >>
> >>   (d9) (XEN)
> >>   (d9) (XEN) 
> >>   (d9) (XEN) Panic on CPU 0:
> >>   (d9) (XEN) Magic value is wrong: c2c2c2c2
> >>   (d9) (XEN) 
> >>   (d9) (XEN)
> >>   (d9) (XEN) Reboot in five seconds...
> >>   (XEN) d9v0 Triple fault - invoking HVM shutdown action 1
> >>
> >> Reported-by: Paul Durrant 
> >> Signed-off-by: Andrew Cooper 
> > Reviewed-by: Roger Pau Monné 
> 
> :) I can use that tag if you'd like.

Ouch, I think it doesn't really matter because the dots are actually
stripped, so it all ends up at rogerpau@. In any case:

Reviewed-by: Roger Pau Monné 

> >
> > Note sure if it's worth spelling out that multiboot.h dependency was
> > also missing.
> 
> The delta to multiboot.h was a consequence of reformatting.  It was
> present before.

Arg, yes, didn't pay enough attention.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH] Speculative mitigation facilities report wrong status

Hi folks,

On 7/31/19 5:44 PM, Andrew Cooper wrote:
> The check for reporting MD_CLEAR must stay as X86_FEATURE_MD_CLEAR,
> because this is a property in microcode which no controls, and nothing
> further to virtualise at Xen's level.

There are two solution, which one would you like?

solution1: make sure set X86_FEATURE_SC_VERW_PV/HVM, under
X86_FEATURE_MD_CLEAR exist.

~ 1084 if ( opt_md_clear_pv && boot_cpu_has(X86_FEATURE_MD_CLEAR))
  1085 setup_force_cpu_cap(X86_FEATURE_SC_VERW_PV);
~ 1086 if ( opt_md_clear_pv || opt_md_clear_hvm &&
boot_cpu_has(X86_FEATURE_MD_CLEAR))
  1087 setup_force_cpu_cap(X86_FEATURE_SC_VERW_IDLE);
~ 1088 if ( opt_md_clear_hvm && !(caps & ARCH_CAPS_SKIP_L1DFL) &&
!opt_l1d_flush && boot_cpu_has(X86_FEATURE_MD_CLEAR))
  1089 setup_force_cpu_cap(X86_FEATURE_SC_VERW_HVM);


Solution2:

   365 #ifdef CONFIG_HVM
   366 printk("  Support for HVM VMs:%s%s%s%s%s\n",
   367    (boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
   368 boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ||
~  369 (boot_cpu_has(X86_FEATURE_MD_CLEAR) &&
boot_cpu_has(X86_FEATURE_SC_VERW_HVM)) ||
   370 opt_eager_fpu)   ?
""   : " None",
   371    boot_cpu_has(X86_FEATURE_SC_MSR_HVM)  ? "
MSR_SPEC_CTRL" : "",
   372    boot_cpu_has(X86_FEATURE_SC_RSB_HVM)  ? "
RSB"   : "",
   373    opt_eager_fpu ? "
EAGER_FPU" : "",
~  374    (boot_cpu_has(X86_FEATURE_MD_CLEAR) &&
boot_cpu_has(X86_FEATURE_SC_VERW_HVM)) ? " MD_CLEAR"  : "");
   375
   376 #endif
   377 #ifdef CONFIG_PV
   378 printk("  Support for PV VMs:%s%s%s%s%s\n",
   379    (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
   380 boot_cpu_has(X86_FEATURE_SC_RSB_PV) ||
~  381 (boot_cpu_has(X86_FEATURE_MD_CLEAR) &&
boot_cpu_has(X86_FEATURE_SC_VERW_PV)) ||
   382 opt_eager_fpu)   ?
""   : " None",
   383    boot_cpu_has(X86_FEATURE_SC_MSR_PV)   ? "
MSR_SPEC_CTRL" : "",
   384    boot_cpu_has(X86_FEATURE_SC_RSB_PV)   ? "
RSB"   : "",
   385    opt_eager_fpu ? "
EAGER_FPU" : "",
~  386    (boot_cpu_has(X86_FEATURE_MD_CLEAR) &&
boot_cpu_has(X86_FEATURE_SC_VERW_PV))  ? " MD_CLEAR"  : "");

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

Re: [Xen-devel] [PATCH] x86/boot: Fix build dependenices for reloc.c

On 31/07/2019 11:38, Wei Liu wrote:
> On Tue, 30 Jul 2019 at 18:08, Andrew Cooper  wrote:
>> c/s 201f852eaf added start_info.h and kconfig.h to reloc.c, but only updated
>> start_info.h in RELOC_DEPS.
>>
>> This causes reloc.c to not be regenerated when Kconfig changes.  It is most
>> noticeable when enabling CONFIG_PVH and finding the resulting binary crash
>> early with:
>>
>>   (d9) (XEN)
>>   (d9) (XEN) 
>>   (d9) (XEN) Panic on CPU 0:
>>   (d9) (XEN) Magic value is wrong: c2c2c2c2
>>   (d9) (XEN) 
>>   (d9) (XEN)
>>   (d9) (XEN) Reboot in five seconds...
>>   (XEN) d9v0 Triple fault - invoking HVM shutdown action 1
>>
> Nice. I saw this before but never got around fixing it. Thanks for
> tracking it down. :-)

It has been bugging me for ages, and yet still took Paul stumbling over
it to wonder "how hard would this to be to figure out properly?".  The
answer is 10s to identify the likely cause, and far longer than that to
debug the dependency tracking part.

This bit of the build system is nasty.  David Woodhouse had an idea of
how to drop it all, so with any luck, it isn't going to survive long.

~Andrew

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

Re: [Xen-devel] [PATCH] x86/boot: Fix build dependenices for reloc.c

2019-07-31 Thread Wei Liu

On Tue, 30 Jul 2019 at 18:08, Andrew Cooper  wrote:
>
> c/s 201f852eaf added start_info.h and kconfig.h to reloc.c, but only updated
> start_info.h in RELOC_DEPS.
>
> This causes reloc.c to not be regenerated when Kconfig changes.  It is most
> noticeable when enabling CONFIG_PVH and finding the resulting binary crash
> early with:
>
>   (d9) (XEN)
>   (d9) (XEN) 
>   (d9) (XEN) Panic on CPU 0:
>   (d9) (XEN) Magic value is wrong: c2c2c2c2
>   (d9) (XEN) 
>   (d9) (XEN)
>   (d9) (XEN) Reboot in five seconds...
>   (XEN) d9v0 Triple fault - invoking HVM shutdown action 1
>

Nice. I saw this before but never got around fixing it. Thanks for
tracking it down. :-)

Wei.

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

[Xen-devel] [PATCH v4 1/2] xen/arm: extend fdt_property_interrupts

Extend fdt_property_interrupts to deal with other domain than the hwdom.

The prototype of fdt_property_interrupts() has been modified
to support both hwdom and domU in one function.

This is a preparatory for the patch "xen/arm: merge make_timer_node and
make_timer_domU_node". Original goal is to merge make_timer_node and
make_timer_domU_node functions. See discussion in e-mail, the Message-ID is:
<20190620103805.927-1-viktor.mitin...@gmail.com>

Note: there is no functional changes introduced by this patch.

Suggested-by: Julien Grall 
Signed-off-by: Viktor Mitin 
---
 xen/arch/arm/domain_build.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 4c8404155a..d04a1c3aec 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -621,17 +621,19 @@ static void __init set_interrupt(gic_interrupt_t 
interrupt,
  *  "interrupts": contains the list of interrupts
  *  "interrupt-parent": link to the GIC
  */
-static int __init fdt_property_interrupts(void *fdt, gic_interrupt_t *intr,
-  unsigned num_irq)
+static int __init fdt_property_interrupts(const struct kernel_info *kinfo,
+gic_interrupt_t *intr, unsigned num_irq)
 {
 int res;
+uint32_t phandle = is_hardware_domain(kinfo->d) ?
+   dt_interrupt_controller->phandle : GUEST_PHANDLE_GIC;
 
-res = fdt_property(fdt, "interrupts", intr, sizeof (intr[0]) * num_irq);
+res = fdt_property(kinfo->fdt, "interrupts",
+   intr, sizeof (intr[0]) * num_irq);
 if ( res )
 return res;
 
-res = fdt_property_cell(fdt, "interrupt-parent",
-dt_interrupt_controller->phandle);
+res = fdt_property_cell(kinfo->fdt, "interrupt-parent", phandle);
 
 return res;
 }
@@ -733,7 +735,7 @@ static int __init make_hypervisor_node(struct domain *d,
  *  TODO: Handle properly the cpumask;
  */
 set_interrupt(intr, d->arch.evtchn_irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
-res = fdt_property_interrupts(fdt, , 1);
+res = fdt_property_interrupts(kinfo, , 1);
 if ( res )
 return res;
 
@@ -960,8 +962,10 @@ static int __init make_gic_node(const struct domain *d, 
void *fdt,
 return res;
 }
 
-static int __init make_timer_node(const struct domain *d, void *fdt)
+static int __init make_timer_node(const struct kernel_info *kinfo)
 {
+void *fdt = kinfo->fdt;
+
 static const struct dt_device_match timer_ids[] __initconst =
 {
 DT_MATCH_COMPATIBLE("arm,armv7-timer"),
@@ -1016,7 +1020,7 @@ static int __init make_timer_node(const struct domain *d, 
void *fdt)
 dt_dprintk("  Virt interrupt %u\n", irq);
 set_interrupt(intrs[2], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
 
-res = fdt_property_interrupts(fdt, intrs, 3);
+res = fdt_property_interrupts(kinfo, intrs, 3);
 if ( res )
 return res;
 
@@ -1377,7 +1381,7 @@ static int __init handle_node(struct domain *d, struct 
kernel_info *kinfo,
 if ( device_get_class(node) == DEVICE_GIC )
 return make_gic_node(d, kinfo->fdt, node);
 if ( dt_match_node(timer_matches, node) )
-return make_timer_node(d, kinfo->fdt);
+return make_timer_node(kinfo);
 
 /* Skip nodes used by Xen */
 if ( dt_device_used_by(node) == DOMID_XEN )
-- 
2.17.1


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

[Xen-devel] [PATCH v4 2/2] xen/arm: merge make_timer_node and make_timer_domU_node

Merged make_timer_node and make_timer_domU_node into one function
make_timer_node.

Kept the domU version for the compatible as it is simpler.
Kept the hw version for the clock as it is relevant for the both cases.

Suggested-by: Julien Grall 
Signed-off-by: Viktor Mitin 
---
v4 updates:
   updated "Kept the domU version for the compatible as it is simpler"

 xen/arch/arm/domain_build.c | 109 +---
 1 file changed, 39 insertions(+), 70 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d04a1c3aec..4d7c3411a6 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -964,8 +964,12 @@ static int __init make_gic_node(const struct domain *d, 
void *fdt,
 
 static int __init make_timer_node(const struct kernel_info *kinfo)
 {
+int res;
 void *fdt = kinfo->fdt;
-
+unsigned int irq[MAX_TIMER_PPI];
+gic_interrupt_t intrs[3];
+u32 clock_frequency;
+bool clock_valid;
 static const struct dt_device_match timer_ids[] __initconst =
 {
 DT_MATCH_COMPATIBLE("arm,armv7-timer"),
@@ -973,15 +977,6 @@ static int __init make_timer_node(const struct kernel_info 
*kinfo)
 { /* sentinel */ },
 };
 struct dt_device_node *dev;
-u32 len;
-const void *compatible;
-int res;
-unsigned int irq;
-gic_interrupt_t intrs[3];
-u32 clock_frequency;
-bool clock_valid;
-
-dt_dprintk("Create timer node\n");
 
 dev = dt_find_matching_node(NULL, timer_ids);
 if ( !dev )
@@ -990,35 +985,49 @@ static int __init make_timer_node(const struct 
kernel_info *kinfo)
 return -FDT_ERR_XEN(ENOENT);
 }
 
-compatible = dt_get_property(dev, "compatible", );
-if ( !compatible )
-{
-dprintk(XENLOG_ERR, "Can't find compatible property for timer node\n");
-return -FDT_ERR_XEN(ENOENT);
-}
-
 res = fdt_begin_node(fdt, "timer");
 if ( res )
 return res;
 
-res = fdt_property(fdt, "compatible", compatible, len);
-if ( res )
-return res;
+if ( !is_64bit_domain(kinfo->d) )
+{
+res = fdt_property_string(fdt, "compatible", "arm,armv7-timer");
+if ( res )
+return res;
+}
+else
+{
+res = fdt_property_string(fdt, "compatible", "arm,armv8-timer");
+if ( res )
+return res;
+}
 
 /* The timer IRQ is emulated by Xen. It always exposes an active-low
  * level-sensitive interrupt */
+if ( is_hardware_domain(kinfo->d) )
+{
+irq[TIMER_PHYS_SECURE_PPI] = timer_get_irq(TIMER_PHYS_SECURE_PPI);
+irq[TIMER_PHYS_NONSECURE_PPI] =
+timer_get_irq(TIMER_PHYS_NONSECURE_PPI);
+irq[TIMER_VIRT_PPI] = timer_get_irq(TIMER_VIRT_PPI);
+}
+else
+{
+irq[TIMER_PHYS_SECURE_PPI] = GUEST_TIMER_PHYS_S_PPI;
+irq[TIMER_PHYS_NONSECURE_PPI] = GUEST_TIMER_PHYS_NS_PPI;
+irq[TIMER_VIRT_PPI] = GUEST_TIMER_VIRT_PPI;
+}
 
-irq = timer_get_irq(TIMER_PHYS_SECURE_PPI);
-dt_dprintk("  Secure interrupt %u\n", irq);
-set_interrupt(intrs[0], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+dt_dprintk("  Secure interrupt %u\n", irq[TIMER_PHYS_SECURE_PPI]);
+set_interrupt(intrs[0], irq[TIMER_PHYS_SECURE_PPI],
+ 0xf, DT_IRQ_TYPE_LEVEL_LOW);
 
-irq = timer_get_irq(TIMER_PHYS_NONSECURE_PPI);
-dt_dprintk("  Non secure interrupt %u\n", irq);
-set_interrupt(intrs[1], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+dt_dprintk("  Non secure interrupt %u\n", irq[TIMER_PHYS_NONSECURE_PPI]);
+set_interrupt(intrs[1], irq[TIMER_PHYS_NONSECURE_PPI],
+ 0xf, DT_IRQ_TYPE_LEVEL_LOW);
 
-irq = timer_get_irq(TIMER_VIRT_PPI);
-dt_dprintk("  Virt interrupt %u\n", irq);
-set_interrupt(intrs[2], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+dt_dprintk("  Virt interrupt %u\n", irq[TIMER_VIRT_PPI]);
+set_interrupt(intrs[2], irq[TIMER_VIRT_PPI], 0xf, DT_IRQ_TYPE_LEVEL_LOW);
 
 res = fdt_property_interrupts(kinfo, intrs, 3);
 if ( res )
@@ -1603,46 +1612,6 @@ static int __init make_gic_domU_node(const struct domain 
*d, void *fdt)
 }
 }
 
-static int __init make_timer_domU_node(const struct domain *d, void *fdt)
-{
-int res;
-gic_interrupt_t intrs[3];
-
-res = fdt_begin_node(fdt, "timer");
-if ( res )
-return res;
-
-if ( !is_64bit_domain(d) )
-{
-res = fdt_property_string(fdt, "compatible", "arm,armv7-timer");
-if ( res )
-return res;
-}
-else
-{
-res = fdt_property_string(fdt, "compatible", "arm,armv8-timer");
-if ( res )
-return res;
-}
-
-set_interrupt(intrs[0], GUEST_TIMER_PHYS_S_PPI, 0xf, 
DT_IRQ_TYPE_LEVEL_LOW);
-set_interrupt(intrs[1], GUEST_TIMER_PHYS_NS_PPI, 0xf, 
DT_IRQ_TYPE_LEVEL_LOW);
-set_interrupt(intrs[2], GUEST_TIMER_VIRT_PPI, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
-
-res =

Re: [Xen-devel] [PATCH] Speculative mitigation facilities report wrong status


I will improve it soon.

thanks

James

From: Andrew Cooper 
Sent: Wednesday, July 31, 2019 5:44:50 PM
To: xen-devel@lists.xenproject.org ; Jin Nan 
Wang 
Cc: roger@citrix.com ; Jan Beulich 
; w...@xen.org 
Subject: Re: [PATCH] Speculative mitigation facilities report wrong status

On 31/07/2019 10:30, Jin Nan Wang wrote:
> Diff with 'spec-ctrl=no' and without.
> 
> --- xen.dmesg.5.log 2019-07-31 14:55:38.138173874 +0800
> +++ xen.dmesg.6.log 2019-07-31 14:59:50.223516313 +0800
> @@ -7,7 +7,7 @@
>  (XEN) Xen version 4.12.0_14-1 (abu...@suse.de) (gcc (SUSE Linux) 4.8.5) 
> debug=n  Mon Jun 17 15:08:33 UTC 2019
>  (XEN) Latest ChangeSet:
>  (XEN) Bootloader: GRUB2 2.02
> -(XEN) Command line: vga=gfx-1024x768x16 crashkernel=251M<4G ucode=scan 
> console=vga,com1 loglvl=all guest_loglvl=all
> +(XEN) Command line: vga=gfx-1024x768x16 crashkernel=251M<4G ucode=scan 
> spec-ctrl=no console=vga,com1 loglvl=all guest_loglvl=all
>  (XEN) Xen image load base address: 0
>  (XEN) Video information:
>  (XEN)  VGA is graphics mode 1024x768, 16 bpp
> @@ -159,12 +159,12 @@
>  (XEN) Speculative mitigation facilities:
>  (XEN)   Hardware features: IBRS/IBPB STIBP L1D_FLUSH SSBD MD_CLEAR
>  (XEN)   Compiled-in support: INDIRECT_THUNK SHADOW_PAGING
> -(XEN)   Xen settings: BTI-Thunk JMP, SPEC_CTRL: IBRS+ SSBD-, Other: IBPB 
> L1D_FLUSH VERW
> +(XEN)   Xen settings: BTI-Thunk JMP, SPEC_CTRL: IBRS- SSBD-, Other:
>  (XEN)   L1TF: believed vulnerable, maxphysaddr L1D 46, CPUID 46, Safe 
> address 3000
> -(XEN)   Support for HVM VMs: MSR_SPEC_CTRL RSB EAGER_FPU MD_CLEAR
> -(XEN)   Support for PV VMs: MSR_SPEC_CTRL RSB EAGER_FPU MD_CLEAR
> -(XEN)   XPTI (64-bit PV only): Dom0 enabled, DomU enabled (with PCID)
> -(XEN)   PV L1TF shadowing: Dom0 disabled, DomU enabled
> +(XEN)   Support for HVM VMs: None MD_CLEAR
> +(XEN)   Support for PV VMs: None MD_CLEAR
> +(XEN)   XPTI (64-bit PV only): Dom0 disabled, DomU disabled (with PCID)
> +(XEN)   PV L1TF shadowing: Dom0 disabled, DomU disabled
>  (XEN) Using scheduler: SMP Credit Scheduler rev2 (credit2)
>  (XEN) Initializing Credit2 scheduler
>  (XEN)  load_precision_shift: 18
> ==
>
> In "Support for HVM VMs: Support for PV VMs: " lines,
> Others feature is reported as "NONE", MD_CLEAR not.
>
> code review:
> xen/arch/x86/spec_ctrl.c:
> 99 disable_common:
>100 opt_rsb_pv = false;
>101 opt_rsb_hvm = false;
>102 opt_md_clear_pv = 0;   <- they have been disable when 
> 'spec-ctrl=no'
>103 opt_md_clear_hvm = 0;
>104
>
> X86_FEATURE_SC_VERW_PV, X86_FEATURE_SC_VERW_HVM will not be enabled
>
>  1070 if ( opt_md_clear_pv )
>   1071 setup_force_cpu_cap(X86_FEATURE_SC_VERW_PV);
>   1072 if ( opt_md_clear_pv || opt_md_clear_hvm )
>   1073 setup_force_cpu_cap(X86_FEATURE_SC_VERW_IDLE);
>   1074 if ( opt_md_clear_hvm && !(caps & ARCH_CAPS_SKIP_L1DFL) && 
> !opt_l1d_flush )
>   1075 setup_force_cpu_cap(X86_FEATURE_SC_VERW_HVM);
>
> But when we report the status of MD_CLEAR, we use X86_FEATURE_MD_CLEAR to 
> check.
> it seems not good.
>
>360 printk("  Support for HVM VMs:%s%s%s%s%s\n",
>361(boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
>362 boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ||
>363 opt_eager_fpu)   ? ""  
>  : " None",
>364boot_cpu_has(X86_FEATURE_SC_MSR_HVM)  ? " 
> MSR_SPEC_CTRL" : "",
>365boot_cpu_has(X86_FEATURE_SC_RSB_HVM)  ? " RSB"  
>  : "",
>366opt_eager_fpu ? " EAGER_FPU"
>  : "",
>367>   boot_cpu_has(X86_FEATURE_MD_CLEAR)? " MD_CLEAR" 
>  : "");
>368
>369 #endif
>370 #ifdef CONFIG_PV
>371 printk("  Support for PV VMs:%s%s%s%s%s\n",
>372(boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
>373 boot_cpu_has(X86_FEATURE_SC_RSB_PV) ||
>374 opt_eager_fpu)   ? ""  
>  : " None",
>375boot_cpu_has(X86_FEATURE_SC_MSR_PV)   ? " 
> MSR_SPEC_CTRL" : "",
>376boot_cpu_has(X86_FEATURE_SC_RSB_PV)   ? " RSB"  
>  : "",
>377opt_eager_fpu ? " EAGER_FPU"
>  : "",
>378>   boot_cpu_has(X86_FEATURE_MD_CLEAR)? " MD_CLEAR" 
>  : "");
>
> There is a patch for this issue.

Thankyou for the report.  However, the patch is only half correct.

It should only be the first part, adding the extra check to avoid "None".

The check for reporting MD_CLEAR must stay as X86_FEATURE_MD_CLEAR,
because this is a property in microcode which no controls, and nothing
further to virtualise at Xen's level.

I.e. even with spec-ctrl=no, if the microcode is

Re: [Xen-devel] [PATCH] x86/boot: Fix build dependenices for reloc.c

On 31/07/2019 09:47, Roger Pau Monné wrote:
> On Tue, Jul 30, 2019 at 06:07:54PM +0100, Andrew Cooper wrote:
>> c/s 201f852eaf added start_info.h and kconfig.h to reloc.c, but only updated
>> start_info.h in RELOC_DEPS.
>>
>> This causes reloc.c to not be regenerated when Kconfig changes.  It is most
>> noticeable when enabling CONFIG_PVH and finding the resulting binary crash
>> early with:
>>
>>   (d9) (XEN)
>>   (d9) (XEN) 
>>   (d9) (XEN) Panic on CPU 0:
>>   (d9) (XEN) Magic value is wrong: c2c2c2c2
>>   (d9) (XEN) 
>>   (d9) (XEN)
>>   (d9) (XEN) Reboot in five seconds...
>>   (XEN) d9v0 Triple fault - invoking HVM shutdown action 1
>>
>> Reported-by: Paul Durrant 
>> Signed-off-by: Andrew Cooper 
> Reviewed-by: Roger Pau Monné 

:) I can use that tag if you'd like.

>
> Note sure if it's worth spelling out that multiboot.h dependency was
> also missing.

The delta to multiboot.h was a consequence of reformatting.  It was
present before.

~Andrew

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

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

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 xen  1585ed3c702e680ae492d852c8cff62cf300df99
baseline version:
 xen  22ec7474348fea2c4a32b0872dd3385bf3785a26

Last test of basis   139434  2019-07-28 09:18:39 Z3 days
Testing same since   139557  2019-07-31 09:18:58 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Andrii Anisov 
  Dario Faggioli 
  George Dunlap 
  Jan Beulich 
  Juergen Gross 
  Julien Grall 
  Paul Durrant 
  Stefano Stabellini 
  Wei Liu 
  Zhang Chen 

jobs:
 coverity-amd64   pass



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   22ec747434..1585ed3c70  1585ed3c702e680ae492d852c8cff62cf300df99 -> 
coverity-tested/smoke

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

[Xen-devel] [PATCH] Speculative mitigation facilities report wrong status