Re: [Xen-devel] [PATCH] xen/x86: add diagnostic printout to xen_mc_flush() in case of error
On 26/11/2018 21:11, Boris Ostrovsky wrote:
> On 11/23/18 11:24 AM, Juergen Gross wrote:
>> Failure of an element of a Xen multicall is signalled via a WARN()
>> only unless the kernel is compiled with MC_DEBUG. It is impossible to
>
> s/unless/if
>
>
>> know which element failed and why it did so.
>>
>> Change that by printing the related information even without MC_DEBUG,
>> even if maybe in some limited form (e.g. without information which
>> caller produced the failing element).
>>
>> Move the printing out of the switch statement in order to have the
>> same information for a single call.
>>
>> Signed-off-by: Juergen Gross
>> ---
>> arch/x86/xen/multicalls.c | 35 ---
>> 1 file changed, 20 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/x86/xen/multicalls.c b/arch/x86/xen/multicalls.c
>> index 2bce7958ce8b..0766a08bdf45 100644
>> --- a/arch/x86/xen/multicalls.c
>> +++ b/arch/x86/xen/multicalls.c
>> @@ -69,6 +69,11 @@ void xen_mc_flush(void)
>>
>> trace_xen_mc_flush(b->mcidx, b->argidx, b->cbidx);
>>
>> +#if MC_DEBUG
>> +memcpy(b->debug, b->entries,
>> + b->mcidx * sizeof(struct multicall_entry));
>> +#endif
>> +
>> switch (b->mcidx) {
>> case 0:
>> /* no-op */
>> @@ -87,32 +92,34 @@ void xen_mc_flush(void)
>> break;
>>
>> default:
>> -#if MC_DEBUG
>> -memcpy(b->debug, b->entries,
>> - b->mcidx * sizeof(struct multicall_entry));
>> -#endif
>> -
>> if (HYPERVISOR_multicall(b->entries, b->mcidx) != 0)
>> BUG();
>> for (i = 0; i < b->mcidx; i++)
>> if (b->entries[i].result < 0)
>> ret++;
>> +}
>>
>> +if (WARN_ON(ret)) {
>> +pr_err("%d of %d multicall(s) failed: cpu %d\n",
>> + ret, b->mcidx, smp_processor_id());
>> +for (i = 0; i < b->mcidx; i++) {
>> +if (b->entries[i].result < 0) {
>> #if MC_DEBUG
>> -if (ret) {
>> -printk(KERN_ERR "%d multicall(s) failed: cpu %d\n",
>> - ret, smp_processor_id());
>> -dump_stack();
>> -for (i = 0; i < b->mcidx; i++) {
>> -printk(KERN_DEBUG " call %2d/%d: op=%lu
>> arg=[%lx] result=%ld\t%pF\n",
>> - i+1, b->mcidx,
>> +pr_err(" call %2d: op=%lu arg=[%lx]
>> result=%ld\t%pF\n",
>> + i + 1,
>> b->debug[i].op,
>> b->debug[i].args[0],
>> b->entries[i].result,
>> b->caller[i]);
>> +#else
>> +pr_err(" call %2d: op=%lu arg=[%lx]
>> result=%ld\n",
>> + i + 1,
>> + b->entries[i].op,
>> + b->entries[i].args[0],
>> + b->entries[i].result);
>> +#endif
>
> Doesn't (non-debug) hypervisor corrupt op and args?
No. Only debug hypervisor does so.
See my patch (and rather lengthy discussion) on xen-devel:
https://lists.xen.org/archives/html/xen-devel/2018-11/msg02714.html
>
> (Also, we don't really need to print anything when b->entries[i].result
> == 0)
Right. Did you miss the:
+ if (b->entries[i].result < 0) {
above?
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
On 26/11/2018 17:50, Jan Beulich wrote: On 26.11.18 at 17:17, wrote: >> I really fail to see why it is so bad to not clobber data in a case >> which normally should never occur. > > See Andrew's original reply. You're also clobbering on potential > success paths. I think you are missing a "not" here. But yes, I agree. This is a downside _I_ think we can live with. How many cases (with productive hypervisors) are known where multicall parameters have been illegally re-used? I guess the number is exactly 0. I know of least one case where not clobbering would be useful for diagnostic reasons. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
On 26/11/2018 17:51, Julien Grall wrote: > > > On 26/11/2018 16:17, Juergen Gross wrote: >> On 26/11/2018 17:01, Julien Grall wrote: >>> >>> >>> On 26/11/2018 15:29, Juergen Gross wrote: On 26/11/2018 15:58, Jan Beulich wrote: On 26.11.18 at 15:23, wrote: >> On 26/11/2018 15:01, Jan Beulich wrote: >> On 26.11.18 at 14:52, wrote: I don't think the hypervisor should explicitly try to make it as hard as possible for the guest to find problems in the code. >>> >>> That's indeed not the hypervisor's goal. Instead it tries to make >>> it as hard as possible for the guest (developer) to make wrong >>> assumptions. >> >> Let's look at the current example why I wrote this patch: >> >> The Linux kernel's use of multicalls should never trigger any single >> call to return an error (return value < 0). A kernel compiled for >> productive use will catch such errors, but has no knowledge which >> single call has failed, as it doesn't keep track of the single >> entries >> (non-productive kernels have an option available in the respective >> source to copy the entries before doing the multicall in order to >> have >> some diagnostic data available in case of such an error). Catching an >> error from a multicall right now means a WARN() with a stack >> backtrace >> (for the multicall itself, not for the entry causing the error). >> >> I have a customer report for a case where such a backtrace was >> produced >> and a kernel crash some seconds later, obviously due to illegally >> unmapped memory pages resulting from the failed multicall. >> Unfortunately >> there are multiple possibilities what might have gone wrong and I >> don't >> know which one was the culprit. The problem can't be a very common >> one, >> because there is only one such report right now, which might >> depend on >> a special driver. >> >> Finding this bug without a known reproducer and the current amount of >> diagnostic data is next to impossible. So I'd like to have more data >> available without having to hurt performance for the 99.99% of >> the >> cases where nothing bad happens. >> >> In case you have an idea how to solve this problem in another way >> I'd be >> happy to follow that route. I'd really like to be able to have a >> better >> clue in case such an error occurs in future. > > Since you have a production kernel, I assume you also have a > production hypervisor. This hypervisor doesn't clobber the > arguments if I'm not mistaken. Therefore > - in the debugging scenario you (can) have all data available by > virtue of the information getting copied in the kernel, > - in the release scenario you have all data available since it's > left un-clobbered. > Am I missing anything (I don't view mixed debug/release setups > of kernel and hypervisor as overly important here)? No, you are missing nothing here. OTOH a debug hypervisor destroying debug data is kind of weird, so I posted this patch. >>> >>> This is a quite common approach if you want to enforce the other entity >>> to not rely on some fields. This also follows what we do for hypercalls >>> (at least on Arm). >> >> I don't question that general mechanism. >> >> What I question is doing the clobbering in this case where the caller >> would need to take special measures for copying the needed debug data >> which will hit performance. So not doing the clobbering would only be >> in the very rare error case, not in the common case where the guest >> should really have no need to see the preserved data. >> >> I really fail to see why it is so bad to not clobber data in a case >> which normally should never occur. >> The only outcome of clobbering the >> data in the error case is making diagnosis of that error much harder. >> Its not as if there would be secret data suddenly made available to the >> guest. Its just avoiding the need for the guest to copy the data for >> each multicall for the very unlikely chance an error might occur. And we >> are not speaking of a hypercall issued then and now, but of the path hit >> for nearly every memory-management action and context switch of PV >> guests. So doing always the copy would really be visible. > > For a first, now the behavior will not be the same as when handling a > single hypercall. I would much prefer if the behavior is kept the same > everywhere. Clobbering the multicall parameters in memory and the registers of a hypercall are different from the beginning. Register clobbering is very valuable as it is easy to mess up register usage on the guest side (just had this issue in grub2 where re-using a register value would occur only without diagnostic prints enabled due to inlining). I fail to see how it is possible to re-use multicall arguments
Re: [Xen-devel] [PATCH v3] xen/balloon: Mark unallocated host memory as UNUSABLE
On 11/26/18 2:57 PM, Igor Druzhinin wrote:
> On 26/11/2018 19:42, Boris Ostrovsky wrote:
>> On 11/26/18 12:10 PM, Igor Druzhinin wrote:
>>> On 26/11/2018 16:25, Boris Ostrovsky wrote:
On 11/25/18 8:00 PM, Igor Druzhinin wrote:
> On 20/12/2017 14:05, Boris Ostrovsky wrote:
>> Commit f5775e0b6116 ("x86/xen: discard RAM regions above the maximum
>> reservation") left host memory not assigned to dom0 as available for
>> memory hotplug.
>>
>> Unfortunately this also meant that those regions could be used by
>> others. Specifically, commit fa564ad96366 ("x86/PCI: Enable a 64bit BAR
>> on AMD Family 15h (Models 00-1f, 30-3f, 60-7f)") may try to map those
>> addresses as MMIO.
>>
>> To prevent this mark unallocated host memory as E820_TYPE_UNUSABLE (thus
>> effectively reverting f5775e0b6116) and keep track of that region as
>> a hostmem resource that can be used for the hotplug.
>>
>> Signed-off-by: Boris Ostrovsky
> This commit breaks Xen balloon memory hotplug for us in Dom0 with
> "hoplug_unpopulated" set to 1. The issue is that the common kernel
> memory onlining procedures require "System RAM" resource to be 1-st
> level. That means by inserting it under "Unusable memory" as the commit
> above does (intentionally or not) we make it 2-nd level and break memory
> onlining.
What do you mean by 1st and 2nd level?
>>> I mean the level of a resource in IOMEM tree (the one that's printed
>>> from /proc/iomem). 1-st level means its parent is root and so on.
>> Ah, OK. Doesn't
>> additional_memory_resource()->insert_resource(iomem_resource) place the
>> RAM at 1st level? And if not, can we make it so?
>>
> That'd mean splitting "Unusable memory" resource. Since it's allocated
> from bootmem it has proven to be quite difficult but there are seem to
> be special functions available particularly for memory resource
> management operations that I've not yet experimented with. So the answer
> is probably - maybe yes but not straightforward.
>
> There are multiple ways to fix it depending on what was the intention of
> original commit and what exactly it tried to workaround. It seems it
> does several things at once:
> 1) Marks non-Dom0 host memory "Unusable memory" in resource tree.
> 2) Keeps track of all the areas safe for hotplug in Dom0
> 3) Changes allocation algorithms itself in balloon driver to use those
> areas
Pretty much. (3) is true in the sense that memory is first allocated
from hostmem_resource (which is non-dom0 RAM).
> Are all the things above necessary to cover the issue in fa564ad96366
> ("x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 00-1f, 30-3f,
> 60-7f)")?
Not anymore, as far as that particular commit is concerned, but that's
because of 03a551734 ("x86/PCI: Move and shrink AMD 64-bit window to
avoid conflict") which was introduced after balloon patch. IIRC there
were some issues with fa564ad96366 unrelated to balloon.
>>> If it's not a problem anymore IIUC, can we revert the change as it still
>>> breaks "hotplug_unpopulated=1" for the reasons I described above?
>> Since this seems to have broken existing feature this would be an
>> option. But before going that route I'd like to see if we can fix the patch.
>>
>> I have been unable to reproduce your problem. Can you describe what you did?
>>
> It doesn't happen on all configurations as sometimes the memory is
> successfully hotplugged to a hole depending on the size of Dom0 memory.
> But we reproduced it quite reliably with small Dom0 sizes like 752MB.
>
> XenServer is using this feature to hotplug additional memory for grant
> table operations so we started a VM and observed a stable hang.
>
> Can we remove "Unusable memory" resources as soon as we finished
> booting? Is removing on-demand is preferable over "shoot them all" in
> that case?
The concern is that in principle nothing prevents someone else to do
exact same thing fa564ad96366 did, which is grab something from right
above end of RAM as the kernel sees it. And that can be done at any point.
>>> Nothing prevents - true, but that's plainly wrong from OS point of view
>>> to grab physical ranges for something without knowing what's actually
>>> behind on that platform.
>> I am not sure I agree that this is plainly wrong. If not for BIOS issues
>> that 03a551734cf mentions I think what the original implementation of
>> fa564ad963 did was perfectly reasonable. Which is why I would prefer to
>> keep keep the hostmem resource *if possible*.
>>
> Exactly, those *are* BIOS issues and are not supposed to be workarounded
> by the OS. And as the next commit showed even the workaround didn't
> quite helped with it.
>
> I agree that having hotmem as a precaution is fine but only if there is
> a non-cringy way to keep things working with it which I'm not sure does
> exist.
We have mo
[Xen-devel] [xen-4.7-testing test] 130773: regressions - trouble: blocked/broken/fail/pass
flight 130773 xen-4.7-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/130773/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-pvopsbroken build-arm64-xsm broken build-arm64 broken test-xtf-amd64-amd64-5 50 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 129540 Regressions which are regarded as allowable (not blocking): build-arm64-pvops 2 hosts-allocate broken REGR. vs. 129540 build-arm64 2 hosts-allocate broken REGR. vs. 129540 build-arm64-xsm 2 hosts-allocate broken REGR. vs. 129540 Tests which did not succeed, but are not blocking: build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a build-arm64-xsm 3 capture-logs broken blocked in 129540 build-arm64-pvops 3 capture-logs broken blocked in 129540 build-arm64 3 capture-logs broken blocked in 129540 test-xtf-amd64-amd64-5 69 xtf/test-hvm64-xsa-278 fail blocked in 129540 test-xtf-amd64-amd64-1 50 xtf/test-hvm64-lbr-tsx-vmentry fail like 129540 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 129540 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 129540 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 129540 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 129540 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 129540 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 129540 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 129540 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 129540 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 129540 test-amd64-amd64-xl-rtds 10 debian-install fail like 129540 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail like 129540 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 129540 test-amd64-i386-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-amd64-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-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-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-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-raw 12 migrate-support-checkfail never pass test-armhf-armhf-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-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-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-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-i386-xl-qemut-win10-i38
Re: [Xen-devel] [PATCH] pvcalls-front: fixes incorrect error handling
On Mon, Nov 26, 2018 at 03:31:39PM -0500, Boris Ostrovsky wrote: > On 11/21/18 9:07 PM, Pan Bian wrote: > > kfree() is incorrectly used to release the pages allocated by > > __get_free_page() and __get_free_pages(). Use the matching deallocators > > i.e., free_page() and free_pages(), respectively. > > > > Signed-off-by: Pan Bian > > --- > > drivers/xen/pvcalls-front.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c > > index 2f11ca7..77224d8 100644 > > --- a/drivers/xen/pvcalls-front.c > > +++ b/drivers/xen/pvcalls-front.c > > @@ -385,8 +385,8 @@ static int create_active(struct sock_mapping *map, int > > *evtchn) > > out_error: > > if (*evtchn >= 0) > > xenbus_free_evtchn(pvcalls_front_dev, *evtchn); > > - kfree(map->active.data.in); > > - kfree(map->active.ring); > > + free_pages((unsigned long)map->active.data.in, PVCALLS_RING_ORDER); > > Is map->active.data.in guaranteed to be NULL when entering this routine? I am not sure yet. Sorry for that. I observed the mismatches between __get_free_page and kfree, and submitted the patch. But I think your consideration is reasonable. A better solution is to directly free bytes, a local variable that holds __get_free_pages return value. If you agree, I will rewrite the patch. Thanks, Pan > > -boris > > > + free_page((unsigned long)map->active.ring); > > return ret; > > } > > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Invalid OEM Table ID: Length cannot exceed 8 characters
Hi,
I wanted to install Xen from source on Fedora 28.
I choose the stable-4.11 branch, compiled it, and got an error at
sudo make install
make[7]: Entering directory
'/home/vagrant/xen/tools/firmware/seabios-dir-remote'
Compiling IASL src/fw/ssdt-misc.hex
out/src/fw/ssdt-misc.dsl.i 4: DefinitionBlock ("ssdt-misc.aml", "SSDT",
0x01, "BXPC", "BXSSDTSUSP", 0x1)
Error6155 -
Invalid OEM Table ID ^ (Length cannot exceed 8 characters)
ASL Input: out/src/fw/ssdt-misc.dsl.i - 102 lines, 2567 bytes, 35 keywords
Listing File: out/src/fw/ssdt-misc.lst - 8393 bytes
Hex Dump: out/src/fw/ssdt-misc.hex - 4096 bytes
I tried to fix by removing 2 characters, but the file
out/src/fw/ssdt-misc.dsl.i is automatically regenerated.
Can anyone explain the problem and propose a fix ?
Thank you.
--
Mathieu Tarral
Sent with ProtonMail Secure Email.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 130765: regressions - trouble: blocked/broken/fail/pass
flight 130765 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/130765/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken build-arm64 broken build-arm64-pvopsbroken build-i386-xsm6 xen-buildfail REGR. vs. 129817 build-i3866 xen-buildfail REGR. vs. 129817 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-ovmf-amd64 16 guest-localmigrate/x10 fail in 130620 pass in 130765 test-armhf-armhf-xl-multivcpu 7 xen-bootfail in 130620 pass in 130765 test-amd64-amd64-examine 4 memdisk-try-append fail pass in 130620 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 15 guest-saverestore.2 fail pass in 130620 Regressions which are regarded as allowable (not blocking): build-arm64 2 hosts-allocate broken REGR. vs. 129817 build-arm64-xsm 2 hosts-allocate broken REGR. vs. 129817 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 129817 Tests which did not succeed, but are not blocking: test-amd64-i386-rumprun-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-livepatch 1 build-check(1) blocked n/a test-amd64-i386-xl-shadow 1 build-check(1) blocked n/a build-i386-rumprun1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-arm64-arm64-examine 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-pvshim 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-migrupgrade 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-examine 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 16 depriv-audit-qemu/create/privcmd blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 17 depriv-audit-qemu/create/gntdev blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 18 depriv-audit-qemu/create/evtchn blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 19 depriv-audi
Re: [Xen-devel] [PATCH] pvcalls-front: fixes incorrect error handling
On 11/21/18 9:07 PM, Pan Bian wrote: > kfree() is incorrectly used to release the pages allocated by > __get_free_page() and __get_free_pages(). Use the matching deallocators > i.e., free_page() and free_pages(), respectively. > > Signed-off-by: Pan Bian > --- > drivers/xen/pvcalls-front.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c > index 2f11ca7..77224d8 100644 > --- a/drivers/xen/pvcalls-front.c > +++ b/drivers/xen/pvcalls-front.c > @@ -385,8 +385,8 @@ static int create_active(struct sock_mapping *map, int > *evtchn) > out_error: > if (*evtchn >= 0) > xenbus_free_evtchn(pvcalls_front_dev, *evtchn); > - kfree(map->active.data.in); > - kfree(map->active.ring); > + free_pages((unsigned long)map->active.data.in, PVCALLS_RING_ORDER); Is map->active.data.in guaranteed to be NULL when entering this routine? -boris > + free_page((unsigned long)map->active.ring); > return ret; > } > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 130819: tolerable all pass - PUSHED
flight 130819 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/130819/ 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 version targeted for testing: xen dc80c424844578048b457730e293a65267dea01c baseline version: xen 901abfef5de149546b16fba6f4d5bd7def08c672 Last test of basis 130289 2018-11-17 11:00:36 Z9 days Failing since130490 2018-11-19 09:00:27 Z7 days 62 attempts Testing same since 130819 2018-11-26 18:00:35 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Dario Faggioli Doug Goldstein Ian Jackson Jan Beulich Julien Grall Kevin Tian Len Brown Norbert Manthey Olaf Hering Paul Durrant Rafael J. Wysocki Razvan Cojocaru Roger Pau Monné Sergey Dyasli Tamas K Lengyel Tim Deegan Wei Liu jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 901abfef5d..dc80c42484 dc80c424844578048b457730e293a65267dea01c -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/x86: add diagnostic printout to xen_mc_flush() in case of error
On 11/23/18 11:24 AM, Juergen Gross wrote:
> Failure of an element of a Xen multicall is signalled via a WARN()
> only unless the kernel is compiled with MC_DEBUG. It is impossible to
s/unless/if
> know which element failed and why it did so.
>
> Change that by printing the related information even without MC_DEBUG,
> even if maybe in some limited form (e.g. without information which
> caller produced the failing element).
>
> Move the printing out of the switch statement in order to have the
> same information for a single call.
>
> Signed-off-by: Juergen Gross
> ---
> arch/x86/xen/multicalls.c | 35 ---
> 1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/xen/multicalls.c b/arch/x86/xen/multicalls.c
> index 2bce7958ce8b..0766a08bdf45 100644
> --- a/arch/x86/xen/multicalls.c
> +++ b/arch/x86/xen/multicalls.c
> @@ -69,6 +69,11 @@ void xen_mc_flush(void)
>
> trace_xen_mc_flush(b->mcidx, b->argidx, b->cbidx);
>
> +#if MC_DEBUG
> + memcpy(b->debug, b->entries,
> +b->mcidx * sizeof(struct multicall_entry));
> +#endif
> +
> switch (b->mcidx) {
> case 0:
> /* no-op */
> @@ -87,32 +92,34 @@ void xen_mc_flush(void)
> break;
>
> default:
> -#if MC_DEBUG
> - memcpy(b->debug, b->entries,
> -b->mcidx * sizeof(struct multicall_entry));
> -#endif
> -
> if (HYPERVISOR_multicall(b->entries, b->mcidx) != 0)
> BUG();
> for (i = 0; i < b->mcidx; i++)
> if (b->entries[i].result < 0)
> ret++;
> + }
>
> + if (WARN_ON(ret)) {
> + pr_err("%d of %d multicall(s) failed: cpu %d\n",
> +ret, b->mcidx, smp_processor_id());
> + for (i = 0; i < b->mcidx; i++) {
> + if (b->entries[i].result < 0) {
> #if MC_DEBUG
> - if (ret) {
> - printk(KERN_ERR "%d multicall(s) failed: cpu %d\n",
> -ret, smp_processor_id());
> - dump_stack();
> - for (i = 0; i < b->mcidx; i++) {
> - printk(KERN_DEBUG " call %2d/%d: op=%lu
> arg=[%lx] result=%ld\t%pF\n",
> -i+1, b->mcidx,
> + pr_err(" call %2d: op=%lu arg=[%lx]
> result=%ld\t%pF\n",
> +i + 1,
> b->debug[i].op,
> b->debug[i].args[0],
> b->entries[i].result,
> b->caller[i]);
> +#else
> + pr_err(" call %2d: op=%lu arg=[%lx]
> result=%ld\n",
> +i + 1,
> +b->entries[i].op,
> +b->entries[i].args[0],
> +b->entries[i].result);
> +#endif
Doesn't (non-debug) hypervisor corrupt op and args?
(Also, we don't really need to print anything when b->entries[i].result
== 0)
-boris
> }
> }
> -#endif
> }
>
> b->mcidx = 0;
> @@ -126,8 +133,6 @@ void xen_mc_flush(void)
> b->cbidx = 0;
>
> local_irq_restore(flags);
> -
> - WARN_ON(ret);
> }
>
> struct multicall_space __xen_mc_entry(size_t args)
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v3 10/14] qdev-props: call object_apply_global_props()
Hi
On Mon, Nov 26, 2018 at 5:27 PM Igor Mammedov wrote:
>
> On Wed, 7 Nov 2018 16:36:48 +0400
> Marc-André Lureau wrote:
>
> > It's now possible to use the common function.
> >
> > Teach object_apply_global_props() to warn if Error argument is NULL.
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> > hw/core/qdev-properties.c | 24 ++--
> > qom/object.c | 6 +-
> > 2 files changed, 7 insertions(+), 23 deletions(-)
> >
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index 8728cbab9f..239535a4cb 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -1223,28 +1223,8 @@ int qdev_prop_check_globals(void)
> >
> > void qdev_prop_set_globals(DeviceState *dev)
> > {
> > -int i;
> > -
> > -for (i = 0; i < global_props()->len; i++) {
> > -GlobalProperty *prop;
> > -Error *err = NULL;
> > -
> > -prop = g_array_index(global_props(), GlobalProperty *, i);
> > -if (object_dynamic_cast(OBJECT(dev), prop->driver) == NULL) {
> > -continue;
> > -}
> > -prop->used = true;
> > -object_property_parse(OBJECT(dev), prop->value, prop->property,
> > &err);
> > -if (err != NULL) {
> > -error_prepend(&err, "can't apply global %s.%s=%s: ",
> > - prop->driver, prop->property, prop->value);
> > -if (!dev->hotplugged) {
> > -error_propagate(&error_fatal, err);
> > -} else {
> > -warn_report_err(err);
> > -}
> > -}
> > -}
> > +object_apply_global_props(OBJECT(dev), global_props(),
> > + dev->hotplugged ? NULL : &error_fatal);
> arguably, it's up to caller to decide it warn or not.
> I'd leave it warning code out of object_apply_global_props() and let caller
> do the job
The problem is that there may be multiple errors, and the remaining
globals should be applied.
I'll add a comment.
> > }
> >
> > /* --- 64bit unsigned int 'size' type --- */
> > diff --git a/qom/object.c b/qom/object.c
> > index 9acdf9e16d..b1a7f70550 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -392,7 +392,11 @@ void object_apply_global_props(Object *obj, GArray
> > *props, Error **errp)
> > if (err != NULL) {
> > error_prepend(&err, "can't apply global %s.%s=%s: ",
> >p->driver, p->property, p->value);
> > -error_propagate(errp, err);
> > +if (errp) {
> > +error_propagate(errp, err);
> > +} else {
> > +warn_report_err(err);
> > +}
> > }
> > }
> > }
>
>
--
Marc-André Lureau
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt test] 130768: regressions - trouble: blocked/broken/fail/pass
flight 130768 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/130768/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken build-arm64 broken build-arm64-pvopsbroken test-armhf-armhf-libvirt-raw 15 guest-start/debian.repeat fail REGR. vs. 129914 Regressions which are regarded as allowable (not blocking): build-arm64-pvops 2 hosts-allocate broken REGR. vs. 129914 build-arm64-xsm 2 hosts-allocate broken REGR. vs. 129914 build-arm64 2 hosts-allocate broken REGR. vs. 129914 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a build-arm64-xsm 3 capture-logs broken blocked in 129914 build-arm64 3 capture-logs broken blocked in 129914 build-arm64-pvops 3 capture-logs broken blocked in 129914 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 129914 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 129914 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail 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-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-amd64-amd64-libvirt-vhd 12 migrate-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 fd54e4fdc82050f97cbf48add3975db7e2ca09be baseline version: libvirt f1e8d2f09a4329641825b0c0e784d8e339dd71ea Last test of basis 129914 2018-11-13 04:23:30 Z 13 days Failing since130113 2018-11-15 12:20:30 Z 11 days8 attempts Testing same since 130768 2018-11-24 13:06:25 Z2 days1 attempts People who touched revisions under test: Andrea Bolognani Christian Ehrhardt Daniel Henrique Barboza Daniel P. Berrangé Erik Skultety Jim Fehlig John Ferlan Julio Faracco Marc Hartmayer Marc-André Lureau Martin Kletzander Michal Privoznik Nikolay Shirokovskiy Pavel Hrdina Peter Chubb Vitaly Kuznetsov Wang Huaqiang Wang Yechao Yi Min Zhao ZhiPeng Lu jobs: build-amd64-xsm pass build-arm64-xsm broken build-i386-xsm pass build-amd64 pass build-arm64 broken build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt blocked build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopsbroken 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 blocked test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt blocked 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
Re: [Xen-devel] [PATCH v3] xen/balloon: Mark unallocated host memory as UNUSABLE
On 26/11/2018 19:42, Boris Ostrovsky wrote:
> On 11/26/18 12:10 PM, Igor Druzhinin wrote:
>> On 26/11/2018 16:25, Boris Ostrovsky wrote:
>>> On 11/25/18 8:00 PM, Igor Druzhinin wrote:
On 20/12/2017 14:05, Boris Ostrovsky wrote:
> Commit f5775e0b6116 ("x86/xen: discard RAM regions above the maximum
> reservation") left host memory not assigned to dom0 as available for
> memory hotplug.
>
> Unfortunately this also meant that those regions could be used by
> others. Specifically, commit fa564ad96366 ("x86/PCI: Enable a 64bit BAR
> on AMD Family 15h (Models 00-1f, 30-3f, 60-7f)") may try to map those
> addresses as MMIO.
>
> To prevent this mark unallocated host memory as E820_TYPE_UNUSABLE (thus
> effectively reverting f5775e0b6116) and keep track of that region as
> a hostmem resource that can be used for the hotplug.
>
> Signed-off-by: Boris Ostrovsky
This commit breaks Xen balloon memory hotplug for us in Dom0 with
"hoplug_unpopulated" set to 1. The issue is that the common kernel
memory onlining procedures require "System RAM" resource to be 1-st
level. That means by inserting it under "Unusable memory" as the commit
above does (intentionally or not) we make it 2-nd level and break memory
onlining.
>>> What do you mean by 1st and 2nd level?
>>>
>> I mean the level of a resource in IOMEM tree (the one that's printed
>> from /proc/iomem). 1-st level means its parent is root and so on.
>
> Ah, OK. Doesn't
> additional_memory_resource()->insert_resource(iomem_resource) place the
> RAM at 1st level? And if not, can we make it so?
>
That'd mean splitting "Unusable memory" resource. Since it's allocated
from bootmem it has proven to be quite difficult but there are seem to
be special functions available particularly for memory resource
management operations that I've not yet experimented with. So the answer
is probably - maybe yes but not straightforward.
>>
There are multiple ways to fix it depending on what was the intention of
original commit and what exactly it tried to workaround. It seems it
does several things at once:
1) Marks non-Dom0 host memory "Unusable memory" in resource tree.
2) Keeps track of all the areas safe for hotplug in Dom0
3) Changes allocation algorithms itself in balloon driver to use those
areas
>>> Pretty much. (3) is true in the sense that memory is first allocated
>>> from hostmem_resource (which is non-dom0 RAM).
>>>
Are all the things above necessary to cover the issue in fa564ad96366
("x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 00-1f, 30-3f,
60-7f)")?
>>> Not anymore, as far as that particular commit is concerned, but that's
>>> because of 03a551734 ("x86/PCI: Move and shrink AMD 64-bit window to
>>> avoid conflict") which was introduced after balloon patch. IIRC there
>>> were some issues with fa564ad96366 unrelated to balloon.
>>>
>> If it's not a problem anymore IIUC, can we revert the change as it still
>> breaks "hotplug_unpopulated=1" for the reasons I described above?
>
> Since this seems to have broken existing feature this would be an
> option. But before going that route I'd like to see if we can fix the patch.
>
> I have been unable to reproduce your problem. Can you describe what you did?
>
It doesn't happen on all configurations as sometimes the memory is
successfully hotplugged to a hole depending on the size of Dom0 memory.
But we reproduced it quite reliably with small Dom0 sizes like 752MB.
XenServer is using this feature to hotplug additional memory for grant
table operations so we started a VM and observed a stable hang.
>
>>
Can we remove "Unusable memory" resources as soon as we finished
booting? Is removing on-demand is preferable over "shoot them all" in
that case?
>>> The concern is that in principle nothing prevents someone else to do
>>> exact same thing fa564ad96366 did, which is grab something from right
>>> above end of RAM as the kernel sees it. And that can be done at any point.
>>>
>> Nothing prevents - true, but that's plainly wrong from OS point of view
>> to grab physical ranges for something without knowing what's actually
>> behind on that platform.
>
> I am not sure I agree that this is plainly wrong. If not for BIOS issues
> that 03a551734cf mentions I think what the original implementation of
> fa564ad963 did was perfectly reasonable. Which is why I would prefer to
> keep keep the hostmem resource *if possible*.
>
Exactly, those *are* BIOS issues and are not supposed to be workarounded
by the OS. And as the next commit showed even the workaround didn't
quite helped with it.
I agree that having hotmem as a precaution is fine but only if there is
a non-cringy way to keep things working with it which I'm not sure does
exist.
Igor
>
> -boris
>
>
>> I think we shouldn't consider this as a valid
>> thing to do and don't try to workaround initially incorrect co
Re: [Xen-devel] [PATCH v3] xen/balloon: Mark unallocated host memory as UNUSABLE
On 11/26/18 12:10 PM, Igor Druzhinin wrote:
> On 26/11/2018 16:25, Boris Ostrovsky wrote:
>> On 11/25/18 8:00 PM, Igor Druzhinin wrote:
>>> On 20/12/2017 14:05, Boris Ostrovsky wrote:
Commit f5775e0b6116 ("x86/xen: discard RAM regions above the maximum
reservation") left host memory not assigned to dom0 as available for
memory hotplug.
Unfortunately this also meant that those regions could be used by
others. Specifically, commit fa564ad96366 ("x86/PCI: Enable a 64bit BAR
on AMD Family 15h (Models 00-1f, 30-3f, 60-7f)") may try to map those
addresses as MMIO.
To prevent this mark unallocated host memory as E820_TYPE_UNUSABLE (thus
effectively reverting f5775e0b6116) and keep track of that region as
a hostmem resource that can be used for the hotplug.
Signed-off-by: Boris Ostrovsky
>>> This commit breaks Xen balloon memory hotplug for us in Dom0 with
>>> "hoplug_unpopulated" set to 1. The issue is that the common kernel
>>> memory onlining procedures require "System RAM" resource to be 1-st
>>> level. That means by inserting it under "Unusable memory" as the commit
>>> above does (intentionally or not) we make it 2-nd level and break memory
>>> onlining.
>> What do you mean by 1st and 2nd level?
>>
> I mean the level of a resource in IOMEM tree (the one that's printed
> from /proc/iomem). 1-st level means its parent is root and so on.
Ah, OK. Doesn't
additional_memory_resource()->insert_resource(iomem_resource) place the
RAM at 1st level? And if not, can we make it so?
>
>>> There are multiple ways to fix it depending on what was the intention of
>>> original commit and what exactly it tried to workaround. It seems it
>>> does several things at once:
>>> 1) Marks non-Dom0 host memory "Unusable memory" in resource tree.
>>> 2) Keeps track of all the areas safe for hotplug in Dom0
>>> 3) Changes allocation algorithms itself in balloon driver to use those areas
>> Pretty much. (3) is true in the sense that memory is first allocated
>> from hostmem_resource (which is non-dom0 RAM).
>>
>>> Are all the things above necessary to cover the issue in fa564ad96366
>>> ("x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 00-1f, 30-3f,
>>> 60-7f)")?
>> Not anymore, as far as that particular commit is concerned, but that's
>> because of 03a551734 ("x86/PCI: Move and shrink AMD 64-bit window to
>> avoid conflict") which was introduced after balloon patch. IIRC there
>> were some issues with fa564ad96366 unrelated to balloon.
>>
> If it's not a problem anymore IIUC, can we revert the change as it still
> breaks "hotplug_unpopulated=1" for the reasons I described above?
Since this seems to have broken existing feature this would be an
option. But before going that route I'd like to see if we can fix the patch.
I have been unable to reproduce your problem. Can you describe what you did?
>
>>> Can we remove "Unusable memory" resources as soon as we finished
>>> booting? Is removing on-demand is preferable over "shoot them all" in
>>> that case?
>> The concern is that in principle nothing prevents someone else to do
>> exact same thing fa564ad96366 did, which is grab something from right
>> above end of RAM as the kernel sees it. And that can be done at any point.
>>
> Nothing prevents - true, but that's plainly wrong from OS point of view
> to grab physical ranges for something without knowing what's actually
> behind on that platform.
I am not sure I agree that this is plainly wrong. If not for BIOS issues
that 03a551734cf mentions I think what the original implementation of
fa564ad963 did was perfectly reasonable. Which is why I would prefer to
keep keep the hostmem resource *if possible*.
-boris
> I think we shouldn't consider this as a valid
> thing to do and don't try to workaround initially incorrect code.
>
>> -boris
>>
>>> Does it even make sense to remove the 1-st level only restriction in
>>> kernel/resource.c ?
>>
>>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.10-testing test] 130758: regressions - trouble: blocked/broken/fail/pass
flight 130758 xen-4.10-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/130758/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken build-arm64 broken build-arm64-pvopsbroken test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stopfail REGR. vs. 129676 Tests which are failing intermittently (not blocking): test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail in 130667 pass in 130758 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail pass in 130667 Regressions which are regarded as allowable (not blocking): build-arm64-xsm 2 hosts-allocate broken REGR. vs. 129676 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 129676 build-arm64 2 hosts-allocate broken REGR. vs. 129676 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a build-arm64-pvops 3 capture-logs broken blocked in 129676 build-arm64-xsm 3 capture-logs broken blocked in 129676 build-arm64 3 capture-logs broken blocked in 129676 test-xtf-amd64-amd64-5 69 xtf/test-hvm64-xsa-278 fail blocked in 129676 test-xtf-amd64-amd64-2 69 xtf/test-hvm64-xsa-278 fail in 130667 blocked in 129676 test-xtf-amd64-amd64-4 69 xtf/test-hvm64-xsa-278 fail in 130667 blocked in 129676 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 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 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-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-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 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-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail 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-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-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
[Xen-devel] [PATCH v2] mm: make opt_bootscrub non-init
LLVM code generation can attempt to load from a variable in the next
condition of an expression under certain circumstances, thus turning
the following condition:
if ( system_state < SYS_STATE_active && opt_bootscrub == BOOTSCRUB_IDLE )
Into:
0x82d080223967 <+103>: cmpl $0x3,0x37b032(%rip) # 0x82d08059e9a0
0x82d08022396e <+110>: setb -0x29(%rbp)
0x82d080223972 <+114>: cmpl $0x2,0x228a8b(%rip) # 0x82d08044c404
Such code will trigger a page fault if system_state >=
SYS_STATE_active because opt_bootscrub will be unmapped.
Fix this by making opt_bootscrub non-init, thus preventing the page
fault. The LLVM bug with the discussion about this issue can be found
at:
https://bugs.llvm.org/show_bug.cgi?id=39707
I haven't been able to find any other instances of such conditional
expression that uses system_state together with an init variable or
function.
Signed-off-by: Roger Pau Monné
Reviewed-by: Sergey Dyasli
Acked-by: Andrew Cooper
Acked-by: Julien Grall
Acked-by: Wei Liu
---
Cc: Andrew Cooper
Cc: George Dunlap
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Julien Grall
Cc: Konrad Rzeszutek Wilk
Cc: Stefano Stabellini
Cc: Tim Deegan
Cc: Wei Liu
Cc: Sergey Dyasli
---
Changes since v1:
- Make opt_bootscrub read mostly.
- Add a comment about why opt_bootscrub is not in the init section.
---
xen/common/page_alloc.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 08ee8cfbb9..b4086781c4 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -166,7 +166,14 @@ enum bootscrub_mode {
BOOTSCRUB_ON,
BOOTSCRUB_IDLE,
};
-static enum bootscrub_mode __initdata opt_bootscrub = BOOTSCRUB_IDLE;
+/*
+ * opt_bootscrub should live in the init section, since it's not accessed
+ * afterwards. However at least LLVM assumes there are no side effects of
+ * accessing the variable, and optimizes the condition so opt_bootscrub is
+ * read regardless of the value of system_state:
+ * https://bugs.llvm.org/show_bug.cgi?id=39707
+ */
+static enum bootscrub_mode __read_mostly opt_bootscrub = BOOTSCRUB_IDLE;
static int __init parse_bootscrub_param(const char *s)
{
/* Interpret 'bootscrub' alone in its positive boolean form */
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 00/14] XSA-277 followup
On Wed, Nov 21, 2018 at 5:08 PM Andrew Cooper wrote:
>
> On 21/11/2018 22:42, Tamas K Lengyel wrote:
> > On Wed, Nov 21, 2018 at 2:22 PM Andrew Cooper
> > wrote:
> >> On 21/11/2018 17:19, Tamas K Lengyel wrote:
> >>> On Wed, Nov 21, 2018 at 6:21 AM Andrew Cooper
> >>> wrote:
> This covers various fixes related to XSA-277 which weren't in security
> supported areas, and associated cleanup.
>
> The biggest issue noticed here is that altp2m's use of hardware #VE
> support
> will cause general memory corruption if the guest ever balloons out the
> VEINFO
> page. The only safe way I think of doing this is for Xen to alloc
> annonymous
> domheap pages for the VEINFO, and for the guest to map them in a similar
> way
> to the shared info and grant table frames.
> >>> Since ballooning presents all sorts of problems when used with altp2m
> >>> I would suggest just making the two explicitly incompatible during
> >>> domain creation. Beside the info page being possibly ballooned out the
> >>> other problem is when ballooning causes altp2m views to be reset
> >>> completely, removing mem_access permissions and remapped entries.
> >> If only it were that simple.
> >>
> >> For reasons of history and/or poor terminology, "ballooning" means two
> >> things.
> >>
> >> 1) The act of the Toolstack interacting with the balloon driver inside a
> >> VM, to change the current amount of RAM used by the guest.
> >>
> >> 2) XENMEM_{increase,decrease}_reservation which are the underlying
> >> hypercalls used by guest kernels.
> >>
> >> For the toolstack interaction side of things, this is a mess. There is
> >> a single xenstore key, and a blind assumption that all guests know what
> >> changes to memory/target mean. There is no negotiation of whether a
> >> balloon driver is running in the guest, and if one is running, there is
> >> no ability for the balloon driver to nack a request it can't fulfil.
> >> The sole feedback mechanism which exists is the toolstack looking to see
> >> whether the domain has changed the amount of RAM it is using.
> >>
> >> PV guests are fairly "special" by any reasonable judgement. They are
> >> fully aware of their memory layout , an of changes to it across
> >> migrate. "Ballooning" was implemented at a time when most computers had
> >> MB of RAM rather than GB, and the knowledge a PV guest had was "I've got
> >> a random set of MFNs which aren't currently used by anything important,
> >> and can be handed back to Xen on request. Xen guests also have shared
> >> memory constructs such as the shared_info page, and grant tables. A PV
> >> guest gets access to these by programming the frame straight into to the
> >> pagetables, and Xen's permission model DTRT.
> >>
> >> Then HVM guests came along. For reasons of trying to get things
> >> working, they inherited a lot of same interfaces as PV guests, despite
> >> the fundamental differences in the way they work. One of the biggest
> >> differences was the fact that HVM guests have their gfn=>mfn space
> >> managed by Xen rather than themselves, and in particular, you can no
> >> longer map shared memory structures in the PV way.
> >>
> >> For a shared memory structure to be usable, a mapping has to be put into
> >> the guests P2M, so the guest can create a regular pagetable entry
> >> pointing at it. For reasons which are beyond me, Xen doesn't have any
> >> knowledge of the guests physical layout, and guests arbitrary mutative
> >> capabilities on their GFN space, but with a hypercall set that has
> >> properties such as a return value of "how many items of this batch
> >> succeeded", and replacement properties rather than error properties when
> >> trying to modify a GFN which already has something in it.
> >>
> >> Whatever the reasons, it is commonplace for guests to
> >> decrease_reservation out some RAM to create holes for the shared memory
> >> mappings, because it is the only safe way to avoid irreparably
> >> clobbering something else (especially if you're HVMLoader and in charge
> >> of trying to construct the E820/ACPI tables).
> >>
> >> tl;dr If you actually prohibit XENMEM_decrease_reservation, HVM guests
> >> don't boot, and that's long before a balloon driver gets up and running.
> > Thanks for the detailed write-up. This explains why I could never get
> > altp2m working from domain start, no matter where in the startup logic
> > of the toolstack I placed the altp2m activation (had to resort to
> > activating altp2m settings only after I detect the guest OS is fully
> > booted and things have settled down).
>
> So, in theory it should all work, even from the start.
>
> In practice, the implementation quality of altp2m leaves a lot to be
> desired, and it was designed to have the "all logic inside the guest"
> model, which in practice means that it only ever started once the guest
> had come up sufficiently.
>
> Do you recall more specifically where you tried insert
[Xen-devel] [PATCH] amd-iommu: remove page merging code
The page merging logic makes use of bits 1-8 and bit 63 of a PTE, which used
to be specified as ignored. However, bits 5 and 6 are now specified as
'accessed' and 'dirty' bits and their use only remains safe as long as
the DTE 'Host Access Dirty' bits remain clear. The code is also of dubious
benefit and was the subject XSA-275.
This patch removes the code, freeing up the remaining PTE 'ignored' bits
for other potential use and shortening the source by 170 lines.
Signed-off-by: Paul Durrant
---
Cc: Suravee Suthikulpanit
Cc: Brian Woods
---
xen/drivers/passthrough/amd/iommu_map.c | 172 +---
1 file changed, 1 insertion(+), 171 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_map.c
b/xen/drivers/passthrough/amd/iommu_map.c
index 0ac3f473b3..c4dbd96227 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -323,134 +323,6 @@ uint64_t amd_iommu_get_address_from_pte(void *pte)
return ptr;
}
-/* For each pde, We use ignored bits (bit 1 - bit 8 and bit 63)
- * to save pde count, pde count = 511 is a candidate of page coalescing.
- */
-static unsigned int get_pde_count(uint64_t pde)
-{
-unsigned int count;
-uint64_t upper_mask = 1ULL << 63 ;
-uint64_t lower_mask = 0xFF << 1;
-
-count = ((pde & upper_mask) >> 55) | ((pde & lower_mask) >> 1);
-return count;
-}
-
-/* Convert pde count into iommu pte ignored bits */
-static void set_pde_count(uint64_t *pde, unsigned int count)
-{
-uint64_t upper_mask = 1ULL << 8 ;
-uint64_t lower_mask = 0xFF;
-uint64_t pte_mask = (~(1ULL << 63)) & (~(0xFF << 1));
-
-*pde &= pte_mask;
-*pde |= ((count & upper_mask ) << 55) | ((count & lower_mask ) << 1);
-}
-
-/* Return 1, if pages are suitable for merging at merge_level.
- * otherwise increase pde count if mfn is contigous with mfn - 1
- */
-static bool iommu_update_pde_count(struct domain *d, unsigned long pt_mfn,
- unsigned long dfn, unsigned long mfn,
- unsigned int merge_level)
-{
-unsigned int pde_count, next_level;
-unsigned long first_mfn;
-uint64_t *table, *pde, *ntable;
-uint64_t ntable_maddr, mask;
-struct domain_iommu *hd = dom_iommu(d);
-bool ok = false;
-
-ASSERT( spin_is_locked(&hd->arch.mapping_lock) && pt_mfn );
-
-next_level = merge_level - 1;
-
-/* get pde at merge level */
-table = map_domain_page(_mfn(pt_mfn));
-pde = table + pfn_to_pde_idx(dfn, merge_level);
-
-/* get page table of next level */
-ntable_maddr = amd_iommu_get_address_from_pte(pde);
-ntable = map_domain_page(_mfn(paddr_to_pfn(ntable_maddr)));
-
-/* get the first mfn of next level */
-first_mfn = amd_iommu_get_address_from_pte(ntable) >> PAGE_SHIFT;
-
-if ( first_mfn == 0 )
-goto out;
-
-mask = (1ULL<< (PTE_PER_TABLE_SHIFT * next_level)) - 1;
-
-if ( ((first_mfn & mask) == 0) &&
- (((dfn & mask) | first_mfn) == mfn) )
-{
-pde_count = get_pde_count(*pde);
-
-if ( pde_count == (PTE_PER_TABLE_SIZE - 1) )
-ok = true;
-else if ( pde_count < (PTE_PER_TABLE_SIZE - 1))
-{
-pde_count++;
-set_pde_count(pde, pde_count);
-}
-}
-
-else
-/* non-contiguous mapping */
-set_pde_count(pde, 0);
-
-out:
-unmap_domain_page(ntable);
-unmap_domain_page(table);
-
-return ok;
-}
-
-static int iommu_merge_pages(struct domain *d, unsigned long pt_mfn,
- unsigned long dfn, unsigned int flags,
- unsigned int merge_level)
-{
-uint64_t *table, *pde, *ntable;
-uint64_t ntable_mfn;
-unsigned long first_mfn;
-struct domain_iommu *hd = dom_iommu(d);
-
-ASSERT( spin_is_locked(&hd->arch.mapping_lock) && pt_mfn );
-
-table = map_domain_page(_mfn(pt_mfn));
-pde = table + pfn_to_pde_idx(dfn, merge_level);
-
-/* get first mfn */
-ntable_mfn = amd_iommu_get_address_from_pte(pde) >> PAGE_SHIFT;
-
-if ( ntable_mfn == 0 )
-{
-unmap_domain_page(table);
-return 1;
-}
-
-ntable = map_domain_page(_mfn(ntable_mfn));
-first_mfn = amd_iommu_get_address_from_pte(ntable) >> PAGE_SHIFT;
-
-if ( first_mfn == 0 )
-{
-unmap_domain_page(ntable);
-unmap_domain_page(table);
-return 1;
-}
-
-/* setup super page mapping, next level = 0 */
-set_iommu_pde_present((uint32_t *)pde, first_mfn, 0,
- !!(flags & IOMMUF_writable),
- !!(flags & IOMMUF_readable));
-
-amd_iommu_flush_all_pages(d);
-
-unmap_domain_page(ntable);
-unmap_domain_page(table);
-return 0;
-}
-
/* Walk io page tables and build level page tables if necessary
* {Re, un}mapping super page frames causes re-allocation of io
* page tables.
@@ -656,7 +528,6 @@ int amd_iommu_map_page(struct domain *d, dfn_t d
[Xen-devel] [xen-unstable-smoke test] 130814: regressions - FAIL
flight 130814 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/130814/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 130289 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386 1 build-check(1) blocked n/a test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen d1e70f9b19287aa8763f7778d8e8b2dbee990db3 baseline version: xen 901abfef5de149546b16fba6f4d5bd7def08c672 Last test of basis 130289 2018-11-17 11:00:36 Z9 days Failing since130490 2018-11-19 09:00:27 Z7 days 61 attempts Testing same since 130814 2018-11-26 15:01:04 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Dario Faggioli Doug Goldstein Ian Jackson Jan Beulich Julien Grall Kevin Tian Len Brown Norbert Manthey Olaf Hering Paul Durrant Rafael J. Wysocki Razvan Cojocaru Roger Pau Monné Sergey Dyasli Tamas K Lengyel Tim Deegan Wei Liu jobs: build-amd64 fail build-armhf pass build-amd64-libvirt blocked test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 blocked test-amd64-amd64-libvirt blocked sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 1328 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] xen/balloon: Mark unallocated host memory as UNUSABLE
On 26/11/2018 16:25, Boris Ostrovsky wrote:
> On 11/25/18 8:00 PM, Igor Druzhinin wrote:
>> On 20/12/2017 14:05, Boris Ostrovsky wrote:
>>> Commit f5775e0b6116 ("x86/xen: discard RAM regions above the maximum
>>> reservation") left host memory not assigned to dom0 as available for
>>> memory hotplug.
>>>
>>> Unfortunately this also meant that those regions could be used by
>>> others. Specifically, commit fa564ad96366 ("x86/PCI: Enable a 64bit BAR
>>> on AMD Family 15h (Models 00-1f, 30-3f, 60-7f)") may try to map those
>>> addresses as MMIO.
>>>
>>> To prevent this mark unallocated host memory as E820_TYPE_UNUSABLE (thus
>>> effectively reverting f5775e0b6116) and keep track of that region as
>>> a hostmem resource that can be used for the hotplug.
>>>
>>> Signed-off-by: Boris Ostrovsky
>> This commit breaks Xen balloon memory hotplug for us in Dom0 with
>> "hoplug_unpopulated" set to 1. The issue is that the common kernel
>> memory onlining procedures require "System RAM" resource to be 1-st
>> level. That means by inserting it under "Unusable memory" as the commit
>> above does (intentionally or not) we make it 2-nd level and break memory
>> onlining.
>
> What do you mean by 1st and 2nd level?
>
I mean the level of a resource in IOMEM tree (the one that's printed
from /proc/iomem). 1-st level means its parent is root and so on.
>>
>> There are multiple ways to fix it depending on what was the intention of
>> original commit and what exactly it tried to workaround. It seems it
>> does several things at once:
>> 1) Marks non-Dom0 host memory "Unusable memory" in resource tree.
>> 2) Keeps track of all the areas safe for hotplug in Dom0
>> 3) Changes allocation algorithms itself in balloon driver to use those areas
>
> Pretty much. (3) is true in the sense that memory is first allocated
> from hostmem_resource (which is non-dom0 RAM).
>
>>
>> Are all the things above necessary to cover the issue in fa564ad96366
>> ("x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 00-1f, 30-3f,
>> 60-7f)")?
>
> Not anymore, as far as that particular commit is concerned, but that's
> because of 03a551734 ("x86/PCI: Move and shrink AMD 64-bit window to
> avoid conflict") which was introduced after balloon patch. IIRC there
> were some issues with fa564ad96366 unrelated to balloon.
>
If it's not a problem anymore IIUC, can we revert the change as it still
breaks "hotplug_unpopulated=1" for the reasons I described above?
>
>>
>> Can we remove "Unusable memory" resources as soon as we finished
>> booting? Is removing on-demand is preferable over "shoot them all" in
>> that case?
>
> The concern is that in principle nothing prevents someone else to do
> exact same thing fa564ad96366 did, which is grab something from right
> above end of RAM as the kernel sees it. And that can be done at any point.
>
Nothing prevents - true, but that's plainly wrong from OS point of view
to grab physical ranges for something without knowing what's actually
behind on that platform. I think we shouldn't consider this as a valid
thing to do and don't try to workaround initially incorrect code.
>
> -boris
>
>>
>> Does it even make sense to remove the 1-st level only restriction in
>> kernel/resource.c ?
>
>
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
On 26/11/2018 16:17, Juergen Gross wrote: On 26/11/2018 17:01, Julien Grall wrote: On 26/11/2018 15:29, Juergen Gross wrote: On 26/11/2018 15:58, Jan Beulich wrote: On 26.11.18 at 15:23, wrote: On 26/11/2018 15:01, Jan Beulich wrote: On 26.11.18 at 14:52, wrote: I don't think the hypervisor should explicitly try to make it as hard as possible for the guest to find problems in the code. That's indeed not the hypervisor's goal. Instead it tries to make it as hard as possible for the guest (developer) to make wrong assumptions. Let's look at the current example why I wrote this patch: The Linux kernel's use of multicalls should never trigger any single call to return an error (return value < 0). A kernel compiled for productive use will catch such errors, but has no knowledge which single call has failed, as it doesn't keep track of the single entries (non-productive kernels have an option available in the respective source to copy the entries before doing the multicall in order to have some diagnostic data available in case of such an error). Catching an error from a multicall right now means a WARN() with a stack backtrace (for the multicall itself, not for the entry causing the error). I have a customer report for a case where such a backtrace was produced and a kernel crash some seconds later, obviously due to illegally unmapped memory pages resulting from the failed multicall. Unfortunately there are multiple possibilities what might have gone wrong and I don't know which one was the culprit. The problem can't be a very common one, because there is only one such report right now, which might depend on a special driver. Finding this bug without a known reproducer and the current amount of diagnostic data is next to impossible. So I'd like to have more data available without having to hurt performance for the 99.99% of the cases where nothing bad happens. In case you have an idea how to solve this problem in another way I'd be happy to follow that route. I'd really like to be able to have a better clue in case such an error occurs in future. Since you have a production kernel, I assume you also have a production hypervisor. This hypervisor doesn't clobber the arguments if I'm not mistaken. Therefore - in the debugging scenario you (can) have all data available by virtue of the information getting copied in the kernel, - in the release scenario you have all data available since it's left un-clobbered. Am I missing anything (I don't view mixed debug/release setups of kernel and hypervisor as overly important here)? No, you are missing nothing here. OTOH a debug hypervisor destroying debug data is kind of weird, so I posted this patch. This is a quite common approach if you want to enforce the other entity to not rely on some fields. This also follows what we do for hypercalls (at least on Arm). I don't question that general mechanism. What I question is doing the clobbering in this case where the caller would need to take special measures for copying the needed debug data which will hit performance. So not doing the clobbering would only be in the very rare error case, not in the common case where the guest should really have no need to see the preserved data. I really fail to see why it is so bad to not clobber data in a case which normally should never occur. The only outcome of clobbering the data in the error case is making diagnosis of that error much harder. Its not as if there would be secret data suddenly made available to the guest. Its just avoiding the need for the guest to copy the data for each multicall for the very unlikely chance an error might occur. And we are not speaking of a hypercall issued then and now, but of the path hit for nearly every memory-management action and context switch of PV guests. So doing always the copy would really be visible. For a first, now the behavior will not be the same as when handling a single hypercall. I would much prefer if the behavior is kept the same everywhere. Secondly, you are introducing an ABI change without explicitly telling it. This should be clarified in the public interface and probably the multicall code to avoid re-introducing the clobbering in the future. But I am concerned that the conditional clobbering (i.e depending on the return) will be overlooked by the guest and will probably introduce some more interesting^weird bug. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
>>> On 26.11.18 at 17:17, wrote: > I really fail to see why it is so bad to not clobber data in a case > which normally should never occur. See Andrew's original reply. You're also clobbering on potential success paths. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] xen/balloon: Mark unallocated host memory as UNUSABLE
On 11/25/18 8:00 PM, Igor Druzhinin wrote:
> On 20/12/2017 14:05, Boris Ostrovsky wrote:
>> Commit f5775e0b6116 ("x86/xen: discard RAM regions above the maximum
>> reservation") left host memory not assigned to dom0 as available for
>> memory hotplug.
>>
>> Unfortunately this also meant that those regions could be used by
>> others. Specifically, commit fa564ad96366 ("x86/PCI: Enable a 64bit BAR
>> on AMD Family 15h (Models 00-1f, 30-3f, 60-7f)") may try to map those
>> addresses as MMIO.
>>
>> To prevent this mark unallocated host memory as E820_TYPE_UNUSABLE (thus
>> effectively reverting f5775e0b6116) and keep track of that region as
>> a hostmem resource that can be used for the hotplug.
>>
>> Signed-off-by: Boris Ostrovsky
> This commit breaks Xen balloon memory hotplug for us in Dom0 with
> "hoplug_unpopulated" set to 1. The issue is that the common kernel
> memory onlining procedures require "System RAM" resource to be 1-st
> level. That means by inserting it under "Unusable memory" as the commit
> above does (intentionally or not) we make it 2-nd level and break memory
> onlining.
What do you mean by 1st and 2nd level?
>
> There are multiple ways to fix it depending on what was the intention of
> original commit and what exactly it tried to workaround. It seems it
> does several things at once:
> 1) Marks non-Dom0 host memory "Unusable memory" in resource tree.
> 2) Keeps track of all the areas safe for hotplug in Dom0
> 3) Changes allocation algorithms itself in balloon driver to use those areas
Pretty much. (3) is true in the sense that memory is first allocated
from hostmem_resource (which is non-dom0 RAM).
>
> Are all the things above necessary to cover the issue in fa564ad96366
> ("x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 00-1f, 30-3f,
> 60-7f)")?
Not anymore, as far as that particular commit is concerned, but that's
because of 03a551734 ("x86/PCI: Move and shrink AMD 64-bit window to
avoid conflict") which was introduced after balloon patch. IIRC there
were some issues with fa564ad96366unrelated to balloon.
>
> Can we remove "Unusable memory" resources as soon as we finished
> booting? Is removing on-demand is preferable over "shoot them all" in
> that case?
The concern is that in principle nothing prevents someone else to do
exact same thing fa564ad96366 did, which is grab something from right
above end of RAM as the kernel sees it. And that can be done at any point.
-boris
>
> Does it even make sense to remove the 1-st level only restriction in
> kernel/resource.c ?
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86emul: suppress default test harness build with incapable assembler
On Mon, Nov 26, 2018 at 08:50:14AM -0700, Jan Beulich wrote:
> A top level "make build", as used e.g. by osstest, wants to build all
> "all" targets in enabled tools subdirectories, which by default also
> includes the emulator test harness. The use of, in particular, {evex}
> insn pseudo-prefixes in, again in particular, test_x86_emulator.c causes
> this build to fail though when the assembler is not new enough. Take
> another big hammer and suppress the default harness build altogether
> also when this and other pseudo-prefixes are not supported by the
> specified (or defaulted to) assembler.
>
> Signed-off-by: Jan Beulich
Acked-by: Wei Liu
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
On 26/11/2018 17:01, Julien Grall wrote: > > > On 26/11/2018 15:29, Juergen Gross wrote: >> On 26/11/2018 15:58, Jan Beulich wrote: >> On 26.11.18 at 15:23, wrote: On 26/11/2018 15:01, Jan Beulich wrote: On 26.11.18 at 14:52, wrote: >> I don't think the hypervisor should explicitly try to make it as >> hard as >> possible for the guest to find problems in the code. > > That's indeed not the hypervisor's goal. Instead it tries to make > it as hard as possible for the guest (developer) to make wrong > assumptions. Let's look at the current example why I wrote this patch: The Linux kernel's use of multicalls should never trigger any single call to return an error (return value < 0). A kernel compiled for productive use will catch such errors, but has no knowledge which single call has failed, as it doesn't keep track of the single entries (non-productive kernels have an option available in the respective source to copy the entries before doing the multicall in order to have some diagnostic data available in case of such an error). Catching an error from a multicall right now means a WARN() with a stack backtrace (for the multicall itself, not for the entry causing the error). I have a customer report for a case where such a backtrace was produced and a kernel crash some seconds later, obviously due to illegally unmapped memory pages resulting from the failed multicall. Unfortunately there are multiple possibilities what might have gone wrong and I don't know which one was the culprit. The problem can't be a very common one, because there is only one such report right now, which might depend on a special driver. Finding this bug without a known reproducer and the current amount of diagnostic data is next to impossible. So I'd like to have more data available without having to hurt performance for the 99.99% of the cases where nothing bad happens. In case you have an idea how to solve this problem in another way I'd be happy to follow that route. I'd really like to be able to have a better clue in case such an error occurs in future. >>> >>> Since you have a production kernel, I assume you also have a >>> production hypervisor. This hypervisor doesn't clobber the >>> arguments if I'm not mistaken. Therefore >>> - in the debugging scenario you (can) have all data available by >>> virtue of the information getting copied in the kernel, >>> - in the release scenario you have all data available since it's >>> left un-clobbered. >>> Am I missing anything (I don't view mixed debug/release setups >>> of kernel and hypervisor as overly important here)? >> >> No, you are missing nothing here. OTOH a debug hypervisor destroying >> debug data is kind of weird, so I posted this patch. > > This is a quite common approach if you want to enforce the other entity > to not rely on some fields. This also follows what we do for hypercalls > (at least on Arm). I don't question that general mechanism. What I question is doing the clobbering in this case where the caller would need to take special measures for copying the needed debug data which will hit performance. So not doing the clobbering would only be in the very rare error case, not in the common case where the guest should really have no need to see the preserved data. I really fail to see why it is so bad to not clobber data in a case which normally should never occur. The only outcome of clobbering the data in the error case is making diagnosis of that error much harder. Its not as if there would be secret data suddenly made available to the guest. Its just avoiding the need for the guest to copy the data for each multicall for the very unlikely chance an error might occur. And we are not speaking of a hypercall issued then and now, but of the path hit for nearly every memory-management action and context switch of PV guests. So doing always the copy would really be visible. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] Increase framebuffer size to todays standards
>>> On 26.11.18 at 17:03, wrote:
> Am Mon, 26 Nov 2018 03:31:27 -0700
> schrieb "Jan Beulich" :
>
>> And I think a change like this should (a) address the more general
>> case rather than just your laptop (or laptops in general) and (b)
>> actually add some headroom. Hence at the very least I'd see us
>> go to 4096x3072. WHUXGA would even call for 7680x4800.
>
> So should I resend this patch with higher values, or should I remove
> the bounds check entirely? Not sure what it is trying to achieve, the
> framebuffer may fail either way if the BIOS provides bogus values.
I have to forward this question: Stefano introduced all five MAX_*
values here when splitting out the LFB code in commit e7cb35e8b1
("xen: introduce a generic framebuffer driver"). I apparently didn't
even notice back then that three of them are entirely unused, and
the two dimension ones had no upper bound before.
Stefano: Why were all of these introduced (there's no explanation
in the description) and what were the values derived from? Will
anything break if we remove them?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] viridian: fix assertion failure
On Mon, Nov 26, 2018 at 03:38:52PM +, Paul Durrant wrote: > Whilst attempting to crash an apparently wedged Windows domain using > 'xen-hvmcrash' I managed to trigger the following ASSERT: > > (XEN) Assertion '!vp->ptr' failed at viridian.c:607 > > with stack: > > (XEN)[] viridian_map_guest_page+0x1b4/0x1b6 > (XEN)[] viridian_synic_load_vcpu_ctxt+0x39/0x3b > (XEN)[] viridian.c#viridian_load_vcpu_ctxt+0x93/0xcc > (XEN)[] hvm_load+0x10e/0x19e > (XEN)[] arch_do_domctl+0xb74/0x25b4 > (XEN)[] do_domctl+0x16f7/0x19d8 > > This happened because viridian_map_guest_page() was not written to cope > with being called multiple times, but this is unfortunately exactly what > happens when xen-hvmcrash re-loads the domain context (having clobbered > the values of RIP). > > This patch simply makes viridian_map_guest_page() return immediately if it > finds the page already mapped (i.e. vp->ptr != NULL). > > Signed-off-by: Paul Durrant Reviewed-by: Roger Pau Monné Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] Increase framebuffer size to todays standards
Am Mon, 26 Nov 2018 03:31:27 -0700 schrieb "Jan Beulich" : > And I think a change like this should (a) address the more general > case rather than just your laptop (or laptops in general) and (b) > actually add some headroom. Hence at the very least I'd see us > go to 4096x3072. WHUXGA would even call for 7680x4800. So should I resend this patch with higher values, or should I remove the bounds check entirely? Not sure what it is trying to achieve, the framebuffer may fail either way if the BIOS provides bogus values. Olaf pgp1doGdylznu.pgp Description: Digitale Signatur von OpenPGP ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
On 26/11/2018 15:29, Juergen Gross wrote: On 26/11/2018 15:58, Jan Beulich wrote: On 26.11.18 at 15:23, wrote: On 26/11/2018 15:01, Jan Beulich wrote: On 26.11.18 at 14:52, wrote: I don't think the hypervisor should explicitly try to make it as hard as possible for the guest to find problems in the code. That's indeed not the hypervisor's goal. Instead it tries to make it as hard as possible for the guest (developer) to make wrong assumptions. Let's look at the current example why I wrote this patch: The Linux kernel's use of multicalls should never trigger any single call to return an error (return value < 0). A kernel compiled for productive use will catch such errors, but has no knowledge which single call has failed, as it doesn't keep track of the single entries (non-productive kernels have an option available in the respective source to copy the entries before doing the multicall in order to have some diagnostic data available in case of such an error). Catching an error from a multicall right now means a WARN() with a stack backtrace (for the multicall itself, not for the entry causing the error). I have a customer report for a case where such a backtrace was produced and a kernel crash some seconds later, obviously due to illegally unmapped memory pages resulting from the failed multicall. Unfortunately there are multiple possibilities what might have gone wrong and I don't know which one was the culprit. The problem can't be a very common one, because there is only one such report right now, which might depend on a special driver. Finding this bug without a known reproducer and the current amount of diagnostic data is next to impossible. So I'd like to have more data available without having to hurt performance for the 99.99% of the cases where nothing bad happens. In case you have an idea how to solve this problem in another way I'd be happy to follow that route. I'd really like to be able to have a better clue in case such an error occurs in future. Since you have a production kernel, I assume you also have a production hypervisor. This hypervisor doesn't clobber the arguments if I'm not mistaken. Therefore - in the debugging scenario you (can) have all data available by virtue of the information getting copied in the kernel, - in the release scenario you have all data available since it's left un-clobbered. Am I missing anything (I don't view mixed debug/release setups of kernel and hypervisor as overly important here)? No, you are missing nothing here. OTOH a debug hypervisor destroying debug data is kind of weird, so I posted this patch. This is a quite common approach if you want to enforce the other entity to not rely on some fields. This also follows what we do for hypercalls (at least 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 RFC] mm/memory_hotplug: Introduce memory block types
On 26.11.18 15:20, Michal Suchánek wrote:
> On Mon, 26 Nov 2018 14:33:29 +0100
> David Hildenbrand wrote:
>
>> On 26.11.18 13:30, David Hildenbrand wrote:
>>> On 23.11.18 19:06, Michal Suchánek wrote:
>
If we are going to fake the driver information we may as well add the
type attribute and be done with it.
I think the problem with the patch was more with the semantic than the
attribute itself.
What is normal, paravirtualized, and standby memory?
I can understand DIMM device, baloon device, or whatever mechanism for
adding memory you might have.
I can understand "memory designated as standby by the cluster
administrator".
However, DIMM vs baloon is orthogonal to standby and should not be
conflated into one property.
paravirtualized means nothing at all in relationship to memory type and
the desired online policy to me.
>>>
>>> Right, so with whatever we come up, it should allow to make a decision
>>> in user space about
>>> - if memory is to be onlined automatically
>>
>> And I will think about if we really should model standby memory. Maybe
>> it is really better to have in user space something like (as Dan noted)
>
> If it is possible to designate the memory as standby or online in the
> s390 admin interface and the kernel does have access to this
> information it makes sense to forward it to userspace (as separate
> s390-specific property). If not then you need to make some kind of
> assumption like below and the user can tune the script according to
> their usecase.
Also true, standby memory really represents a distinct type of memory
block (memory seems to be there but really isn't). Right now I am
thinking about something like this (tried to formulate it on a very
generic level because we can't predict which mechanism might want to
make use of these types in the future).
/*
* Memory block types allow user space to formulate rules if and how to
* online memory blocks. The types are exposed to user space as text
* strings in sysfs. While the typical online strategies are described
* along with the types, there are use cases where that can differ (e.g.
* use MOVABLE zone for more reliable huge page usage, use NORMAL zone
* due to zone imbalance or because memory unplug is not intended).
*
* MEMORY_BLOCK_NONE:
* No memory block is to be created (e.g. device memory). Used internally
* only.
*
* MEMORY_BLOCK_REMOVABLE:
* This memory block type should be treated as if it can be
* removed/unplugged from the system again. E.g. there is a hardware
* interface to unplug such memory. This memory block type is usually
* onlined to the MOVABLE zone, to e.g. make offlining of it more
* reliable. Examples include ACPI and PPC DIMMs.
*
* MEMORY_BLOCK_UNREMOVABLE:
* This memory block type should be treated as if it can not be
* removed/unplugged again. E.g. there is no hardware interface to
* unplug such memory. This memory block type is usually onlined to
* the NORMAL zone, as offlining is not beneficial. Examples include boot
* memory on most architectures and memory added via balloon devices.
*
* MEMORY_BLOCK_STANDBY:
* The memory block type should be treated as if it can be
* removed/unplugged again, however the actual memory hot(un)plug is
* performed by onlining/offlining. In virtual environments, such memory
* is usually added during boot and never removed. Onlining memory will
* result in memory getting allocated to a VM. This memory type is usually
* not onlined automatically but explicitly by the administrator. One
* example is standby memory on s390x.
*/
>
>>
>> if (isS390x() && type == "dimm") {
>> /* don't online, on s390x system DIMMs are standby memory */
>> }
>
> Thanks
>
> Michal
>
--
Thanks,
David / dhildenb
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 10/14] x86/mm: Switch {get, put}_gfn() infrastructure to using gfn_t
On Wed, Nov 21, 2018 at 01:21:18PM +, Andy Cooper wrote:
> Seemingly, a majority of users either override the helpers anyway, or have an
> gfn_t in their hands.
>
> Update the API, and adjust all users to match.
>
> Doing this highlighted a gaping altp2m security hole in
> vmx_vcpu_update_vmfunc_ve(), which will need addressing now we can discuss the
> problem and options publicly.
>
> Signed-off-by: Andrew Cooper
As far as the svm and iommu parts
Acked-by: Brian Woods
> ---
> CC: Jan Beulich
> CC: Wei Liu
> CC: Roger Pau Monné
> CC: Razvan Cojocaru
> CC: Tamas K Lengyel
> CC: George Dunlap
> CC: Tim Deegan
> CC: Paul Durrant
> CC: Jun Nakajima
> CC: Kevin Tian
> CC: Boris Ostrovsky
> CC: Suravee Suthikulpanit
> CC: Brian Woods
> ---
> xen/arch/x86/cpu/mcheck/mcaction.c| 2 +-
> xen/arch/x86/cpu/mcheck/mce.c | 14 +++
> xen/arch/x86/cpu/mcheck/vmce.c| 4 +-
> xen/arch/x86/cpu/mcheck/vmce.h| 2 +-
> xen/arch/x86/debug.c | 6 +--
> xen/arch/x86/domain.c | 19 -
> xen/arch/x86/domctl.c | 8 ++--
> xen/arch/x86/hvm/dm.c | 12 +++---
> xen/arch/x86/hvm/emulate.c| 16
> xen/arch/x86/hvm/grant_table.c| 4 +-
> xen/arch/x86/hvm/hvm.c| 25 ++--
> xen/arch/x86/hvm/mtrr.c | 2 +-
> xen/arch/x86/hvm/svm/svm.c| 2 +-
> xen/arch/x86/hvm/vmx/vmx.c| 7 ++--
> xen/arch/x86/mm.c | 10 ++---
> xen/arch/x86/mm/hap/hap.c | 2 +-
> xen/arch/x86/mm/hap/nested_hap.c | 6 +--
> xen/arch/x86/mm/mem_access.c | 5 +--
> xen/arch/x86/mm/mem_sharing.c | 24 +--
> xen/arch/x86/mm/p2m.c | 45 ++--
> xen/arch/x86/mm/shadow/common.c | 4 +-
> xen/arch/x86/mm/shadow/multi.c| 68
> +++
> xen/arch/x86/mm/shadow/types.h| 4 --
> xen/common/grant_table.c | 10 ++---
> xen/common/memory.c | 24 +--
> xen/drivers/passthrough/amd/iommu_guest.c | 8
> xen/include/asm-x86/guest_pt.h| 4 --
> xen/include/asm-x86/p2m.h | 30 +++---
> 28 files changed, 172 insertions(+), 195 deletions(-)
>
> diff --git a/xen/arch/x86/cpu/mcheck/mcaction.c
> b/xen/arch/x86/cpu/mcheck/mcaction.c
> index e422674..c8e0cf2 100644
> --- a/xen/arch/x86/cpu/mcheck/mcaction.c
> +++ b/xen/arch/x86/cpu/mcheck/mcaction.c
> @@ -89,7 +89,7 @@ mc_memerr_dhandler(struct mca_binfo *binfo,
> ASSERT(d);
> gfn = get_gpfn_from_mfn((bank->mc_addr) >> PAGE_SHIFT);
>
> -if ( unmmap_broken_page(d, _mfn(mfn), gfn) )
> +if ( unmmap_broken_page(d, _mfn(mfn), _gfn(gfn)) )
> {
> printk("Unmap broken memory %lx for DOM%d failed\n",
> mfn, d->domain_id);
> diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
> index 30cdb06..c96c053 100644
> --- a/xen/arch/x86/cpu/mcheck/mce.c
> +++ b/xen/arch/x86/cpu/mcheck/mce.c
> @@ -1469,9 +1469,6 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
> struct domain *d;
> struct mcinfo_msr *msr;
> unsigned int i;
> -paddr_t gaddr;
> -unsigned long gfn, mfn;
> -p2m_type_t t;
>
> domid = (mc_msrinject->mcinj_domid == DOMID_SELF) ?
> current->domain->domain_id : mc_msrinject->mcinj_domid;
> @@ -1489,11 +1486,12 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
>i < mc_msrinject->mcinj_count;
>i++, msr++ )
> {
> -gaddr = msr->value;
> -gfn = PFN_DOWN(gaddr);
> -mfn = mfn_x(get_gfn(d, gfn, &t));
> +p2m_type_t t;
> +paddr_t gaddr = msr->value;
> +gfn_t gfn = _gfn(PFN_DOWN(gaddr));
> +mfn_t mfn = get_gfn(d, gfn, &t);
>
> -if ( mfn == mfn_x(INVALID_MFN) )
> +if ( mfn_eq(mfn, INVALID_MFN) )
> {
> put_gfn(d, gfn);
> put_domain(d);
> @@ -1501,7 +1499,7 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
> -EINVAL, gfn, domid);
> }
>
> -msr->value = pfn_to_paddr(mfn) | (gaddr & (PAGE_SIZE - 1));
> +msr->value = mfn_to_maddr(mfn) | (gaddr & (PAGE_SIZE - 1));
>
> put_gfn(d, gfn);
> }
> diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
> index f15835e..e257e94 100644
> --- a/xen/arch/x86/cpu/mcheck/vmce.c
> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
>
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
Juergen Gross writes ("Re: [PATCH] xen: only clobber multicall elements without
error"):
> On 26/11/2018 15:54, Ian Jackson wrote:
> > Maybe they could be clobbered losslessly ? Eg, by xoring with 0xaa or
> > something.
>
> This would be rather hacky: I'd need to find out if clobbering was
> performed or not and the way of clobbering would be kind of an interface
> which I guess we'd like to avoid.
You can bake the precise method of clobbering into your kernel
debugging dump stuff, along with a mechanism which guesses whether
things have been clobbered, so long as you don't go utterly wrong if
your guesses are wrong.
Since after all Xen doesn't guarantee to preserve the arguments at
all, and therefore you mustn't rely on it anyway.
This seems fine for a debug dump mechanism - debug dump mechanisms are
generally best effort anyway.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86emul: suppress default test harness build with incapable assembler
A top level "make build", as used e.g. by osstest, wants to build all
"all" targets in enabled tools subdirectories, which by default also
includes the emulator test harness. The use of, in particular, {evex}
insn pseudo-prefixes in, again in particular, test_x86_emulator.c causes
this build to fail though when the assembler is not new enough. Take
another big hammer and suppress the default harness build altogether
also when this and other pseudo-prefixes are not supported by the
specified (or defaulted to) assembler.
Signed-off-by: Jan Beulich
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -80,6 +80,10 @@ endef
$(foreach flavor,$(SIMD) $(FMA),$(eval $(call simd-check-cc,$(flavor
+# Also explicitly check for {evex} pseudo-prefix support, which got introduced
+# only after AVX512F and some of its extensions.
+TARGET-$(shell echo 'asm("{evex} vzeroall");' | $(CC) -x c -c -o /dev/null -
|| echo y) :=
+
ifeq ($(TARGET-y),)
$(warning Test harness not built, use newer compiler than "$(CC)")
endif
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] viridian: fix assertion failure
Whilst attempting to crash an apparently wedged Windows domain using
'xen-hvmcrash' I managed to trigger the following ASSERT:
(XEN) Assertion '!vp->ptr' failed at viridian.c:607
with stack:
(XEN)[] viridian_map_guest_page+0x1b4/0x1b6
(XEN)[] viridian_synic_load_vcpu_ctxt+0x39/0x3b
(XEN)[] viridian.c#viridian_load_vcpu_ctxt+0x93/0xcc
(XEN)[] hvm_load+0x10e/0x19e
(XEN)[] arch_do_domctl+0xb74/0x25b4
(XEN)[] do_domctl+0x16f7/0x19d8
This happened because viridian_map_guest_page() was not written to cope
with being called multiple times, but this is unfortunately exactly what
happens when xen-hvmcrash re-loads the domain context (having clobbered
the values of RIP).
This patch simply makes viridian_map_guest_page() return immediately if it
finds the page already mapped (i.e. vp->ptr != NULL).
Signed-off-by: Paul Durrant
---
Cc: Jan Beulich
Cc: Andrew Cooper
Cc: Wei Liu
Cc: "Roger Pau Monné"
v2:
- Stop leaking page references
---
xen/arch/x86/hvm/viridian/viridian.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/hvm/viridian/viridian.c
b/xen/arch/x86/hvm/viridian/viridian.c
index 7d73f41de6..c78b2918d9 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -602,10 +602,12 @@ void viridian_map_guest_page(struct vcpu *v, struct
viridian_page *vp)
{
struct domain *d = v->domain;
unsigned long gmfn = vp->msr.fields.pfn;
-struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
+struct page_info *page;
-ASSERT(!vp->ptr);
+if ( vp->ptr )
+return;
+page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
if ( !page )
goto fail;
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v5 15/24] hw: i386: Export the i386 ACPI SRAT build method
On Thu, 22 Nov 2018 00:27:33 +0100 Samuel Ortiz wrote: > On Thu, Nov 15, 2018 at 02:28:54PM +0100, Igor Mammedov wrote: > > On Mon, 5 Nov 2018 02:40:38 +0100 > > Samuel Ortiz wrote: > > > > > This is the standard way of building SRAT on x86 platfoms. But future > > > machine types could decide to define their own custom SRAT build method > > > through the ACPI builder methods. > > > Moreover, we will also need to reach build_srat() from outside of > > > acpi-build in order to use it as the ACPI builder SRAT build method. > > SRAT is usually highly machine specific (memory holes, layout, guest OS > > specific quirks) so it's hard to generalize it. > Hence the need for an SRAT builder interface. so far builder interface (trying to generalize acpi_build()) looks not necessary. I'd suggest to drop and call build_start() directly. > > I'd drop SRAT related patches from this series and introduce > > i386/virt specific SRAT when you post patches for it. > virt uses the existing i386 build_srat() routine, there's nothing > special about it. So this would be purely duplicated code. Looking at build_srat(), it has a bunch of code to handle legacy PC layout. You probably don't need any of it for new is86/virt machine and can make simpler version of it. In addition (probably repeating question I've asked elsewhere), Do you have to use split initial memory model for new machine? Is it possible to use only pc-dimms both for initial and hotplugged memory at some address (4Gb?) without cutting out PCI hole or any toher holes in RAM layout? > Cheers, > Samuel. > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] viridian: fix assertion failure
> -Original Message- > From: Roger Pau Monne > Sent: 26 November 2018 15:32 > To: Paul Durrant > Cc: xen-devel@lists.xenproject.org; Jan Beulich ; > Andrew Cooper ; Wei Liu > Subject: Re: [PATCH] viridian: fix assertion failure > > On Mon, Nov 26, 2018 at 03:08:50PM +, Paul Durrant wrote: > > > -Original Message- > > > From: Roger Pau Monne > > > Sent: 26 November 2018 15:04 > > > To: Paul Durrant > > > Cc: xen-devel@lists.xenproject.org; Jan Beulich ; > > > Andrew Cooper ; Wei Liu > > > > Subject: Re: [PATCH] viridian: fix assertion failure > > > > > > On Mon, Nov 26, 2018 at 02:54:54PM +, Paul Durrant wrote: > > > > Whilst attempting to crash an apparently wedged Windows domain using > > > > 'xen-hvmcrash' I managed to trigger the following ASSERT: > > > > > > > > (XEN) Assertion '!vp->ptr' failed at viridian.c:607 > > > > > > > > with stack: > > > > > > > > (XEN)[] viridian_map_guest_page+0x1b4/0x1b6 > > > > (XEN)[] > viridian_synic_load_vcpu_ctxt+0x39/0x3b > > > > (XEN)[] > > > viridian.c#viridian_load_vcpu_ctxt+0x93/0xcc > > > > (XEN)[] hvm_load+0x10e/0x19e > > > > (XEN)[] arch_do_domctl+0xb74/0x25b4 > > > > (XEN)[] do_domctl+0x16f7/0x19d8 > > > > > > > > This happened because viridian_map_guest_page() was not written to > cope > > > > with being called multiple times, but this is unfortunately exactly > what > > > > happens when xen-hvmcrash re-loads the domain context (having > clobbered > > > > the values of RIP). > > > > > > > > This patch simply makes viridian_map_guest_page() return immediately > if > > > it > > > > finds the page already mapped (i.e. vp->ptr != NULL). > > > > > > > > Signed-off-by: Paul Durrant > > > > --- > > > > Cc: Jan Beulich > > > > Cc: Andrew Cooper > > > > Cc: Wei Liu > > > > Cc: "Roger Pau Monné" > > > > --- > > > > xen/arch/x86/hvm/viridian/viridian.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/xen/arch/x86/hvm/viridian/viridian.c > > > b/xen/arch/x86/hvm/viridian/viridian.c > > > > index 7d73f41de6..b99501eea2 100644 > > > > --- a/xen/arch/x86/hvm/viridian/viridian.c > > > > +++ b/xen/arch/x86/hvm/viridian/viridian.c > > > > @@ -604,7 +604,8 @@ void viridian_map_guest_page(struct vcpu *v, > struct > > > viridian_page *vp) > > > > unsigned long gmfn = vp->msr.fields.pfn; > > > > struct page_info *page = get_page_from_gfn(d, gmfn, NULL, > > > P2M_ALLOC); > > > > > > > > -ASSERT(!vp->ptr); > > > > +if ( vp->ptr ) > > > > +return; > > > > > > Isn't this likely to get the page reference counting out of sync? You > > > will return without a put_page, and viridian_unmap_guest_page will set > > > vp->ptr = NULL and call put_page_and_type without taking into account > > > the reference count. > > > > > > > Damn. Missed that the side effect was in the initializer. I'll need to > move it below the check. > > > > > If viridian_map_guest_page needs to be called multiple times you > > > likely need some reference counting AFAICT, so that the last call to > > > viridian_unmap_guest_page is the one that releases the page and sets > > > vp->ptr = NULL. > > > > I shouldn't need to reference count. When the MSR is set by the guest > there is always an unmap then a map. It is just the context load case that > should only map on the first call (to handle resume correctly). > > OK, I haven't looked at the usage, but if there's a paired number of > maps/unmaps adding a reference counter to viridian_page is fairly > trivial and would make the implementation of map/unmap always correct > regardless of it's usage. > No, maps and unmaps are not paired. You can have an arbitrary number of maps, but only one unmap. Anyway, v2 coming in a minute. Paul > Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] amd-iommu: replace occurrences of bool_t with bool
On Mon, Nov 26, 2018 at 09:35:26AM -0600, Brian Woods wrote: > On Mon, Nov 26, 2018 at 09:05:25AM +, Paul Durrant wrote: > > Bring the coding style up to date. No functional change. > > > > Signed-off-by: Paul Durrant > > Acked-by: Brian Woods > > -- > Brian Woods Meant for another patch, please ignore this one. -- Brian Woods ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] viridian: fix assertion failure
On Mon, Nov 26, 2018 at 03:08:50PM +, Paul Durrant wrote: > > -Original Message- > > From: Roger Pau Monne > > Sent: 26 November 2018 15:04 > > To: Paul Durrant > > Cc: xen-devel@lists.xenproject.org; Jan Beulich ; > > Andrew Cooper ; Wei Liu > > Subject: Re: [PATCH] viridian: fix assertion failure > > > > On Mon, Nov 26, 2018 at 02:54:54PM +, Paul Durrant wrote: > > > Whilst attempting to crash an apparently wedged Windows domain using > > > 'xen-hvmcrash' I managed to trigger the following ASSERT: > > > > > > (XEN) Assertion '!vp->ptr' failed at viridian.c:607 > > > > > > with stack: > > > > > > (XEN)[] viridian_map_guest_page+0x1b4/0x1b6 > > > (XEN)[] viridian_synic_load_vcpu_ctxt+0x39/0x3b > > > (XEN)[] > > viridian.c#viridian_load_vcpu_ctxt+0x93/0xcc > > > (XEN)[] hvm_load+0x10e/0x19e > > > (XEN)[] arch_do_domctl+0xb74/0x25b4 > > > (XEN)[] do_domctl+0x16f7/0x19d8 > > > > > > This happened because viridian_map_guest_page() was not written to cope > > > with being called multiple times, but this is unfortunately exactly what > > > happens when xen-hvmcrash re-loads the domain context (having clobbered > > > the values of RIP). > > > > > > This patch simply makes viridian_map_guest_page() return immediately if > > it > > > finds the page already mapped (i.e. vp->ptr != NULL). > > > > > > Signed-off-by: Paul Durrant > > > --- > > > Cc: Jan Beulich > > > Cc: Andrew Cooper > > > Cc: Wei Liu > > > Cc: "Roger Pau Monné" > > > --- > > > xen/arch/x86/hvm/viridian/viridian.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/xen/arch/x86/hvm/viridian/viridian.c > > b/xen/arch/x86/hvm/viridian/viridian.c > > > index 7d73f41de6..b99501eea2 100644 > > > --- a/xen/arch/x86/hvm/viridian/viridian.c > > > +++ b/xen/arch/x86/hvm/viridian/viridian.c > > > @@ -604,7 +604,8 @@ void viridian_map_guest_page(struct vcpu *v, struct > > viridian_page *vp) > > > unsigned long gmfn = vp->msr.fields.pfn; > > > struct page_info *page = get_page_from_gfn(d, gmfn, NULL, > > P2M_ALLOC); > > > > > > -ASSERT(!vp->ptr); > > > +if ( vp->ptr ) > > > +return; > > > > Isn't this likely to get the page reference counting out of sync? You > > will return without a put_page, and viridian_unmap_guest_page will set > > vp->ptr = NULL and call put_page_and_type without taking into account > > the reference count. > > > > Damn. Missed that the side effect was in the initializer. I'll need to move > it below the check. > > > If viridian_map_guest_page needs to be called multiple times you > > likely need some reference counting AFAICT, so that the last call to > > viridian_unmap_guest_page is the one that releases the page and sets > > vp->ptr = NULL. > > I shouldn't need to reference count. When the MSR is set by the guest there > is always an unmap then a map. It is just the context load case that should > only map on the first call (to handle resume correctly). OK, I haven't looked at the usage, but if there's a paired number of maps/unmaps adding a reference counter to viridian_page is fairly trivial and would make the implementation of map/unmap always correct regardless of it's usage. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] amd-iommu: replace occurrences of bool_t with bool
On Mon, Nov 26, 2018 at 09:05:25AM +, Paul Durrant wrote: > Bring the coding style up to date. No functional change. > > Signed-off-by: Paul Durrant Acked-by: Brian Woods -- Brian Woods ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
On 26/11/2018 15:58, Jan Beulich wrote: On 26.11.18 at 15:23, wrote: >> On 26/11/2018 15:01, Jan Beulich wrote: >> On 26.11.18 at 14:52, wrote: I don't think the hypervisor should explicitly try to make it as hard as possible for the guest to find problems in the code. >>> >>> That's indeed not the hypervisor's goal. Instead it tries to make >>> it as hard as possible for the guest (developer) to make wrong >>> assumptions. >> >> Let's look at the current example why I wrote this patch: >> >> The Linux kernel's use of multicalls should never trigger any single >> call to return an error (return value < 0). A kernel compiled for >> productive use will catch such errors, but has no knowledge which >> single call has failed, as it doesn't keep track of the single entries >> (non-productive kernels have an option available in the respective >> source to copy the entries before doing the multicall in order to have >> some diagnostic data available in case of such an error). Catching an >> error from a multicall right now means a WARN() with a stack backtrace >> (for the multicall itself, not for the entry causing the error). >> >> I have a customer report for a case where such a backtrace was produced >> and a kernel crash some seconds later, obviously due to illegally >> unmapped memory pages resulting from the failed multicall. Unfortunately >> there are multiple possibilities what might have gone wrong and I don't >> know which one was the culprit. The problem can't be a very common one, >> because there is only one such report right now, which might depend on >> a special driver. >> >> Finding this bug without a known reproducer and the current amount of >> diagnostic data is next to impossible. So I'd like to have more data >> available without having to hurt performance for the 99.99% of the >> cases where nothing bad happens. >> >> In case you have an idea how to solve this problem in another way I'd be >> happy to follow that route. I'd really like to be able to have a better >> clue in case such an error occurs in future. > > Since you have a production kernel, I assume you also have a > production hypervisor. This hypervisor doesn't clobber the > arguments if I'm not mistaken. Therefore > - in the debugging scenario you (can) have all data available by > virtue of the information getting copied in the kernel, > - in the release scenario you have all data available since it's > left un-clobbered. > Am I missing anything (I don't view mixed debug/release setups > of kernel and hypervisor as overly important here)? No, you are missing nothing here. OTOH a debug hypervisor destroying debug data is kind of weird, so I posted this patch. I'll add the related Linux kernel patch (in case it is acked by Boris) with or without this hypervisor patch, but I thought it would be better to have the hypervisor patch in place, especially as e.g. a hypervisor from xen-unstable might have a bug which could be easier to diagnose with this patch in place. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] x86/msr: Handle MSR_TSC_AUX consistently for PV and HVM guests
On Thu, Nov 15, 2018 at 09:47:17PM +, Andy Cooper wrote: > With PVRDTSCP mode removed, handling of MSR_TSC_AUX can move into the common > code. Move its storage into struct vcpu_msrs (dropping the HVM-specific > msr_tsc_aux), and add an RDPID feature check as this bit also enumerates the > presence of the MSR. > > Drop hvm_msr_tsc_aux() entirely, and use v->arch.msrs->tsc_aux directly. > Update hvm_load_cpu_ctxt() to check that the incoming ctxt.msr_tsc_aux isn't > out of range. In practice, no previous version of Xen ever wrote an > out-of-range value. Add MSR_TSC_AUX to the list of MSRs migrated for PV > guests. > > Signed-off-by: Andrew Cooper Acked-by: Brian Woods -- Brian Woods ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
On 26/11/2018 15:54, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH] xen: only clobber multicall elements without
> error"):
>> On 23.11.18 at 14:25, wrote:
>>> In debug builds the hypervisor will deliberately clobber processed
>>> elements of the multicall structure. In order to ease diagnostic data
>>> printout in the affected guest only clobber elements which didn't
>>> return an error.
>>
>> Besides what Andrew has said such a relaxation reduces
>> the guarding against bad guest side code. If a guest really
>> wishes to produce diagnostics, I think it should go to the
>> lengths of copying arguments (if they can't be re-calculated
>> anyway). Suppressing the clobbering in more cases merely
>> invites guests to read the arguments after the call, which
>> they simply should not do. Not clobbering the values in
>> release builds is a performance choice, and we ought to be
>> allowed to change our opinion regarding this implementation
>> detail at any point in time.
>
> Maybe they could be clobbered losslessly ? Eg, by xoring with 0xaa or
> something.
This would be rather hacky: I'd need to find out if clobbering was
performed or not and the way of clobbering would be kind of an interface
which I guess we'd like to avoid.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] xl: free bitmaps on exit
On Mon, Nov 26, 2018 at 04:04:08PM +0100, Olaf Hering wrote: > Am Mon, 26 Nov 2018 15:58:22 +0100 > schrieb Olaf Hering : > > > +++ b/tools/xl/xl.c > > @@ -229,6 +229,9 @@ static void parse_global_config(const char *configfile, > > Actually I think that should go to xl_ctx_free() instead. I moved it down to > parse_global_config() by accident. You can't just put the _dispose into xl_ctx_free. Because xl_ctx_free can be called before calling parse_global_config. You also need to put the _init calls into xl_ctx_alloc. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] qemu assert in staging during HVM live migration
Am Mon, 26 Nov 2018 14:32:05 + schrieb Anthony PERARD : > Also, a backtrace of the original problem would be nice. There is no backtrace, all the data I could grab was sent to this list. I was unable to reproduce it as well. Olaf pgpL73JOm72zk.pgp Description: Digitale Signatur von OpenPGP ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] viridian: fix assertion failure
> -Original Message- > From: Roger Pau Monne > Sent: 26 November 2018 15:04 > To: Paul Durrant > Cc: xen-devel@lists.xenproject.org; Jan Beulich ; > Andrew Cooper ; Wei Liu > Subject: Re: [PATCH] viridian: fix assertion failure > > On Mon, Nov 26, 2018 at 02:54:54PM +, Paul Durrant wrote: > > Whilst attempting to crash an apparently wedged Windows domain using > > 'xen-hvmcrash' I managed to trigger the following ASSERT: > > > > (XEN) Assertion '!vp->ptr' failed at viridian.c:607 > > > > with stack: > > > > (XEN)[] viridian_map_guest_page+0x1b4/0x1b6 > > (XEN)[] viridian_synic_load_vcpu_ctxt+0x39/0x3b > > (XEN)[] > viridian.c#viridian_load_vcpu_ctxt+0x93/0xcc > > (XEN)[] hvm_load+0x10e/0x19e > > (XEN)[] arch_do_domctl+0xb74/0x25b4 > > (XEN)[] do_domctl+0x16f7/0x19d8 > > > > This happened because viridian_map_guest_page() was not written to cope > > with being called multiple times, but this is unfortunately exactly what > > happens when xen-hvmcrash re-loads the domain context (having clobbered > > the values of RIP). > > > > This patch simply makes viridian_map_guest_page() return immediately if > it > > finds the page already mapped (i.e. vp->ptr != NULL). > > > > Signed-off-by: Paul Durrant > > --- > > Cc: Jan Beulich > > Cc: Andrew Cooper > > Cc: Wei Liu > > Cc: "Roger Pau Monné" > > --- > > xen/arch/x86/hvm/viridian/viridian.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/xen/arch/x86/hvm/viridian/viridian.c > b/xen/arch/x86/hvm/viridian/viridian.c > > index 7d73f41de6..b99501eea2 100644 > > --- a/xen/arch/x86/hvm/viridian/viridian.c > > +++ b/xen/arch/x86/hvm/viridian/viridian.c > > @@ -604,7 +604,8 @@ void viridian_map_guest_page(struct vcpu *v, struct > viridian_page *vp) > > unsigned long gmfn = vp->msr.fields.pfn; > > struct page_info *page = get_page_from_gfn(d, gmfn, NULL, > P2M_ALLOC); > > > > -ASSERT(!vp->ptr); > > +if ( vp->ptr ) > > +return; > > Isn't this likely to get the page reference counting out of sync? You > will return without a put_page, and viridian_unmap_guest_page will set > vp->ptr = NULL and call put_page_and_type without taking into account > the reference count. > Damn. Missed that the side effect was in the initializer. I'll need to move it below the check. > If viridian_map_guest_page needs to be called multiple times you > likely need some reference counting AFAICT, so that the last call to > viridian_unmap_guest_page is the one that releases the page and sets > vp->ptr = NULL. I shouldn't need to reference count. When the MSR is set by the guest there is always an unmap then a map. It is just the context load case that should only map on the first call (to handle resume correctly). Paul > > Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-3.1] hw/xen/xen_pt_graphics: Don't trust the BIOS ROM contents so much
On Mon, Nov 19, 2018 at 04:26:58PM +, Peter Maydell wrote: > Coverity (CID 796599) points out that xen_pt_setup_vga() trusts > the rom->size field in the BIOS ROM from a PCI passthrough VGA > device, and uses it as an index into the memory which contains > the BIOS image. A corrupt BIOS ROM could therefore cause us to > index off the end of the buffer. > > Check that the size is within bounds before we use it. > > We are also trusting the pcioffset field, and assuming that > the whole rom_header is present; Coverity doesn't notice these, > but check them too. > > Signed-off-by: Peter Maydell > --- > Disclaimer: compile tested only, as I don't have a Xen setup, > let alone one with pass-through PCI graphics. > > Note that https://xenbits.xen.org/xsa/advisory-124.html > defines that bugs which are only exploitable by a malicious > piece of hardware that is passed through to the guest are > not security vulnerabilities as far as the Xen Project is > concerned, and are treated like normal non-security-related bugs. > So this is just a bugfix, not a security issue. > > Marked "for-3.1" because it would let us squash another Coverity > issue, and it is a bug fix; on the other hand it's an obscure > corner case and has been this way since forever. I haven't tested that patch either, but the changes looks fine, so: Acked-by: Anthony PERARD Thanks, -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] viridian: fix assertion failure
On Mon, Nov 26, 2018 at 02:54:54PM +, Paul Durrant wrote: > Whilst attempting to crash an apparently wedged Windows domain using > 'xen-hvmcrash' I managed to trigger the following ASSERT: > > (XEN) Assertion '!vp->ptr' failed at viridian.c:607 > > with stack: > > (XEN)[] viridian_map_guest_page+0x1b4/0x1b6 > (XEN)[] viridian_synic_load_vcpu_ctxt+0x39/0x3b > (XEN)[] viridian.c#viridian_load_vcpu_ctxt+0x93/0xcc > (XEN)[] hvm_load+0x10e/0x19e > (XEN)[] arch_do_domctl+0xb74/0x25b4 > (XEN)[] do_domctl+0x16f7/0x19d8 > > This happened because viridian_map_guest_page() was not written to cope > with being called multiple times, but this is unfortunately exactly what > happens when xen-hvmcrash re-loads the domain context (having clobbered > the values of RIP). > > This patch simply makes viridian_map_guest_page() return immediately if it > finds the page already mapped (i.e. vp->ptr != NULL). > > Signed-off-by: Paul Durrant > --- > Cc: Jan Beulich > Cc: Andrew Cooper > Cc: Wei Liu > Cc: "Roger Pau Monné" > --- > xen/arch/x86/hvm/viridian/viridian.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/hvm/viridian/viridian.c > b/xen/arch/x86/hvm/viridian/viridian.c > index 7d73f41de6..b99501eea2 100644 > --- a/xen/arch/x86/hvm/viridian/viridian.c > +++ b/xen/arch/x86/hvm/viridian/viridian.c > @@ -604,7 +604,8 @@ void viridian_map_guest_page(struct vcpu *v, struct > viridian_page *vp) > unsigned long gmfn = vp->msr.fields.pfn; > struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); > > -ASSERT(!vp->ptr); > +if ( vp->ptr ) > +return; Isn't this likely to get the page reference counting out of sync? You will return without a put_page, and viridian_unmap_guest_page will set vp->ptr = NULL and call put_page_and_type without taking into account the reference count. If viridian_map_guest_page needs to be called multiple times you likely need some reference counting AFAICT, so that the last call to viridian_unmap_guest_page is the one that releases the page and sets vp->ptr = NULL. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1] xl: free bitmaps on exit
Am Mon, 26 Nov 2018 15:58:22 +0100 schrieb Olaf Hering : > +++ b/tools/xl/xl.c > @@ -229,6 +229,9 @@ static void parse_global_config(const char *configfile, Actually I think that should go to xl_ctx_free() instead. I moved it down to parse_global_config() by accident. Olaf pgptRsIXARFmB.pgp Description: Digitale Signatur von OpenPGP ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
>>> On 26.11.18 at 15:23, wrote: > On 26/11/2018 15:01, Jan Beulich wrote: > On 26.11.18 at 14:52, wrote: >>> I don't think the hypervisor should explicitly try to make it as hard as >>> possible for the guest to find problems in the code. >> >> That's indeed not the hypervisor's goal. Instead it tries to make >> it as hard as possible for the guest (developer) to make wrong >> assumptions. > > Let's look at the current example why I wrote this patch: > > The Linux kernel's use of multicalls should never trigger any single > call to return an error (return value < 0). A kernel compiled for > productive use will catch such errors, but has no knowledge which > single call has failed, as it doesn't keep track of the single entries > (non-productive kernels have an option available in the respective > source to copy the entries before doing the multicall in order to have > some diagnostic data available in case of such an error). Catching an > error from a multicall right now means a WARN() with a stack backtrace > (for the multicall itself, not for the entry causing the error). > > I have a customer report for a case where such a backtrace was produced > and a kernel crash some seconds later, obviously due to illegally > unmapped memory pages resulting from the failed multicall. Unfortunately > there are multiple possibilities what might have gone wrong and I don't > know which one was the culprit. The problem can't be a very common one, > because there is only one such report right now, which might depend on > a special driver. > > Finding this bug without a known reproducer and the current amount of > diagnostic data is next to impossible. So I'd like to have more data > available without having to hurt performance for the 99.99% of the > cases where nothing bad happens. > > In case you have an idea how to solve this problem in another way I'd be > happy to follow that route. I'd really like to be able to have a better > clue in case such an error occurs in future. Since you have a production kernel, I assume you also have a production hypervisor. This hypervisor doesn't clobber the arguments if I'm not mistaken. Therefore - in the debugging scenario you (can) have all data available by virtue of the information getting copied in the kernel, - in the release scenario you have all data available since it's left un-clobbered. Am I missing anything (I don't view mixed debug/release setups of kernel and hypervisor as overly important here)? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v1] xl: free bitmaps on exit
Every invocation of xl via valgrind will show three leaks. Since libxl_bitmap_alloc uses NOGC, the caller has to free the memory after use. Signed-off-by: Olaf Hering --- tools/xl/xl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/xl/xl.c b/tools/xl/xl.c index 7d2142f16f..9756a83526 100644 --- a/tools/xl/xl.c +++ b/tools/xl/xl.c @@ -229,6 +229,9 @@ static void parse_global_config(const char *configfile, else libxl_bitmap_set_any(&global_pv_affinity_mask); +libxl_bitmap_dispose(&global_pv_affinity_mask); +libxl_bitmap_dispose(&global_hvm_affinity_mask); +libxl_bitmap_dispose(&global_vm_affinity_mask); xlu_cfg_destroy(config); } ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
Jan Beulich writes ("Re: [PATCH] xen: only clobber multicall elements without
error"):
> On 23.11.18 at 14:25, wrote:
> > In debug builds the hypervisor will deliberately clobber processed
> > elements of the multicall structure. In order to ease diagnostic data
> > printout in the affected guest only clobber elements which didn't
> > return an error.
>
> Besides what Andrew has said such a relaxation reduces
> the guarding against bad guest side code. If a guest really
> wishes to produce diagnostics, I think it should go to the
> lengths of copying arguments (if they can't be re-calculated
> anyway). Suppressing the clobbering in more cases merely
> invites guests to read the arguments after the call, which
> they simply should not do. Not clobbering the values in
> release builds is a performance choice, and we ought to be
> allowed to change our opinion regarding this implementation
> detail at any point in time.
Maybe they could be clobbered losslessly ? Eg, by xoring with 0xaa or
something.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] viridian: fix assertion failure
Whilst attempting to crash an apparently wedged Windows domain using 'xen-hvmcrash' I managed to trigger the following ASSERT: (XEN) Assertion '!vp->ptr' failed at viridian.c:607 with stack: (XEN)[] viridian_map_guest_page+0x1b4/0x1b6 (XEN)[] viridian_synic_load_vcpu_ctxt+0x39/0x3b (XEN)[] viridian.c#viridian_load_vcpu_ctxt+0x93/0xcc (XEN)[] hvm_load+0x10e/0x19e (XEN)[] arch_do_domctl+0xb74/0x25b4 (XEN)[] do_domctl+0x16f7/0x19d8 This happened because viridian_map_guest_page() was not written to cope with being called multiple times, but this is unfortunately exactly what happens when xen-hvmcrash re-loads the domain context (having clobbered the values of RIP). This patch simply makes viridian_map_guest_page() return immediately if it finds the page already mapped (i.e. vp->ptr != NULL). Signed-off-by: Paul Durrant --- Cc: Jan Beulich Cc: Andrew Cooper Cc: Wei Liu Cc: "Roger Pau Monné" --- xen/arch/x86/hvm/viridian/viridian.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c index 7d73f41de6..b99501eea2 100644 --- a/xen/arch/x86/hvm/viridian/viridian.c +++ b/xen/arch/x86/hvm/viridian/viridian.c @@ -604,7 +604,8 @@ void viridian_map_guest_page(struct vcpu *v, struct viridian_page *vp) unsigned long gmfn = vp->msr.fields.pfn; struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC); -ASSERT(!vp->ptr); +if ( vp->ptr ) +return; if ( !page ) goto fail; -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] qemu assert in staging during HVM live migration
On Fri, Nov 23, 2018 at 04:57:08PM +0100, Olaf Hering wrote: > Am Fri, 23 Nov 2018 15:54:49 + > schrieb Anthony PERARD : > > > Is your toolstack runs the QMP command 'query-block' or some other > > command related to block behind libxl's back? > > This is plain xl create + xl migrate from staging. > > > Beside that, I don't know if something else might be wrong. What patches > > is there on your qemu-2.12 (at least patches related to xen-disk)? > > Nothing related. I will see if I can get some runtime to try the suggested > changes. Also, a backtrace of the original problem would be nice. I haven't been able to reproduce with a simple `xl migrate domU localhost` Thanks, -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 20/21] xen_backend: remove xen_sysdev_init() function
On Fri, Nov 23, 2018 at 11:30:39PM +0800, Mao Zhongyi wrote: > The init function doesn't do anything at all, so we > just omit it. > > Cc: sstabell...@kernel.org > Cc: anthony.per...@citrix.com > Cc: xen-devel@lists.xenproject.org > Cc: peter.mayd...@linaro.org > > Signed-off-by: Mao Zhongyi > Signed-off-by: Zhang Shengju Acked-by: Anthony PERARD Thanks, -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86emul: fix test harness 32-bit "clean" target handling
On Mon, Nov 26, 2018 at 07:13:33AM -0700, Jan Beulich wrote:
> When preparing what is now 52c37f7ab9 ("x86emul: also allow running the
> 32-bit harness on a 64-bit distro") I first wrongly used XEN_TARGET_ARCH
> instead of XEN_COMPILE_ARCH. When realizing the mistake I forgot to also
> switch around the use in the expression controlling the rule
> dependencies, causing "make distclean" to fail on 64-bit distros.
>
> Reported-by: Paul Durrant
> Signed-off-by: Jan Beulich
> Tested-by: Paul Durrant
Acked-by: Wei Liu
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
On 26/11/2018 15:01, Jan Beulich wrote: On 26.11.18 at 14:52, wrote: >> I don't think the hypervisor should explicitly try to make it as hard as >> possible for the guest to find problems in the code. > > That's indeed not the hypervisor's goal. Instead it tries to make > it as hard as possible for the guest (developer) to make wrong > assumptions. Let's look at the current example why I wrote this patch: The Linux kernel's use of multicalls should never trigger any single call to return an error (return value < 0). A kernel compiled for productive use will catch such errors, but has no knowledge which single call has failed, as it doesn't keep track of the single entries (non-productive kernels have an option available in the respective source to copy the entries before doing the multicall in order to have some diagnostic data available in case of such an error). Catching an error from a multicall right now means a WARN() with a stack backtrace (for the multicall itself, not for the entry causing the error). I have a customer report for a case where such a backtrace was produced and a kernel crash some seconds later, obviously due to illegally unmapped memory pages resulting from the failed multicall. Unfortunately there are multiple possibilities what might have gone wrong and I don't know which one was the culprit. The problem can't be a very common one, because there is only one such report right now, which might depend on a special driver. Finding this bug without a known reproducer and the current amount of diagnostic data is next to impossible. So I'd like to have more data available without having to hurt performance for the 99.99% of the cases where nothing bad happens. In case you have an idea how to solve this problem in another way I'd be happy to follow that route. I'd really like to be able to have a better clue in case such an error occurs in future. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 130811: regressions - FAIL
flight 130811 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/130811/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 130289 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386 1 build-check(1) blocked n/a test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen fc3637e9af9a301d92695999299a3e9a8458c3c1 baseline version: xen 901abfef5de149546b16fba6f4d5bd7def08c672 Last test of basis 130289 2018-11-17 11:00:36 Z9 days Failing since130490 2018-11-19 09:00:27 Z7 days 60 attempts Testing same since 130733 2018-11-23 16:00:40 Z2 days 25 attempts People who touched revisions under test: Andrew Cooper Dario Faggioli Doug Goldstein Ian Jackson Jan Beulich Julien Grall Kevin Tian Len Brown Norbert Manthey Olaf Hering Paul Durrant Rafael J. Wysocki Razvan Cojocaru Roger Pau Monné Sergey Dyasli Tamas K Lengyel Tim Deegan Wei Liu jobs: build-amd64 fail build-armhf pass build-amd64-libvirt blocked test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 blocked test-amd64-amd64-libvirt blocked sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 1190 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] mm/memory_hotplug: Introduce memory block types
On Mon, 26 Nov 2018 14:33:29 +0100
David Hildenbrand wrote:
> On 26.11.18 13:30, David Hildenbrand wrote:
> > On 23.11.18 19:06, Michal Suchánek wrote:
> >>
> >> If we are going to fake the driver information we may as well add the
> >> type attribute and be done with it.
> >>
> >> I think the problem with the patch was more with the semantic than the
> >> attribute itself.
> >>
> >> What is normal, paravirtualized, and standby memory?
> >>
> >> I can understand DIMM device, baloon device, or whatever mechanism for
> >> adding memory you might have.
> >>
> >> I can understand "memory designated as standby by the cluster
> >> administrator".
> >>
> >> However, DIMM vs baloon is orthogonal to standby and should not be
> >> conflated into one property.
> >>
> >> paravirtualized means nothing at all in relationship to memory type and
> >> the desired online policy to me.
> >
> > Right, so with whatever we come up, it should allow to make a decision
> > in user space about
> > - if memory is to be onlined automatically
>
> And I will think about if we really should model standby memory. Maybe
> it is really better to have in user space something like (as Dan noted)
If it is possible to designate the memory as standby or online in the
s390 admin interface and the kernel does have access to this
information it makes sense to forward it to userspace (as separate
s390-specific property). If not then you need to make some kind of
assumption like below and the user can tune the script according to
their usecase.
>
> if (isS390x() && type == "dimm") {
> /* don't online, on s390x system DIMMs are standby memory */
> }
Thanks
Michal
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86emul: fix test harness 32-bit "clean" target handling
On 26/11/2018 14:13, Jan Beulich wrote:
> When preparing what is now 52c37f7ab9 ("x86emul: also allow running the
> 32-bit harness on a 64-bit distro") I first wrongly used XEN_TARGET_ARCH
> instead of XEN_COMPILE_ARCH. When realizing the mistake I forgot to also
> switch around the use in the expression controlling the rule
> dependencies, causing "make distclean" to fail on 64-bit distros.
>
> Reported-by: Paul Durrant
> Signed-off-by: Jan Beulich
> Tested-by: Paul Durrant
Acked-by: Andrew Cooper
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86emul: fix test harness 32-bit "clean" target handling
When preparing what is now 52c37f7ab9 ("x86emul: also allow running the
32-bit harness on a 64-bit distro") I first wrongly used XEN_TARGET_ARCH
instead of XEN_COMPILE_ARCH. When realizing the mistake I forgot to also
switch around the use in the expression controlling the rule
dependencies, causing "make distclean" to fail on 64-bit distros.
Reported-by: Paul Durrant
Signed-off-by: Jan Beulich
Tested-by: Paul Durrant
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -185,7 +185,7 @@ distclean: clean
install uninstall:
.PHONY: run32 clean32
-ifeq ($(XEN_TARGET_ARCH),x86_64)
+ifeq ($(XEN_COMPILE_ARCH),x86_64)
run32: $(addsuffix .h,$(TESTCASES)) $(addsuffix -opmask.h,$(OPMASK))
run32 clean32: %32:
$(MAKE) -C 32 $*
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
>>> On 26.11.18 at 14:52, wrote: > I don't think the hypervisor should explicitly try to make it as hard as > possible for the guest to find problems in the code. That's indeed not the hypervisor's goal. Instead it tries to make it as hard as possible for the guest (developer) to make wrong assumptions. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v3 00/14] Generalize machine compatibility properties
On Wed, 7 Nov 2018 16:36:38 +0400 Marc-André Lureau wrote: > Hi, > > During "[PATCH v2 05/10] qom/globals: generalize > object_property_set_globals()" review, Eduardo suggested to rework the > GlobalProperty handling, so that -global is limited to QDev only and > we avoid mixing the machine compats and the user-provided -global > properties (instead of generalizing -global to various object kinds, > like I proposed in v2). > > "qdev: do not mix compat props with global props" patch decouples a > bit user-provided -global from machine compat properties. This allows > to get rid of "user_provided" and "errp" fields in following patches. > > Instead of explcitely calling object_apply_global_props() in the > various object post_init, I opted for creating a new TYPE_COMPAT_PROPS > interface. The interface approach gives a lot more flexibility on > which objects can have compat props. This requires some interface > improvments in "qom: teach interfaces to implement post-init". > > A new compat property "x-use-canonical-path-for-ramblock-id" is added > to hostmem for legacy canonical path names, set to true for -file and > -memfd with qemu < 3.2. > > (this series was initially titled "[PATCH v2 00/10] hostmem: use > object "id" for memory region name with >= 3.1", but its focus is more > in refactoring the global and compatilibity properties handling now) That probably all feedback I'm able to give on this round of review, so I'll wait till it will addressed. > > v3: > - GlobalProperties improvements/cleanups > - drop generalizing the -global idea > - "replace" the set_globals flag with a TYPE_COMPAT_PROPS interface > - update hw/i386 machine version to 3.2 > - add "qom: make interface types abstract" interface cleanup > > v2: > - replace "qom/user-creatable: add a few helper macros" patch for a > more optimized "qom: make user_creatable_complete() specific to > UserCreatable" > - rename register_global_list() to register_global_properties() > - call object_property_set_globals() after post-init > - add and use a ObjectClass.set_globals flag, instead of dynamically > check object class in object_property_set_globals() > - use object "id" in >= 3.1 instead of canonical path, add compat > property "x-use-canonical-path-for-ramblock-id" in base hostmem > class. > > Marc-André Lureau (14): > tests: qdev_prop_check_globals() doesn't return "all_used" > qom: make interface types abstract > qom: make user_creatable_complete() specific to UserCreatable > accel: register global_props like machine globals > qdev: move qdev_prop_register_global_list() to tests > qdev: do not mix compat props with global props > qdev: all globals are now user-provided > qdev-props: convert global_props to GArray > qdev-props: remove errp from GlobalProperty > qdev-props: call object_apply_global_props() > qom: teach interfaces to implement post-init > machine: add compat-props interface > hw/i386: add pc-i440fx-3.2 & pc-q35-3.2 > hostmem: use object id for memory region name with >= 3.1 > > include/hw/acpi/acpi_dev_interface.h | 6 +-- > include/hw/arm/linux-boot-if.h | 5 +- > include/hw/boards.h | 3 +- > include/hw/compat.h | 11 > include/hw/fw-path-provider.h| 4 +- > include/hw/hotplug.h | 6 +-- > include/hw/i386/pc.h | 3 ++ > include/hw/intc/intc.h | 4 +- > include/hw/ipmi/ipmi.h | 4 +- > include/hw/isa/isa.h | 4 -- > include/hw/mem/memory-device.h | 4 +- > include/hw/nmi.h | 4 +- > include/hw/qdev-core.h | 9 > include/hw/qdev-properties.h | 30 --- > include/hw/stream.h | 4 +- > include/hw/timer/m48t59.h| 4 +- > include/qom/object.h | 2 + > include/qom/object_interfaces.h | 10 ++-- > include/sysemu/accel.h | 4 +- > include/sysemu/hostmem.h | 3 +- > include/sysemu/tpm.h | 4 +- > target/arm/idau.h| 4 +- > accel/accel.c| 7 +-- > backends/hostmem-file.c | 8 +-- > backends/hostmem-memfd.c | 2 +- > backends/hostmem-ram.c | 9 ++-- > backends/hostmem.c | 31 +++ > hw/core/compat-props.c | 43 +++ > hw/core/machine.c| 18 --- > hw/core/qdev-properties.c| 73 ++--- > hw/core/qdev.c | 4 ++ > hw/i386/pc_piix.c| 21 ++-- > hw/i386/pc_q35.c | 19 ++- > hw/misc/ivshmem.c| 2 +- > hw/virtio/virtio-rng.c | 2 +- > hw/xen/xen-common.c | 9 +++- > qom/cpu.c| 1 - > qom/object.c | 49 +++-- > qom/object_inter
Re: [Xen-devel] [PATCH] xen: only clobber multicall elements without error
On 26/11/2018 11:58, Jan Beulich wrote: On 23.11.18 at 14:25, wrote: >> In debug builds the hypervisor will deliberately clobber processed >> elements of the multicall structure. In order to ease diagnostic data >> printout in the affected guest only clobber elements which didn't >> return an error. > > Besides what Andrew has said such a relaxation reduces > the guarding against bad guest side code. If a guest really > wishes to produce diagnostics, I think it should go to the > lengths of copying arguments (if they can't be re-calculated > anyway). Suppressing the clobbering in more cases merely > invites guests to read the arguments after the call, which > they simply should not do. Not clobbering the values in > release builds is a performance choice, and we ought to be > allowed to change our opinion regarding this implementation > detail at any point in time. Right. And not copying the values before the call is a performance choice on guest side, as errors are not the common case. I know there is no guarantee for the guest that the values are preserved after the call, but in the error case (which should be _very_ rare) it will make diagnosis of that case much easier. I don't think the hypervisor should explicitly try to make it as hard as possible for the guest to find problems in the code. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] x86_emulator distclean failing
> -Original Message- > From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf > Of Jan Beulich > Sent: 26 November 2018 13:41 > To: Paul Durrant > Cc: xen-devel > Subject: Re: [Xen-devel] x86_emulator distclean failing > > >>> On 26.11.18 at 14:04, wrote: > > Anyone else seeing this, or is it just me? This was on a fresh pull of > > staging about 5 mins ago... > > I don't think I ever ran distclean, but I think I can see the issue. > Would you mind trying > > --- a/tools/tests/x86_emulator/Makefile > +++ b/tools/tests/x86_emulator/Makefile > @@ -164,7 +164,7 @@ distclean: clean > install uninstall: > > .PHONY: run32 clean32 > -ifeq ($(XEN_TARGET_ARCH),x86_64) > +ifeq ($(XEN_COMPILE_ARCH),x86_64) > run32: $(addsuffix .h,$(TESTCASES)) $(addsuffix -opmask.h,$(OPMASK)) > run32 clean32: %32: > $(MAKE) -C 32 $* > > Simply an incomplete renamed that I had done when putting > together 52c37f7ab9. That appears to do the trick. Thanks, Paul > > Jan > > > > ___ > 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] x86_emulator distclean failing
>>> On 26.11.18 at 14:04, wrote: > Anyone else seeing this, or is it just me? This was on a fresh pull of > staging about 5 mins ago... I don't think I ever ran distclean, but I think I can see the issue. Would you mind trying --- a/tools/tests/x86_emulator/Makefile +++ b/tools/tests/x86_emulator/Makefile @@ -164,7 +164,7 @@ distclean: clean install uninstall: .PHONY: run32 clean32 -ifeq ($(XEN_TARGET_ARCH),x86_64) +ifeq ($(XEN_COMPILE_ARCH),x86_64) run32: $(addsuffix .h,$(TESTCASES)) $(addsuffix -opmask.h,$(OPMASK)) run32 clean32: %32: $(MAKE) -C 32 $* Simply an incomplete renamed that I had done when putting together 52c37f7ab9. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] mm/memory_hotplug: Introduce memory block types
On 26.11.18 13:30, David Hildenbrand wrote:
> On 23.11.18 19:06, Michal Suchánek wrote:
>> On Fri, 23 Nov 2018 12:13:58 +0100
>> David Hildenbrand wrote:
>>
>>> On 28.09.18 17:03, David Hildenbrand wrote:
How to/when to online hotplugged memory is hard to manage for
distributions because different memory types are to be treated differently.
Right now, we need complicated udev rules that e.g. check if we are
running on s390x, on a physical system or on a virtualized system. But
there is also sometimes the demand to really online memory immediately
while adding in the kernel and not to wait for user space to make a
decision. And on virtualized systems there might be different
requirements, depending on "how" the memory was added (and if it will
eventually get unplugged again - DIMM vs. paravirtualized mechanisms).
On the one hand, we have physical systems where we sometimes
want to be able to unplug memory again - e.g. a DIMM - so we have to online
it to the MOVABLE zone optionally. That decision is usually made in user
space.
On the other hand, we have memory that should never be onlined
automatically, only when asked for by an administrator. Such memory only
applies to virtualized environments like s390x, where the concept of
"standby" memory exists. Memory is detected and added during boot, so it
can be onlined when requested by the admininistrator or some tooling.
Only when onlining, memory will be allocated in the hypervisor.
But then, we also have paravirtualized devices (namely xen and hyper-v
balloons), that hotplug memory that will never ever be removed from a
system right now using offline_pages/remove_memory. If at all, this memory
is logically unplugged and handed back to the hypervisor via ballooning.
For paravirtualized devices it is relevant that memory is onlined as
quickly as possible after adding - and that it is added to the NORMAL
zone. Otherwise, it could happen that too much memory in a row is added
(but not onlined), resulting in out-of-memory conditions due to the
additional memory for "struct pages" and friends. MOVABLE zone as well
as delays might be very problematic and lead to crashes (e.g. zone
imbalance).
Therefore, introduce memory block types and online memory depending on
it when adding the memory. Expose the memory type to user space, so user
space handlers can start to process only "normal" memory. Other memory
block types can be ignored. One thing less to worry about in user space.
>>>
>>> So I was looking into alternatives.
>>>
>>> 1. Provide only "normal" and "standby" memory types to user space. This
>>> way user space can make smarter decisions about how to online memory.
>>> Not really sure if this is the right way to go.
>>>
>>>
>>> 2. Use device driver information (as mentioned by Michal S.).
>>>
>>> The problem right now is that there are no drivers for memory block
>>> devices. The "memory" subsystem has no drivers, so the KOBJ_ADD uevent
>>> will not contain a "DRIVER" information and we ave no idea what kind of
>>> memory block device we hold in our hands.
>>>
>>> $ udevadm info -q all -a /sys/devices/system/memory/memory0
>>>
>>> looking at device '/devices/system/memory/memory0':
>>> KERNEL=="memory0"
>>> SUBSYSTEM=="memory"
>>> DRIVER==""
>>> ATTR{online}=="1"
>>> ATTR{phys_device}=="0"
>>> ATTR{phys_index}==""
>>> ATTR{removable}=="0"
>>> ATTR{state}=="online"
>>> ATTR{valid_zones}=="none"
>>>
>>>
>>> If we would provide "fake" drivers for the memory block devices we want
>>> to treat in a special way in user space (e.g. standby memory on s390x),
>>> user space could use that information to make smarter decisions.
>>>
>>> Adding such drivers might work. My suggestion would be to let ordinary
>>> DIMMs be without a driver for now and only special case standby memory
>>> and eventually paravirtualized memory devices (XEN and Hyper-V).
>>>
>>> Any thoughts?
>>
>> If we are going to fake the driver information we may as well add the
>> type attribute and be done with it.
>>
>> I think the problem with the patch was more with the semantic than the
>> attribute itself.
>>
>> What is normal, paravirtualized, and standby memory?
>>
>> I can understand DIMM device, baloon device, or whatever mechanism for
>> adding memory you might have.
>>
>> I can understand "memory designated as standby by the cluster
>> administrator".
>>
>> However, DIMM vs baloon is orthogonal to standby and should not be
>> conflated into one property.
>>
>> paravirtualized means nothing at all in relationship to memory type and
>> the desired online policy to me.
>
> Right, so with whatever we come up, it should allow to make a decision
> in user space about
> - if memory is to be onlined automatically
And I will think about if we really should
Re: [Xen-devel] [PATCH 4/4] xen/arch: Switch local_irq_restore() to being a static inline helper
>>> On 23.11.18 at 17:52, wrote:
> RFC, as I expect this patch to get some objection for removing the IRQ safety
> check, but the only reasons that change was made in e5fc6434d7 was because I
> talk talked into doing it while trying to clean up some unnecessary use of
> magic numbers.
>
> No users are changing any flags (seeing as I've auditing them all in this
> series), and the improvement in emitted code nets us:
Users not changing any flags as per your audit is only one half of it:
I assume you mean the value returned from local_irq_save() gets
passed back unmodified into local_irq_restore(). The other question
is whether intermediately another change to the CPU register has
occurred, which is meant to persist. With Arm also controlling e.g.
enabled status of aborts this way, it may be even more of an issue
there than on x86.
> +static inline void local_irq_restore(unsigned long flags)
> +{
> +asm volatile ( "push %0; popf;" :: "g" (flags) : "memory" );
I'm afraid this is not entirely right (but perhaps benign): Not
all constants are valid to be handed to PUSH. If you want to
allow immediates in the first place, this would need to be "rme",
I think.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] tools/xenstore: domain can sometimes disappear when destroying connection
There is a circular link formed between domain and a connection. In certain
circustances, when conn is freed, domain is also freed, which leads to use
after free when trying to set the conn field in domain to null.
Signed-off-by: Petre Eftime
---
tools/xenstore/xenstored_domain.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/tools/xenstore/xenstored_domain.c
b/tools/xenstore/xenstored_domain.c
index fa6655033a..f085d40476 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -222,6 +222,7 @@ static void domain_cleanup(void)
{
xc_dominfo_t dominfo;
struct domain *domain;
+ struct connection *tmp_conn;
int notify = 0;
again:
@@ -238,8 +239,14 @@ static void domain_cleanup(void)
continue;
}
if (domain->conn) {
- talloc_unlink(talloc_autofree_context(), domain->conn);
+ /*
+* In certain circumstances conn owns domain and
+* domain will be freed when conn is unlinked.
+*/
+ tmp_conn = domain->conn;
domain->conn = NULL;
+
+ talloc_unlink(talloc_autofree_context(), tmp_conn);
notify = 0; /* destroy_domain() fires the watch */
goto again;
}
--
2.16.5
Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar
Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in
Romania. Registration number J22/2621/2005.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/4] xen/arch: Switch local_irq_save() to being a static inline helper
>>> On 23.11.18 at 17:52, wrote:
> ... rather than a macro which writes to its parameter by name. Take the
> opportunity to fold the assignment into the flags declaraion where
> appropriate.
Do you really? Why not ...
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -401,7 +401,7 @@ void *map_domain_page(mfn_t mfn)
> lpae_t pte;
> int i, slot;
>
> -local_irq_save(flags);
> +flags = local_irq_save();
... here, for example? There are a few more cases, I think.
> --- a/xen/include/asm-x86/system.h
> +++ b/xen/include/asm-x86/system.h
> @@ -262,11 +262,15 @@ static inline unsigned long local_save_flags(void)
> return flags;
> }
>
> -#define local_irq_save(x)\
> -({ \
> -x = local_save_flags(); \
> -local_irq_disable(); \
> -})
> +static inline unsigned long local_irq_save(void)
> +{
> +unsigned long flags = local_save_flags();
> +
> +local_irq_disable();
> +
> +return flags;
> +}
Do we really need/want to retain this as a per-arch construct?
Since you touch all instances anyway, do we really want to
stick to its misleading name?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-3.2 v3 10/14] qdev-props: call object_apply_global_props()
On Wed, 7 Nov 2018 16:36:48 +0400
Marc-André Lureau wrote:
> It's now possible to use the common function.
>
> Teach object_apply_global_props() to warn if Error argument is NULL.
>
> Signed-off-by: Marc-André Lureau
> ---
> hw/core/qdev-properties.c | 24 ++--
> qom/object.c | 6 +-
> 2 files changed, 7 insertions(+), 23 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 8728cbab9f..239535a4cb 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1223,28 +1223,8 @@ int qdev_prop_check_globals(void)
>
> void qdev_prop_set_globals(DeviceState *dev)
> {
> -int i;
> -
> -for (i = 0; i < global_props()->len; i++) {
> -GlobalProperty *prop;
> -Error *err = NULL;
> -
> -prop = g_array_index(global_props(), GlobalProperty *, i);
> -if (object_dynamic_cast(OBJECT(dev), prop->driver) == NULL) {
> -continue;
> -}
> -prop->used = true;
> -object_property_parse(OBJECT(dev), prop->value, prop->property,
> &err);
> -if (err != NULL) {
> -error_prepend(&err, "can't apply global %s.%s=%s: ",
> - prop->driver, prop->property, prop->value);
> -if (!dev->hotplugged) {
> -error_propagate(&error_fatal, err);
> -} else {
> -warn_report_err(err);
> -}
> -}
> -}
> +object_apply_global_props(OBJECT(dev), global_props(),
> + dev->hotplugged ? NULL : &error_fatal);
arguably, it's up to caller to decide it warn or not.
I'd leave it warning code out of object_apply_global_props() and let caller do
the job
> }
>
> /* --- 64bit unsigned int 'size' type --- */
> diff --git a/qom/object.c b/qom/object.c
> index 9acdf9e16d..b1a7f70550 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -392,7 +392,11 @@ void object_apply_global_props(Object *obj, GArray
> *props, Error **errp)
> if (err != NULL) {
> error_prepend(&err, "can't apply global %s.%s=%s: ",
>p->driver, p->property, p->value);
> -error_propagate(errp, err);
> +if (errp) {
> +error_propagate(errp, err);
> +} else {
> +warn_report_err(err);
> +}
> }
> }
> }
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/4] xen/arch: Switch local_save_flags() to being a static inline helper
>>> On 23.11.18 at 17:52, wrote:
> --- a/xen/include/asm-x86/system.h
> +++ b/xen/include/asm-x86/system.h
> @@ -253,14 +253,18 @@ static inline unsigned long
> array_index_mask_nospec(unsigned long index,
> /* used when interrupts are already enabled or to shutdown the processor */
> #define halt() asm volatile ( "hlt" : : : "memory" )
>
> -#define local_save_flags(x) \
> -({ \
> -BUILD_BUG_ON(sizeof(x) != sizeof(long)); \
> -asm volatile ( "pushf" __OS " ; pop" __OS " %0" : "=g" (x)); \
> -})
> +static inline unsigned long local_save_flags(void)
> +{
> +unsigned long flags;
> +
> +asm volatile ( "pushf; pop %0;" : "=g" (flags) );
> +
> +return flags;
> +}
Provided this doesn't defeat the current possibility of the compiler
POP-ing directly into a stack variable
Reviewed-by: Jan Beulich
Apart from this I'm a little puzzled by the uppercasing of SAVE in
the Arm asm() comments, but I'll leave that to the Arm maintainers.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] x86_emulator distclean failing
Anyone else seeing this, or is it just me? This was on a fresh pull of staging about 5 mins ago... make -C x86_emulator distclean make[5]: Entering directory '/local/scratch/pauldu/xen/tools/tests/x86_emulator' make -C 32 clean make[6]: Entering directory '/local/scratch/pauldu/xen/tools/tests/x86_emulator/32' make -C 32 clean make[7]: Entering directory '/local/scratch/pauldu/xen/tools/tests/x86_emulator/32' make[7]: *** 32: No such file or directory. Stop. make[7]: Leaving directory '/local/scratch/pauldu/xen/tools/tests/x86_emulator/32' ../Makefile:191: recipe for target 'clean32' failed make[6]: *** [clean32] Error 2 make[6]: Leaving directory '/local/scratch/pauldu/xen/tools/tests/x86_emulator/32' Makefile:191: recipe for target 'clean32' failed make[5]: *** [clean32] Error 2 make[5]: Leaving directory '/local/scratch/pauldu/xen/tools/tests/x86_emulator' /local/scratch/pauldu/xen/tools/tests/../../tools/Rules.mk:254: recipe for target 'subdir-distclean-x86_emulator' failed make[4]: *** [subdir-distclean-x86_emulator] Error 2 make[4]: Leaving directory '/local/scratch/pauldu/xen/tools/tests' /local/scratch/pauldu/xen/tools/tests/../../tools/Rules.mk:246: recipe for target 'subdirs-distclean' failed make[3]: *** [subdirs-distclean] Error 2 make[3]: Leaving directory '/local/scratch/pauldu/xen/tools/tests' /local/scratch/pauldu/xen/tools/../tools/Rules.mk:254: recipe for target 'subdir-distclean-tests' failed make[2]: *** [subdir-distclean-tests] Error 2 make[2]: Leaving directory '/local/scratch/pauldu/xen/tools' /local/scratch/pauldu/xen/tools/../tools/Rules.mk:246: recipe for target 'subdirs-distclean' failed make[1]: *** [subdirs-distclean] Error 2 make[1]: Leaving directory '/local/scratch/pauldu/xen/tools' Makefile:242: recipe for target 'distclean-tools' failed make: *** [distclean-tools] Error 2 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/4] xen/arch: Switch local_*_is_enabled() predicates to return bool
>>> On 23.11.18 at 17:52, wrote: > No functional change, as the value returned was previously always 0 or 1. > While altering these, insert blank lines where appropriate and drop the > now-redundant !! from x86's local_irq_is_enabled(). > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] mm: make opt_bootscrub non-init
>>> On 26.11.18 at 13:49, wrote: > On Mon, Nov 26, 2018 at 05:18:20AM -0700, Jan Beulich wrote: >> Furthermore I doubt writing this down would help, because for >> such apparently simple things no-one goes hunt for related >> documentation. I think the only future proof course of action >> would be to port Linux'es section mismatch handling and stop >> allowing problematic cross references. That approach has >> downsides though, which is why I'm not going to advocate it. > > Is Sparse the only option in this regard? I'm specifically not talking about sparse, but about Linux'es logic in scripts/mod/modpost.c. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V9 7/7] p2m: change_range_type: Only invalidate mapped gfns
>>> On 22.11.18 at 12:40, wrote: > change_range_type() invalidates gfn ranges to lazily change the type Would you mind correcting the function name here? > of a range of gfns, and also modifies the logdirty rangesets of that > p2m. At the moment, it clips both down by the hostp2m. > > While this will result in correct behavior, it's not entirely efficient, > since invalidated entries outside that range will, on fault, simply be > modified back to "empty" before faulting normally again. > > Separate out the calculation of the two ranges. Keep using the > hostp2m's max_mapped_pfn to clip the logdirty ranges, but use the > current p2m's max_mapped_pfn to further clip the invalidation range > for alternate p2ms. > > Signed-off-by: George Dunlap Subject to possible adjustments due to changes to patch 6 Reviewed-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] mm: make opt_bootscrub non-init
On Mon, Nov 26, 2018 at 05:18:20AM -0700, Jan Beulich wrote: > >>> On 26.11.18 at 13:04, wrote: > > On Mon, Nov 26, 2018 at 03:06:16AM -0700, Jan Beulich wrote: > >> >>> On 23.11.18 at 15:30, wrote: > >> > LLVM code generation can attempt to load from a variable in the next > >> > condition of an expression under certain circumstances, thus turning > >> > the following condition: > >> > > >> > if ( system_state < SYS_STATE_active && opt_bootscrub == BOOTSCRUB_IDLE ) > >> > > >> > Into: > >> > > >> > 0x82d080223967 <+103>: cmpl $0x3,0x37b032(%rip) # > >> > 0x82d08059e9a0 > > > >> > 0x82d08022396e <+110>: setb -0x29(%rbp) > >> > 0x82d080223972 <+114>: cmpl $0x2,0x228a8b(%rip) # > >> > 0x82d08044c404 > > > >> > > >> > Such code will trigger a page fault if system_state >= > >> > SYS_STATE_active because opt_bootscrub will be unmapped. > >> > > >> > Fix this by making opt_bootscrub non-init, thus preventing the page > >> > fault. The LLVM bug with the discussion about this issue can be found > >> > at: > >> > > >> > https://bugs.llvm.org/show_bug.cgi?id=39707 > >> > > >> > I haven't been able to find any other instances of such conditional > >> > expression that uses system_state together with an init variable or > >> > function. > >> > > >> > Signed-off-by: Roger Pau Monné > >> > >> I can accept this as a band-aid, so I'm not going to nack it, but > >> I don't view this as a feasible solution to the problem. That's in > >> particular because nothing is done at all to prevent future > >> similar issues. Even worse, ... > > > > I'm not sure what's the best way to prevent future issues. Should this > > be mentioned in the coding style? That doesn't seems like the best > > place, but I'm not sure where else could this be documented. > > There was some vaguely similar discussion a little while ago, and > there iirc we had also agreed that the point there (which I don't > recall) is not a style thing. Same here: We're talking about a > correctness issue, not a stylistic one. Hence indeed a separate > document would be needed, but none of the existing ones looks > to be a good fit. > > Furthermore I doubt writing this down would help, because for > such apparently simple things no-one goes hunt for related > documentation. I think the only future proof course of action > would be to port Linux'es section mismatch handling and stop > allowing problematic cross references. That approach has > downsides though, which is why I'm not going to advocate it. Is Sparse the only option in this regard? I think Andrew had played with Sparse on Xen before? Albeit Linux and Xen share some similarities, I'm afraid that using Sparse would mean either modifying Sparse itself, or modifying Xen to match Linux. Are there any other options? Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/tools: Fix gen-cpuid.py's ability to report errors
>>> On 26.11.18 at 13:17, wrote: > On Mon, Nov 26, 2018 at 12:03:07PM +, Andrew Cooper wrote: >> c/s 18596903 "xen/tools: support Python 2 and Python 3" unfortunately >> introduced a TypeError when changing how Fail exceptions were printed: >> >> /local/xen.git/xen/../xen/tools/gen-cpuid.py:Traceback (most recent call >> last): >> File "/local/xen.git/xen/../xen/tools/gen-cpuid.py", line 483, in >> >> sys.stderr.write(e) >> TypeError: expected a character buffer object >> >> Coerce e to a string before printing. While changing this, fold the three >> write() calls making up the line into a single one, and take the opportunity >> to neaten the output. >> >> A sample error is: >> >> /local/xen.git/xen/tools/gen-cpuid.py: Fail: Aliased value between FOO and >> BAR >> >> Signed-off-by: Andrew Cooper > > Acked-by: Wei Liu 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] mm: make opt_bootscrub non-init
>>> On 26.11.18 at 13:25, wrote:
> May I ask what are the downsides? Do you expect a lot of false positive?
Having to split code paths, to introduce redundancy, or to move
code/data out of .init.* that could in fact live there are all
possible issues. Plus the __ref{,data} annotation that Linux has
also invite for abuse (at which point we'd be back to where we
currently are, just at perhaps a smaller scope).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.11-testing test] 130752: trouble: blocked/broken/fail/pass
flight 130752 xen-4.11-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/130752/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-pvopsbroken build-arm64-xsm broken build-arm64 broken Regressions which are regarded as allowable (not blocking): build-arm64-xsm 2 hosts-allocate broken REGR. vs. 129720 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 129720 build-arm64 2 hosts-allocate broken REGR. vs. 129720 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a build-arm64-xsm 3 capture-logs broken blocked in 129720 build-arm64-pvops 3 capture-logs broken blocked in 129720 build-arm64 3 capture-logs broken blocked in 129720 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 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-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-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-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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-i386-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-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-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-win7-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-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-qemut-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-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass version targeted for testing: xen 49caabf2584a2
Re: [Xen-devel] [PATCH RFC] mm/memory_hotplug: Introduce memory block types
On 23.11.18 19:06, Michal Suchánek wrote:
> On Fri, 23 Nov 2018 12:13:58 +0100
> David Hildenbrand wrote:
>
>> On 28.09.18 17:03, David Hildenbrand wrote:
>>> How to/when to online hotplugged memory is hard to manage for
>>> distributions because different memory types are to be treated differently.
>>> Right now, we need complicated udev rules that e.g. check if we are
>>> running on s390x, on a physical system or on a virtualized system. But
>>> there is also sometimes the demand to really online memory immediately
>>> while adding in the kernel and not to wait for user space to make a
>>> decision. And on virtualized systems there might be different
>>> requirements, depending on "how" the memory was added (and if it will
>>> eventually get unplugged again - DIMM vs. paravirtualized mechanisms).
>>>
>>> On the one hand, we have physical systems where we sometimes
>>> want to be able to unplug memory again - e.g. a DIMM - so we have to online
>>> it to the MOVABLE zone optionally. That decision is usually made in user
>>> space.
>>>
>>> On the other hand, we have memory that should never be onlined
>>> automatically, only when asked for by an administrator. Such memory only
>>> applies to virtualized environments like s390x, where the concept of
>>> "standby" memory exists. Memory is detected and added during boot, so it
>>> can be onlined when requested by the admininistrator or some tooling.
>>> Only when onlining, memory will be allocated in the hypervisor.
>>>
>>> But then, we also have paravirtualized devices (namely xen and hyper-v
>>> balloons), that hotplug memory that will never ever be removed from a
>>> system right now using offline_pages/remove_memory. If at all, this memory
>>> is logically unplugged and handed back to the hypervisor via ballooning.
>>>
>>> For paravirtualized devices it is relevant that memory is onlined as
>>> quickly as possible after adding - and that it is added to the NORMAL
>>> zone. Otherwise, it could happen that too much memory in a row is added
>>> (but not onlined), resulting in out-of-memory conditions due to the
>>> additional memory for "struct pages" and friends. MOVABLE zone as well
>>> as delays might be very problematic and lead to crashes (e.g. zone
>>> imbalance).
>>>
>>> Therefore, introduce memory block types and online memory depending on
>>> it when adding the memory. Expose the memory type to user space, so user
>>> space handlers can start to process only "normal" memory. Other memory
>>> block types can be ignored. One thing less to worry about in user space.
>>>
>>
>> So I was looking into alternatives.
>>
>> 1. Provide only "normal" and "standby" memory types to user space. This
>> way user space can make smarter decisions about how to online memory.
>> Not really sure if this is the right way to go.
>>
>>
>> 2. Use device driver information (as mentioned by Michal S.).
>>
>> The problem right now is that there are no drivers for memory block
>> devices. The "memory" subsystem has no drivers, so the KOBJ_ADD uevent
>> will not contain a "DRIVER" information and we ave no idea what kind of
>> memory block device we hold in our hands.
>>
>> $ udevadm info -q all -a /sys/devices/system/memory/memory0
>>
>> looking at device '/devices/system/memory/memory0':
>> KERNEL=="memory0"
>> SUBSYSTEM=="memory"
>> DRIVER==""
>> ATTR{online}=="1"
>> ATTR{phys_device}=="0"
>> ATTR{phys_index}==""
>> ATTR{removable}=="0"
>> ATTR{state}=="online"
>> ATTR{valid_zones}=="none"
>>
>>
>> If we would provide "fake" drivers for the memory block devices we want
>> to treat in a special way in user space (e.g. standby memory on s390x),
>> user space could use that information to make smarter decisions.
>>
>> Adding such drivers might work. My suggestion would be to let ordinary
>> DIMMs be without a driver for now and only special case standby memory
>> and eventually paravirtualized memory devices (XEN and Hyper-V).
>>
>> Any thoughts?
>
> If we are going to fake the driver information we may as well add the
> type attribute and be done with it.
>
> I think the problem with the patch was more with the semantic than the
> attribute itself.
>
> What is normal, paravirtualized, and standby memory?
>
> I can understand DIMM device, baloon device, or whatever mechanism for
> adding memory you might have.
>
> I can understand "memory designated as standby by the cluster
> administrator".
>
> However, DIMM vs baloon is orthogonal to standby and should not be
> conflated into one property.
>
> paravirtualized means nothing at all in relationship to memory type and
> the desired online policy to me.
Right, so with whatever we come up, it should allow to make a decision
in user space about
- if memory is to be onlined automatically
- to which zone memory is to be onlined
The rules are encoded in user space, the type will allow to make a
decision. One important part will be if the memory can eventually be
offlined + remo
Re: [Xen-devel] [PATCH] mm: make opt_bootscrub non-init
Hi Jan, On 26/11/2018 12:18, Jan Beulich wrote: On 26.11.18 at 13:04, wrote: On Mon, Nov 26, 2018 at 03:06:16AM -0700, Jan Beulich wrote: On 23.11.18 at 15:30, wrote: LLVM code generation can attempt to load from a variable in the next condition of an expression under certain circumstances, thus turning the following condition: if ( system_state < SYS_STATE_active && opt_bootscrub == BOOTSCRUB_IDLE ) Into: 0x82d080223967 <+103>: cmpl $0x3,0x37b032(%rip) # 0x82d08059e9a0 0x82d08022396e <+110>: setb -0x29(%rbp) 0x82d080223972 <+114>: cmpl $0x2,0x228a8b(%rip) # 0x82d08044c404 Such code will trigger a page fault if system_state >= SYS_STATE_active because opt_bootscrub will be unmapped. Fix this by making opt_bootscrub non-init, thus preventing the page fault. The LLVM bug with the discussion about this issue can be found at: https://bugs.llvm.org/show_bug.cgi?id=39707 I haven't been able to find any other instances of such conditional expression that uses system_state together with an init variable or function. Signed-off-by: Roger Pau Monné I can accept this as a band-aid, so I'm not going to nack it, but I don't view this as a feasible solution to the problem. That's in particular because nothing is done at all to prevent future similar issues. Even worse, ... I'm not sure what's the best way to prevent future issues. Should this be mentioned in the coding style? That doesn't seems like the best place, but I'm not sure where else could this be documented. There was some vaguely similar discussion a little while ago, and there iirc we had also agreed that the point there (which I don't recall) is not a style thing. Same here: We're talking about a correctness issue, not a stylistic one. Hence indeed a separate document would be needed, but none of the existing ones looks to be a good fit. Furthermore I doubt writing this down would help, because for such apparently simple things no-one goes hunt for related documentation. I think the only future proof course of action would be to port Linux'es section mismatch handling and stop allowing problematic cross references. That approach has downsides though, which is why I'm not going to advocate it. May I ask what are the downsides? Do you expect a lot of false positive? 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] tools: set Dom0 UUID if requested
On Mon, Nov 26, 2018 at 10:40:44AM +, Wei Liu wrote: > Introduce XEN_DOM0_UUID in Xen's global configuration file. Make > xen-init-dom0 accept an extra argument for UUID. > > Also switch xs_open error message to use perror. > > Signed-off-by: Wei Liu > Reviewed-by: Juergen Gross > Reviewed-by: Sergey Dyasli > --- > v4: > 1. use perror > 2. added Rb from Juergen and Sergey as the changes are only cosmetic. Reviewed-by: Anthony PERARD -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 4/5] x86/msr: Handle MSR_TSC_AUX consistently for PV and HVM guests
>>> On 20.11.18 at 15:37, wrote: > With PVRDTSCP mode removed, handling of MSR_TSC_AUX can move into the common > code. Move its storage into struct vcpu_msrs (dropping the HVM-specific > msr_tsc_aux), and add an RDPID feature check as this bit also enumerates the > presence of the MSR. > > Introduce cpu_has_rdpid along with the synthesized cpu_has_msr_tsc_aux to > correct the context switch paths, as MSR_TSC_AUX is enumerated by either > RDTSCP or RDPID. > > Drop hvm_msr_tsc_aux() entirely, and use v->arch.msrs->tsc_aux directly. > Update hvm_load_cpu_ctxt() to check that the incoming ctxt.msr_tsc_aux isn't > out of range. In practice, no previous version of Xen ever wrote an > out-of-range value. Add MSR_TSC_AUX to the list of MSRs migrated for PV > guests, but leave the HVM path using the existing space in hvm_hw_cpu. > > Signed-off-by: Andrew Cooper >[...] > --- > xen/arch/x86/domain.c| 5 ++--- > xen/arch/x86/domctl.c| 2 ++ > xen/arch/x86/hvm/hvm.c | 22 +- > xen/arch/x86/hvm/svm/svm.c | 6 +++--- > xen/arch/x86/hvm/vmx/vmx.c | 6 +++--- > xen/arch/x86/msr.c | 18 ++ > xen/arch/x86/pv/emul-priv-op.c | 4 > xen/include/asm-x86/cpufeature.h | 5 + > xen/include/asm-x86/hvm/hvm.h| 6 -- > xen/include/asm-x86/hvm/vcpu.h | 1 - > xen/include/asm-x86/msr.h| 9 + > 11 files changed, 51 insertions(+), 33 deletions(-) Btw. by the end of this series wouldn't you agree docs/misc/pvrdtscp.c should be gone? And won't docs/man/xen-tsc-mode.pod.7 need adjustment too? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] mm: make opt_bootscrub non-init
>>> On 26.11.18 at 13:04, wrote: > On Mon, Nov 26, 2018 at 03:06:16AM -0700, Jan Beulich wrote: >> >>> On 23.11.18 at 15:30, wrote: >> > LLVM code generation can attempt to load from a variable in the next >> > condition of an expression under certain circumstances, thus turning >> > the following condition: >> > >> > if ( system_state < SYS_STATE_active && opt_bootscrub == BOOTSCRUB_IDLE ) >> > >> > Into: >> > >> > 0x82d080223967 <+103>: cmpl $0x3,0x37b032(%rip) # 0x82d08059e9a0 > >> > 0x82d08022396e <+110>: setb -0x29(%rbp) >> > 0x82d080223972 <+114>: cmpl $0x2,0x228a8b(%rip) # 0x82d08044c404 > >> > >> > Such code will trigger a page fault if system_state >= >> > SYS_STATE_active because opt_bootscrub will be unmapped. >> > >> > Fix this by making opt_bootscrub non-init, thus preventing the page >> > fault. The LLVM bug with the discussion about this issue can be found >> > at: >> > >> > https://bugs.llvm.org/show_bug.cgi?id=39707 >> > >> > I haven't been able to find any other instances of such conditional >> > expression that uses system_state together with an init variable or >> > function. >> > >> > Signed-off-by: Roger Pau Monné >> >> I can accept this as a band-aid, so I'm not going to nack it, but >> I don't view this as a feasible solution to the problem. That's in >> particular because nothing is done at all to prevent future >> similar issues. Even worse, ... > > I'm not sure what's the best way to prevent future issues. Should this > be mentioned in the coding style? That doesn't seems like the best > place, but I'm not sure where else could this be documented. There was some vaguely similar discussion a little while ago, and there iirc we had also agreed that the point there (which I don't recall) is not a style thing. Same here: We're talking about a correctness issue, not a stylistic one. Hence indeed a separate document would be needed, but none of the existing ones looks to be a good fit. Furthermore I doubt writing this down would help, because for such apparently simple things no-one goes hunt for related documentation. I think the only future proof course of action would be to port Linux'es section mismatch handling and stop allowing problematic cross references. That approach has downsides though, which is why I'm not going to advocate it. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/tools: Fix gen-cpuid.py's ability to report errors
On Mon, Nov 26, 2018 at 12:03:07PM +, Andrew Cooper wrote: > c/s 18596903 "xen/tools: support Python 2 and Python 3" unfortunately > introduced a TypeError when changing how Fail exceptions were printed: > > /local/xen.git/xen/../xen/tools/gen-cpuid.py:Traceback (most recent call > last): > File "/local/xen.git/xen/../xen/tools/gen-cpuid.py", line 483, in > sys.stderr.write(e) > TypeError: expected a character buffer object > > Coerce e to a string before printing. While changing this, fold the three > write() calls making up the line into a single one, and take the opportunity > to neaten the output. > > A sample error is: > > /local/xen.git/xen/tools/gen-cpuid.py: Fail: Aliased value between FOO and > BAR > > Signed-off-by: Andrew Cooper Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] drm_gem_get_pages and proper flushing/coherency
Hello, all! My driver (Xen para-virtualized frontend) in some scenarios uses drm_gem_get_pages for allocating backing storage for dumb buffers. There are use-cases which showed some artifacts on the screen (modetest, other) which were worked around by flushing pages of the buffer on page flip with drm_clflush_pages. But, the problem here is that drm_clflush_pages is not available on ARM platforms (it is a NOP) and doing flushes on every page flip seems to be non-optimal. Other drivers that use drm_gem_get_pages seem to use DMA map/unmap on the shmem backed buffer (this is from where drm_gem_get_pages allocates the pages) and this is an obvious approach as the buffer needs to be shared with real HW for DMA - please correct me if my understanding here is wrong. This is the part I missed in my implementation as I don't really have a HW device which needs DMA, but a backend running in a different Xen domain. Thus, as the buffer is backed with cachable pages the backend may see artifacts on its side. I am looking for some advices on what would be the best option to make sure dumb buffers are not flushed every page flip and still the memory remains coherent to the backend. I have implemented a DMA map/unmap of the shmem pages on GEM object creation/destruction and this does solve the problem, but as the backend is not really a DMA device this is a bit misleading. Is there any other (more?) suitable/preferable way(s) to achieve the same? Thank you, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V9 5/7] x86/altp2m: fix display frozen when switching to a new view early
>>> On 22.11.18 at 12:40, wrote:
> @@ -956,18 +1003,14 @@ int p2m_change_type_one(struct domain *d, unsigned
> long gfn_l,
> }
>
> /* Modify the p2m type of a range of gfns from ot to nt. */
> -void p2m_change_type_range(struct domain *d,
> - unsigned long start, unsigned long end,
> - p2m_type_t ot, p2m_type_t nt)
> +static void change_type_range(struct p2m_domain *p2m,
> + unsigned long start, unsigned long end,
> + p2m_type_t ot, p2m_type_t nt)
> {
> unsigned long gfn = start;
> -struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +struct domain *d = p2m->domain;
> int rc = 0;
>
> -ASSERT(ot != nt);
> -ASSERT(p2m_is_changeable(ot) && p2m_is_changeable(nt));
> -
> -p2m_lock(p2m);
> p2m->defer_nested_flush = 1;
While reviewing the first of George's patches I started wondering
why you keep this and ...
> @@ -1011,23 +1054,54 @@ void p2m_change_type_range(struct domain *d,
> p2m->defer_nested_flush = 0;
> if ( nestedhvm_enabled(d) )
> p2m_flush_nestedp2m(d);
... this here, such that it would be called multiple times, despite
p2m_flush_nestedp2m() taking a struct domain, not a struct p2m
as input.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V9 6/7] p2m: Always use hostp2m when clipping rangesets
>>> On 22.11.18 at 12:40, wrote:
> The logdirty rangesets of the altp2ms need to be kept in sync with the
> hostp2m. This means when iterating through the altp2ms, we need to
> use the host p2m to clip the rangeset, not the indiviual altp2m's
> value.
>
> This change also:
>
> - Documents that the end is non-inclusive
>
> - Calculates an "inclusive" value for the end once, rather than
> open-coding the modification, and (worse) back-modifying updates so
> that the calculation ends up correct
>
> - Clarifies the logic deciding whether to call
> change_entry_type_global() or change_entry_type_range()
>
> - Handles the case where start >= hostp2m->max_mapped_pfn
>
> Signed-off-by: George Dunlap
>
> ---
> CC: George Dunlap
> CC: Jan Beulich
> CC: Andrew Cooper
> CC: Wei Liu
> CC: "Roger Pau Monné"
>
> ---
> RFC: Wasn't sure what the best thing was to do if start >=
> host_max_pfn. We silently clip the logdirty rangeset to
> max_mapped_pfn, and the chosen behavior seems consistent with that.
> But it seems like such a request would almost certainly be a bug
> somewhere that people might like to find out about.
Warning about this might be worthwhile as a first step, but I
don't think we should converted this to some form of error
right away, unless the "almost" could be dropped from your
explanation.
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1002,32 +1002,44 @@ int p2m_change_type_one(struct domain *d, unsigned
> long gfn_l,
> return rc;
> }
>
> -/* Modify the p2m type of a range of gfns from ot to nt. */
> +/* Modify the p2m type of [start, end) from ot to nt. */
> static void change_type_range(struct p2m_domain *p2m,
>unsigned long start, unsigned long end,
>p2m_type_t ot, p2m_type_t nt)
> {
> -unsigned long gfn = start;
> +unsigned long rangeset_start, rangeset_end;
> struct domain *d = p2m->domain;
> +unsigned long host_max_pfn = p2m_get_hostp2m(d)->max_mapped_pfn;
> int rc = 0;
>
> +rangeset_start = start;
You never change this value - do you really want to introduce
this redundant variable? I also don't see why you couldn't simply
re-purpose "end".
> +rangeset_end = end - 1;
> +
> +/* Always clip the rangeset down to the host p2m */
> +if ( unlikely(rangeset_end > host_max_pfn) )
> +rangeset_end = host_max_pfn;
> +
> +/* If the requested range is out of scope, return doing nothing */
> +if ( rangeset_start > rangeset_end )
> +return;
The original idea behind not using such an early return was to
make sure the subsequent rangeset manipulations would not
be skipped. If you're convinced this is unnecessary, could you
please reason about this in the description? After all the
logdirty rangeset also needs to cope with subsequently
increasing ->max_mapped_pfn.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] mm: make opt_bootscrub non-init
On Mon, Nov 26, 2018 at 03:06:16AM -0700, Jan Beulich wrote:
> >>> On 23.11.18 at 15:30, wrote:
> > LLVM code generation can attempt to load from a variable in the next
> > condition of an expression under certain circumstances, thus turning
> > the following condition:
> >
> > if ( system_state < SYS_STATE_active && opt_bootscrub == BOOTSCRUB_IDLE )
> >
> > Into:
> >
> > 0x82d080223967 <+103>: cmpl $0x3,0x37b032(%rip) # 0x82d08059e9a0
> >
> > 0x82d08022396e <+110>: setb -0x29(%rbp)
> > 0x82d080223972 <+114>: cmpl $0x2,0x228a8b(%rip) # 0x82d08044c404
> >
> >
> > Such code will trigger a page fault if system_state >=
> > SYS_STATE_active because opt_bootscrub will be unmapped.
> >
> > Fix this by making opt_bootscrub non-init, thus preventing the page
> > fault. The LLVM bug with the discussion about this issue can be found
> > at:
> >
> > https://bugs.llvm.org/show_bug.cgi?id=39707
> >
> > I haven't been able to find any other instances of such conditional
> > expression that uses system_state together with an init variable or
> > function.
> >
> > Signed-off-by: Roger Pau Monné
>
> I can accept this as a band-aid, so I'm not going to nack it, but
> I don't view this as a feasible solution to the problem. That's in
> particular because nothing is done at all to prevent future
> similar issues. Even worse, ...
I'm not sure what's the best way to prevent future issues. Should this
be mentioned in the coding style? That doesn't seems like the best
place, but I'm not sure where else could this be documented.
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -166,7 +166,7 @@ enum bootscrub_mode {
> > BOOTSCRUB_ON,
> > BOOTSCRUB_IDLE,
> > };
> > -static enum bootscrub_mode __initdata opt_bootscrub = BOOTSCRUB_IDLE;
> > +static enum bootscrub_mode opt_bootscrub = BOOTSCRUB_IDLE;
>
> ... no comment gets added here, which will make it rather likely
> for someone to notice the missing __initdata and add it back. For
> such a "trivial" change I wouldn't expect people to go do extra
> archeology.
>
> As an aside - __read_mostly should be added here instead.
I can add a comment, and yes, read_mostly should be used.
> Furthermore, while I trust your audit wrt system_state
> accesses, these aren't the only potentially problematic ones.
> For example in x86 specific code we pass around a boolean
> indicating whether we're initializing the BSP or an AP. In other
> places we compare the passed around struct cpuinfo_x86
> pointer to the address of boot_cpu_data to tell apart the two
> cases. The first example I can spot is guarding a function call
> (to mcetelem_init()), so not a problem here, but I wouldn't
> bet there are no __initdata variable accesses anywhere.
Hm, I guess we would need to use some kind of tool in order to detect
such accesses, Sparse maybe? But then we would likely have to match the
same rules as Linux, or modify Sparse to match our rules.
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen/tools: Fix gen-cpuid.py's ability to report errors
c/s 18596903 "xen/tools: support Python 2 and Python 3" unfortunately
introduced a TypeError when changing how Fail exceptions were printed:
/local/xen.git/xen/../xen/tools/gen-cpuid.py:Traceback (most recent call
last):
File "/local/xen.git/xen/../xen/tools/gen-cpuid.py", line 483, in
sys.stderr.write(e)
TypeError: expected a character buffer object
Coerce e to a string before printing. While changing this, fold the three
write() calls making up the line into a single one, and take the opportunity
to neaten the output.
A sample error is:
/local/xen.git/xen/tools/gen-cpuid.py: Fail: Aliased value between FOO and BAR
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
---
xen/tools/gen-cpuid.py | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index 31fdee9..27569bd 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -479,9 +479,8 @@ if __name__ == "__main__":
sys.exit(main())
except Fail:
e = sys.exc_info()[1]
-sys.stderr.write("%s:" % (sys.argv[0],))
-sys.stderr.write(e)
-sys.stderr.write("\n")
+sys.stderr.write("%s: Fail: %s\n" %
+ (os.path.abspath(sys.argv[0]), str(e)))
sys.exit(1)
except SystemExit:
e = sys.exc_info()[1]
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 2/6] vpci: fix deferral of long operations
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 26 November 2018 11:42
> To: Paul Durrant
> Cc: Julien Grall ; Andrew Cooper
> ; Roger Pau Monne ; Wei
> Liu ; George Dunlap ; Ian
> Jackson ; Stefano Stabellini
> ; xen-devel ;
> Konrad Rzeszutek Wilk ; Tim (Xen.org)
>
> Subject: Re: [PATCH v5 2/6] vpci: fix deferral of long operations
>
> >>> On 20.11.18 at 17:01, wrote:
> > Current logic to handle long running operations is flawed because it
> > doesn't prevent the guest vcpu from running. Fix this by raising a
> > scheduler softirq when preemption is required, so that the do_softirq
> > call in the guest entry path performs a rescheduling. Also move the
> > call to vpci_process_pending into handle_hvm_io_completion, together
> > with the IOREQ code that handles pending IO instructions.
> >
> > Note that a scheduler softirq is also raised when the long running
> > operation is queued in order to prevent the guest vcpu from resuming
> > execution.
> >
> > Signed-off-by: Roger Pau Monné
> > ---
> > Cc: Paul Durrant
> > Cc: Jan Beulich
> > Cc: Andrew Cooper
> > Cc: Wei Liu
> > Cc: George Dunlap
> > Cc: Ian Jackson
> > Cc: Julien Grall
> > Cc: Konrad Rzeszutek Wilk
> > Cc: Stefano Stabellini
> > Cc: Tim Deegan
> > ---
> > Changes since v4:
> > - Add a comment to clarify defer_map raising a scheduler softirq.
> > - Reword commit message.
> > - Raise the scheduler softirq in handle_hvm_io_completion rather than
> >vpci_process_pending.
> >
> > Changes since v3:
> > - Don't use a tasklet.
> > ---
> > xen/arch/x86/hvm/ioreq.c | 9 ++---
>
> Paul - I'll take the liberty and reinstate your v4 R-b for v5.
>
That's fine.
Paul
> Jan
>
> > xen/drivers/vpci/header.c | 5 +
> > 2 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> > index a56d634f31..71f23227e6 100644
> > --- a/xen/arch/x86/hvm/ioreq.c
> > +++ b/xen/arch/x86/hvm/ioreq.c
> > @@ -85,9 +85,6 @@ bool hvm_io_pending(struct vcpu *v)
> > struct hvm_ioreq_server *s;
> > unsigned int id;
> >
> > -if ( has_vpci(d) && vpci_process_pending(v) )
> > -return true;
> > -
> > FOR_EACH_IOREQ_SERVER(d, id, s)
> > {
> > struct hvm_ioreq_vcpu *sv;
> > @@ -186,6 +183,12 @@ bool handle_hvm_io_completion(struct vcpu *v)
> > enum hvm_io_completion io_completion;
> > unsigned int id;
> >
> > +if ( has_vpci(d) && vpci_process_pending(v) )
> > +{
> > +raise_softirq(SCHEDULE_SOFTIRQ);
> > +return false;
> > +}
> > +
> > FOR_EACH_IOREQ_SERVER(d, id, s)
> > {
> > struct hvm_ioreq_vcpu *sv;
> > diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> > index 39dffb21fb..c9bdc2ced3 100644
> > --- a/xen/drivers/vpci/header.c
> > +++ b/xen/drivers/vpci/header.c
> > @@ -184,6 +184,11 @@ static void defer_map(struct domain *d, struct
> pci_dev
> > *pdev,
> > curr->vpci.mem = mem;
> > curr->vpci.cmd = cmd;
> > curr->vpci.rom_only = rom_only;
> > +/*
> > + * Raise a scheduler softirq in order to prevent the guest from
> > resuming
> > + * execution with pending mapping operations.
> > + */
> > +raise_softirq(SCHEDULE_SOFTIRQ);
> > }
> >
> > static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool
> > rom_only)
> > --
> > 2.19.1
>
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] amd-iommu: replace occurrences of u with uint_t...
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 26 November 2018 11:49
> To: Paul Durrant
> Cc: Brian Woods ; Suravee Suthikulpanit
> ; xen-devel de...@lists.xenproject.org>
> Subject: Re: [Xen-devel] [PATCH v2 2/2] amd-iommu: replace occurrences of
> u with uint_t...
>
> >>> On 26.11.18 at 12:32, wrote:
> > @@ -306,7 +307,8 @@ void iommu_dte_set_guest_cr3(u32 *dte, u16 dom_id,
> u64 gcr3,
> > uint64_t amd_iommu_get_address_from_pte(void *pte)
> > {
> > uint32_t *entry = pte;
> > -uint64_t addr_lo, addr_hi, ptr;
> > +uint32_t addr_lo, addr_hi;
> > +uint64_t ptr;
> >
> > addr_lo = get_field_from_reg_u32(entry[0],
> > IOMMU_PTE_ADDR_LOW_MASK,
> > @@ -316,29 +318,30 @@ uint64_t amd_iommu_get_address_from_pte(void *pte)
> > IOMMU_PTE_ADDR_HIGH_MASK,
> > IOMMU_PTE_ADDR_HIGH_SHIFT);
> >
> > -ptr = (addr_hi << 32) | (addr_lo << PAGE_SHIFT);
> > +ptr = ((uint64_t)addr_hi << 32) |
> > + ((uint64_t)addr_lo << PAGE_SHIFT);
> > return ptr;
> > }
>
> This is actually an increase in casts. I assume you've done this for
> consistency, but it would have been nice if the description said so.
Yes, it was indeed for consistency. Still a net reduction overall but, yes, the
comment could have been clearer.
> Anyway,
> Reviewed-by: Jan Beulich
>
Thanks,
Paul
> Jan
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] amd-iommu: replace occurrences of u with uint_t...
>>> On 26.11.18 at 12:32, wrote:
> @@ -306,7 +307,8 @@ void iommu_dte_set_guest_cr3(u32 *dte, u16 dom_id, u64
> gcr3,
> uint64_t amd_iommu_get_address_from_pte(void *pte)
> {
> uint32_t *entry = pte;
> -uint64_t addr_lo, addr_hi, ptr;
> +uint32_t addr_lo, addr_hi;
> +uint64_t ptr;
>
> addr_lo = get_field_from_reg_u32(entry[0],
> IOMMU_PTE_ADDR_LOW_MASK,
> @@ -316,29 +318,30 @@ uint64_t amd_iommu_get_address_from_pte(void *pte)
> IOMMU_PTE_ADDR_HIGH_MASK,
> IOMMU_PTE_ADDR_HIGH_SHIFT);
>
> -ptr = (addr_hi << 32) | (addr_lo << PAGE_SHIFT);
> +ptr = ((uint64_t)addr_hi << 32) |
> + ((uint64_t)addr_lo << PAGE_SHIFT);
> return ptr;
> }
This is actually an increase in casts. I assume you've done this for
consistency, but it would have been nice if the description said so.
Anyway,
Reviewed-by: Jan Beulich
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 2/6] vpci: fix deferral of long operations
>>> On 20.11.18 at 17:01, wrote:
> Current logic to handle long running operations is flawed because it
> doesn't prevent the guest vcpu from running. Fix this by raising a
> scheduler softirq when preemption is required, so that the do_softirq
> call in the guest entry path performs a rescheduling. Also move the
> call to vpci_process_pending into handle_hvm_io_completion, together
> with the IOREQ code that handles pending IO instructions.
>
> Note that a scheduler softirq is also raised when the long running
> operation is queued in order to prevent the guest vcpu from resuming
> execution.
>
> Signed-off-by: Roger Pau Monné
> ---
> Cc: Paul Durrant
> Cc: Jan Beulich
> Cc: Andrew Cooper
> Cc: Wei Liu
> Cc: George Dunlap
> Cc: Ian Jackson
> Cc: Julien Grall
> Cc: Konrad Rzeszutek Wilk
> Cc: Stefano Stabellini
> Cc: Tim Deegan
> ---
> Changes since v4:
> - Add a comment to clarify defer_map raising a scheduler softirq.
> - Reword commit message.
> - Raise the scheduler softirq in handle_hvm_io_completion rather than
>vpci_process_pending.
>
> Changes since v3:
> - Don't use a tasklet.
> ---
> xen/arch/x86/hvm/ioreq.c | 9 ++---
Paul - I'll take the liberty and reinstate your v4 R-b for v5.
Jan
> xen/drivers/vpci/header.c | 5 +
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> index a56d634f31..71f23227e6 100644
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -85,9 +85,6 @@ bool hvm_io_pending(struct vcpu *v)
> struct hvm_ioreq_server *s;
> unsigned int id;
>
> -if ( has_vpci(d) && vpci_process_pending(v) )
> -return true;
> -
> FOR_EACH_IOREQ_SERVER(d, id, s)
> {
> struct hvm_ioreq_vcpu *sv;
> @@ -186,6 +183,12 @@ bool handle_hvm_io_completion(struct vcpu *v)
> enum hvm_io_completion io_completion;
> unsigned int id;
>
> +if ( has_vpci(d) && vpci_process_pending(v) )
> +{
> +raise_softirq(SCHEDULE_SOFTIRQ);
> +return false;
> +}
> +
> FOR_EACH_IOREQ_SERVER(d, id, s)
> {
> struct hvm_ioreq_vcpu *sv;
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index 39dffb21fb..c9bdc2ced3 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -184,6 +184,11 @@ static void defer_map(struct domain *d, struct pci_dev
> *pdev,
> curr->vpci.mem = mem;
> curr->vpci.cmd = cmd;
> curr->vpci.rom_only = rom_only;
> +/*
> + * Raise a scheduler softirq in order to prevent the guest from
> resuming
> + * execution with pending mapping operations.
> + */
> +raise_softirq(SCHEDULE_SOFTIRQ);
> }
>
> static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool
> rom_only)
> --
> 2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 1/2] amd-iommu: replace occurrences of bool_t with bool
Bring the coding style up to date. No functional change (except for
removal of some pointless initializers).
Signed-off-by: Paul Durrant
Reviewed-by: Jan Beulich
---
Cc: Suravee Suthikulpanit
Cc: Brian Woods
---
xen/drivers/passthrough/amd/iommu_map.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_map.c
b/xen/drivers/passthrough/amd/iommu_map.c
index c1daba8422..fde4686ee9 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -45,9 +45,9 @@ static void clear_iommu_pte_present(unsigned long l1_mfn,
unsigned long dfn)
unmap_domain_page(table);
}
-static bool_t set_iommu_pde_present(u32 *pde, unsigned long next_mfn,
-unsigned int next_level,
-bool_t iw, bool_t ir)
+static bool set_iommu_pde_present(u32 *pde, unsigned long next_mfn,
+ unsigned int next_level,
+ bool iw, bool ir)
{
uint64_t addr_lo, addr_hi, maddr_next;
u32 entry;
@@ -123,13 +123,13 @@ static bool_t set_iommu_pde_present(u32 *pde, unsigned
long next_mfn,
return need_flush;
}
-static bool_t set_iommu_pte_present(unsigned long pt_mfn, unsigned long dfn,
-unsigned long next_mfn, int pde_level,
-bool_t iw, bool_t ir)
+static bool set_iommu_pte_present(unsigned long pt_mfn, unsigned long dfn,
+ unsigned long next_mfn, int pde_level,
+ bool iw, bool ir)
{
u64 *table;
u32 *pde;
-bool_t need_flush = 0;
+bool need_flush;
table = map_domain_page(_mfn(pt_mfn));
@@ -347,16 +347,16 @@ static void set_pde_count(u64 *pde, unsigned int count)
/* Return 1, if pages are suitable for merging at merge_level.
* otherwise increase pde count if mfn is contigous with mfn - 1
*/
-static int iommu_update_pde_count(struct domain *d, unsigned long pt_mfn,
- unsigned long dfn, unsigned long mfn,
- unsigned int merge_level)
+static bool iommu_update_pde_count(struct domain *d, unsigned long pt_mfn,
+ unsigned long dfn, unsigned long mfn,
+ unsigned int merge_level)
{
unsigned int pde_count, next_level;
unsigned long first_mfn;
u64 *table, *pde, *ntable;
u64 ntable_maddr, mask;
struct domain_iommu *hd = dom_iommu(d);
-bool_t ok = 0;
+bool ok = false;
ASSERT( spin_is_locked(&hd->arch.mapping_lock) && pt_mfn );
@@ -384,7 +384,7 @@ static int iommu_update_pde_count(struct domain *d,
unsigned long pt_mfn,
pde_count = get_pde_count(*pde);
if ( pde_count == (PTE_PER_TABLE_SIZE - 1) )
-ok = 1;
+ok = true;
else if ( pde_count < (PTE_PER_TABLE_SIZE - 1))
{
pde_count++;
@@ -648,7 +648,7 @@ static int update_paging_mode(struct domain *d, unsigned
long dfn)
int amd_iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn,
unsigned int flags)
{
-bool_t need_flush = 0;
+bool need_flush;
struct domain_iommu *hd = dom_iommu(d);
int rc;
unsigned long pt_mfn[7];
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 2/2] amd-iommu: replace occurrences of u with uint_t...
...for N in {8, 16, 32, 64}.
Bring the coding style up to date.
Also, while in the neighbourhood, fix some tabs and remove use of uint64_t
values where it leads to the need for explicit casting.
Signed-off-by: Paul Durrant
---
Cc: Suravee Suthikulpanit
Cc: Brian Woods
v2:
- Remove some uses of uint64_t variables
- Add missing blanks in pointer casts
- Use paddr_t for address argument in amd_iommu_reserve_domain_unity_map()
---
xen/drivers/passthrough/amd/iommu_map.c | 124 +-
xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 2 +-
2 files changed, 65 insertions(+), 61 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_map.c
b/xen/drivers/passthrough/amd/iommu_map.c
index fde4686ee9..0ac3f473b3 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -37,7 +37,7 @@ static unsigned int pfn_to_pde_idx(unsigned long pfn,
unsigned int level)
static void clear_iommu_pte_present(unsigned long l1_mfn, unsigned long dfn)
{
-u64 *table, *pte;
+uint64_t *table, *pte;
table = map_domain_page(_mfn(l1_mfn));
pte = table + pfn_to_pde_idx(dfn, 1);
@@ -45,15 +45,15 @@ static void clear_iommu_pte_present(unsigned long l1_mfn,
unsigned long dfn)
unmap_domain_page(table);
}
-static bool set_iommu_pde_present(u32 *pde, unsigned long next_mfn,
+static bool set_iommu_pde_present(uint32_t *pde, unsigned long next_mfn,
unsigned int next_level,
bool iw, bool ir)
{
-uint64_t addr_lo, addr_hi, maddr_next;
-u32 entry;
+uint64_t maddr_next;
+uint32_t addr_lo, addr_hi, entry;
bool need_flush = false, old_present;
-maddr_next = (u64)next_mfn << PAGE_SHIFT;
+maddr_next = __pfn_to_paddr(next_mfn);
old_present = get_field_from_reg_u32(pde[0], IOMMU_PTE_PRESENT_MASK,
IOMMU_PTE_PRESENT_SHIFT);
@@ -79,7 +79,8 @@ static bool set_iommu_pde_present(u32 *pde, unsigned long
next_mfn,
IOMMU_PTE_IO_READ_PERMISSION_MASK,
IOMMU_PTE_IO_READ_PERMISSION_SHIFT);
-maddr_old = (addr_hi << 32) | (addr_lo << PAGE_SHIFT);
+maddr_old = ((uint64_t)addr_hi << 32) |
+((uint64_t)addr_lo << PAGE_SHIFT);
if ( maddr_old != maddr_next || iw != old_w || ir != old_r ||
old_level != next_level )
@@ -90,7 +91,7 @@ static bool set_iommu_pde_present(u32 *pde, unsigned long
next_mfn,
addr_hi = maddr_next >> 32;
/* enable read/write permissions,which will be enforced at the PTE */
-set_field_in_reg_u32((u32)addr_hi, 0,
+set_field_in_reg_u32(addr_hi, 0,
IOMMU_PDE_ADDR_HIGH_MASK,
IOMMU_PDE_ADDR_HIGH_SHIFT, &entry);
set_field_in_reg_u32(iw, entry,
@@ -109,7 +110,7 @@ static bool set_iommu_pde_present(u32 *pde, unsigned long
next_mfn,
pde[1] = entry;
/* mark next level as 'present' */
-set_field_in_reg_u32((u32)addr_lo >> PAGE_SHIFT, 0,
+set_field_in_reg_u32(addr_lo >> PAGE_SHIFT, 0,
IOMMU_PDE_ADDR_LOW_MASK,
IOMMU_PDE_ADDR_LOW_SHIFT, &entry);
set_field_in_reg_u32(next_level, entry,
@@ -127,24 +128,24 @@ static bool set_iommu_pte_present(unsigned long pt_mfn,
unsigned long dfn,
unsigned long next_mfn, int pde_level,
bool iw, bool ir)
{
-u64 *table;
-u32 *pde;
+uint64_t *table;
+uint32_t *pde;
bool need_flush;
table = map_domain_page(_mfn(pt_mfn));
-pde = (u32*)(table + pfn_to_pde_idx(dfn, pde_level));
+pde = (uint32_t *)(table + pfn_to_pde_idx(dfn, pde_level));
need_flush = set_iommu_pde_present(pde, next_mfn, 0, iw, ir);
unmap_domain_page(table);
return need_flush;
}
-void amd_iommu_set_root_page_table(
-u32 *dte, u64 root_ptr, u16 domain_id, u8 paging_mode, u8 valid)
+void amd_iommu_set_root_page_table(uint32_t *dte, uint64_t root_ptr,
+ uint16_t domain_id, uint8_t paging_mode,
+ uint8_t valid)
{
-u64 addr_hi, addr_lo;
-u32 entry;
+uint32_t addr_hi, addr_lo, entry;
set_field_in_reg_u32(domain_id, 0,
IOMMU_DEV_TABLE_DOMAIN_ID_MASK,
IOMMU_DEV_TABLE_DOMAIN_ID_SHIFT, &entry);
@@ -153,7 +154,7 @@ void amd_iommu_set_root_page_table(
addr_lo = root_ptr & DMA_32BIT_MASK;
addr_hi = root_ptr >> 32;
-set_field_in_reg_u32((u32)addr_hi, 0,
+set_field_in_reg_u32(addr_hi, 0,
IOMMU_DEV_TABLE_PAGE_TABLE_PTR_HIGH_MASK,
IOMMU_DEV_TABLE_PAGE_TABLE_PTR_HIGH_SHIFT, &entry);
set_field_in_reg_u32(IOMMU_CONTROL_ENABLED, entry,
@@ -164,7 +165,7 @@ void amd_iommu_set_root_page_table(
[Xen-devel] [PATCH v2 0/2] amd-iommu: cosmetic fixes
Paul Durrant (2): amd-iommu: replace occurrences of bool_t with bool amd-iommu: replace occurrences of u with uint_t... xen/drivers/passthrough/amd/iommu_map.c | 148 +- xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 2 +- 2 files changed, 77 insertions(+), 73 deletions(-) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 4/6] pci: add a segment parameter to pci_hide_device
>>> On 20.11.18 at 17:01, wrote: > No functional change expected. > > Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 130808: regressions - trouble: blocked/broken/fail/pass
flight 130808 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/130808/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken build-amd64 6 xen-buildfail REGR. vs. 130289 Regressions which are regarded as allowable (not blocking): build-arm64-xsm 2 hosts-allocate broken REGR. vs. 130289 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386 1 build-check(1) blocked n/a build-arm64-xsm 3 capture-logs broken blocked in 130289 test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen fc3637e9af9a301d92695999299a3e9a8458c3c1 baseline version: xen 901abfef5de149546b16fba6f4d5bd7def08c672 Last test of basis 130289 2018-11-17 11:00:36 Z9 days Failing since130490 2018-11-19 09:00:27 Z7 days 59 attempts Testing same since 130733 2018-11-23 16:00:40 Z2 days 24 attempts People who touched revisions under test: Andrew Cooper Dario Faggioli Doug Goldstein Ian Jackson Jan Beulich Julien Grall Kevin Tian Len Brown Norbert Manthey Olaf Hering Paul Durrant Rafael J. Wysocki Razvan Cojocaru Roger Pau Monné Sergey Dyasli Tamas K Lengyel Tim Deegan Wei Liu jobs: build-arm64-xsm broken build-amd64 fail build-armhf pass build-amd64-libvirt blocked test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-i386 blocked test-amd64-amd64-libvirt blocked sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary broken-job build-arm64-xsm broken broken-step build-arm64-xsm hosts-allocate broken-step build-arm64-xsm capture-logs Not pushing. (No revision log; it would be 1190 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
