Re: [Xen-devel] [PATCH 02/11] xen/arm: vpl011: Add new hvm params in Xen for ring buffer/event setup
>>> On 05.03.17 at 13:35, wrote: > On 02/21/2017 11:25 AM, Bhupinder Thakur wrote: >> --- a/xen/include/public/hvm/params.h >> +++ b/xen/include/public/hvm/params.h >> @@ -203,10 +203,17 @@ >> */ >> #define HVM_PARAM_ACPI_IOPORTS_LOCATION 19 >> >> -/* Deprecated */ >> +#if defined(__arm__) || defined(__aarch64__) >> +#define HVM_PARAM_VPL011_CONSOLE_PFN20 >> +#define HVM_PARAM_VPL011_CONSOLE_EVTCHN 21 >> +#define HVM_PARAM_VPL011_VIRQ 22 >> +#else >> #define HVM_PARAM_MEMORY_EVENT_CR0 20 >> #define HVM_PARAM_MEMORY_EVENT_CR3 21 >> #define HVM_PARAM_MEMORY_EVENT_CR4 22 > > Those parameters are still deprecated but you drop the comment stating that. > >> +#endif >> + > > Those params are x86 specific so should have never been set on ARM. But > I am not sure if it is fine to re-purpose deprecated number. > > I have CCed "The REST" maintainers to have their input here. I think re-purposing something that was never (meant to be) used is fine in a case like this. However, the question is moot with your suggestion to not use params here in the first place. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/11] xen/arm: vpl011: Refactor evtchn_send in Xen to allow sending events from a xen bound channel
>>> On 05.03.17 at 13:39, wrote:
> My knowledge is limited for this code. So I've just CCed "The REST"
> maintainers. Please do CC them in the future.
Indeed.
> On 02/21/2017 11:26 AM, Bhupinder Thakur wrote:
>> Breakup evtchn_send() to allow sending events for a Xen bound channel.
>> Currently,
>> there is a check in evtchn_send() i.e. is_consumer_xen() that if the event
>> channel
>> is bound to a xen consumer then event generation is not allowed for that
>> channel.
>> This check is to disallow a guest from raising an event via this channel.
>> However,
>> it should allow Xen to send a event via this channel as it is required for
>> sending
>> vpl011 event to the dom0.
>>
>> This change introduces a new function raw_evtchn_send() which sends the event
>> without this check. The current evtchn_send() calls this function after
>> doing the
>> xen consumer check. Xen uses the raw_evtchm_send() version to send the event
>> thus
>> bypassing the check.
Why would Xen want to send an event it is itself the consumer of?
Surely there are better ways to communicate state internally? The
more that you say you want the event sent to Dom0...
>> @@ -650,25 +651,21 @@ static long evtchn_close(struct domain *d1, int port1,
>> bool_t guest)
>> return rc;
>> }
>>
>> -int evtchn_send(struct domain *ld, unsigned int lport)
>> +int raw_evtchn_send(struct domain *ld, unsigned int lport, void *data)
>> {
>> struct evtchn *lchn, *rchn;
>> struct domain *rd;
>> -intrport, ret = 0;
>> +int rport, ret=0;
There's only whitespace change here, and just the break existing
formatting. Please avoid stray changes like this.
>> @@ -696,6 +693,32 @@ int evtchn_send(struct domain *ld, unsigned int lport)
>> }
>>
>> out:
>> +if ( !data )
>> +spin_unlock(&lchn->lock);
>> +
>> +return ret;
>> +}
>> +
>> +int evtchn_send(struct domain *ld, unsigned int lport)
>> +{
>> +struct evtchn *lchn;
>> +int ret;
>> +
>> +if ( !port_is_valid(ld, lport) )
>> +return -EINVAL;
>> +
>> +lchn = evtchn_from_port(ld, lport);
>> +
>> +spin_lock(&lchn->lock);
>> +
>> +if ( unlikely(consumer_is_xen(lchn)) )
>> +{
>> +printk("evtchn_send failed to send via xen event channel\n");
>> +return -EINVAL;
As a general remark: Splitting out functionality into a new function
should _never_ be accompanied by silent behavioral changes for
pre-existing code paths. In the case here there was no printk()
before, and I see no justification for one to be added.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 04/11] xen/arm: vpl011: Enable vpl011 emulation for a domain in Xen
>>> On 05.03.17 at 13:46, wrote: > On 02/21/2017 11:26 AM, Bhupinder Thakur wrote: >> Based on one of the domain creation flags, the vpl011 emulation will be >> enabled for >> the domain. >> >> This flag is currently set always but finally it needs to be controlled by >> the user >> through some configuration option. > > We really don't want the pl011 emulation by default. You should provide > a libxl option to enable pl011 emulation. Furthermore I don't think emulation of individual devices should be set up via DOMCRF_*, these should be used only for more general aspects of domain creation. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [xen-unstable test] 106435: regressions - FAIL
> -Original Message- > From: Andrew Cooper [mailto:am...@hermes.cam.ac.uk] On Behalf Of > Andrew Cooper > Sent: 04 March 2017 15:45 > To: Paul Durrant > Cc: osstest service owner ; xen- > de...@lists.xensource.com; Jan Beulich > Subject: Re: [Xen-devel] [xen-unstable test] 106435: regressions - FAIL > > On 04/03/2017 15:19, osstest service owner wrote: > > flight 106435 xen-unstable real [real] > > http://logs.test-lab.xenproject.org/osstest/logs/106435/ > > > > Regressions :-( > > > > Tests which did not succeed and are blocking, > > including tests which could not be run: > > test-amd64-amd64-xl-qemuu-win7-amd64 9 windows-install fail REGR. vs. > 106412 > > test-armhf-armhf-libvirt-raw 9 debian-di-installfail REGR. vs. > > 106412 > > From > http://logs.test-lab.xenproject.org/osstest/logs/106435/test-amd64-amd64- > xl-qemuu-win7-amd64/9.ts-windows-install.log > > Mar 4 12:49:08.069641 (d3) Booting from DVD/CD... > Mar 4 12:49:08.069670 (d3) Booting from :7c00 > Mar 4 12:49:08.069697 (XEN) stdvga.c:178:d3v0 leaving stdvga mode > Mar 4 12:49:13.045676 (XEN) d3: VIRIDIAN GUEST_OS_ID: vendor: 1 os: 4 > major: 6 minor: 1 sp: 0 build: 1db0 > Mar 4 12:49:41.461683 (XEN) d3: VIRIDIAN HYPERCALL: enabled: 1 pfn: 3 > Mar 4 12:49:41.461731 (XEN) d3v0: VIRIDIAN APIC_ASSIST: enabled: 1 pfn: > 3fffe > Mar 4 12:49:41.469596 (XEN) domain_crash called from viridian.c:306 > Mar 4 12:49:41.477595 (XEN) Domain 3 (vcpu#0) crashed on cpu#1: > Mar 4 12:49:41.477633 (XEN) [ Xen-4.9-unstable x86_64 debug=y Not > tainted ] > Mar 4 12:49:41.485589 (XEN) CPU:1 > Mar 4 12:49:41.485620 (XEN) RIP:0010:[] > Mar 4 12:49:41.485650 (XEN) RFLAGS: 0286 CONTEXT: hvm > guest (d3v0) > Mar 4 12:49:41.493591 (XEN) rax: rbx: f800027ede80 > rcx: 0001 > Mar 4 12:49:41.501560 (XEN) rdx: rsi: fa8001291040 > rdi: f800027fbc40 > Mar 4 12:49:41.509588 (XEN) rbp: 0080 rsp: f880009b0d80 > r8: > Mar 4 12:49:41.517584 (XEN) r9: f800027ede80 r10: fa8001291040 > r11: f800027ede90 > Mar 4 12:49:41.517624 (XEN) r12: f88129c0 r13: f800028afbe0 > r14: fa800122db30 > Mar 4 12:49:41.525586 (XEN) r15: f8b96080 cr0: 80050031 > cr4: 06b8 > Mar 4 12:49:41.533582 (XEN) cr3: 00187000 cr2: > Mar 4 12:49:41.541578 (XEN) ds: 002b es: 002b fs: 0053 gs: 002b ss: > 0018 > cs: 0010 > > > Looks like this intermittent bug is still biting. Should we kill > VIRIDIAN APIC_ASSIST until we have got to the bottom of it? > Alternatively, add some printk() to actually give us useful information > when it does go wrong. I actually think this may be a bug in Windows 7. I normally run with this enlightenment on and have seen this very occasionally and only with Windows 7. I think it would probably be useful to stash info about the last interrupt injected into the guest and then reduce the domain_crash() to a gprintk() dumping that info plus info about the interrupt that's about to be injected. I'll post a patch. Paul > > ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [linux-linus test] 106480: regressions - FAIL
flight 106480 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/106480/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-winxpsp3 6 xen-boot fail REGR. vs. 59254 test-amd64-amd64-xl-pvh-intel 11 guest-start fail REGR. vs. 59254 test-armhf-armhf-xl-credit2 11 guest-start fail REGR. vs. 59254 test-armhf-armhf-xl 11 guest-start fail REGR. vs. 59254 test-armhf-armhf-xl-arndale 11 guest-start fail REGR. vs. 59254 test-armhf-armhf-xl-xsm 11 guest-start fail REGR. vs. 59254 test-armhf-armhf-libvirt-xsm 11 guest-start fail REGR. vs. 59254 test-armhf-armhf-xl-cubietruck 11 guest-start fail REGR. vs. 59254 test-armhf-armhf-libvirt 11 guest-start fail REGR. vs. 59254 test-armhf-armhf-xl-multivcpu 11 guest-start fail REGR. vs. 59254 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 9 windows-install fail REGR. vs. 59254 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 11 guest-start fail REGR. vs. 59254 test-amd64-amd64-xl-rtds 9 debian-installfail REGR. vs. 59254 test-armhf-armhf-xl-vhd 9 debian-di-install fail baseline untested test-armhf-armhf-libvirt-raw 9 debian-di-install fail baseline untested test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 59254 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 59254 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 59254 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 59254 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a build-arm64-xsm 5 xen-buildfail never pass test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass build-arm64 5 xen-buildfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass version targeted for testing: linuxc1ae3cfa0e89fa1a7ecc4c99031f5e9ae99d9201 baseline version: linux45820c294fe1b1a9df495d57f40585ef2d069a39 Last test of basis59254 2015-07-09 04:20:48 Z 606 days Failing since 59348 2015-07-10 04:24:05 Z 605 days 315 attempts Testing same since 106480 2017-03-05 23:48:45 Z0 days1 attempts 8045 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm fail build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 fail 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-pvopspass build-armhf-pvopspass
Re: [Xen-devel] [PATCH] xen: Allow a default compiled-in command line using Kconfig
>>> On 06.03.17 at 07:09, wrote: > It seems that I forgot to cc the corresponding Kconfig maintainers. And not just them. > What's worse, I missed some important details in the patch: > 1. the CMDLINE_OVERRRIDE config entry > 2. handle dom0 options(the part after " -- ") in CONFIG_CMDLINE. > > sorry for that. I'll resend v2 of the patch later. You also want to fix indentation in your Kconfig additions. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [libvirt test] 106483: regressions - FAIL
flight 106483 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/106483/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-libvirt 16 guest-start.2 fail in 106473 REGR. vs. 106434 Tests which are failing intermittently (not blocking): test-armhf-armhf-libvirt 15 guest-start/debian.repeat fail pass in 106473 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-libvirt 13 saverestore-support-checkfail like 106434 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail like 106434 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail like 106434 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a build-arm64 5 xen-buildfail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass build-arm64-xsm 5 xen-buildfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass build-arm64-pvops 5 kernel-build fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail never pass version targeted for testing: libvirt cbcfd097692a77fa5ee2da8eed8c6a3172a4ac37 baseline version: libvirt 38063555c8e7b73ed1ab99b36d5084c0093ae8bd Last test of basis 106434 2017-03-04 04:20:26 Z2 days Testing same since 106473 2017-03-05 04:51:19 Z1 days2 attempts People who touched revisions under test: John Ferlan jobs: build-amd64-xsm pass build-arm64-xsm fail build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 fail 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-pvopsfail 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-armhf-armhf-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt blocked test-armhf-armhf-libvirt fail test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-arm64-arm64-libvirt-qcow2 blocked test-armhf-armhf-libvirt-raw pass test-amd64-amd64-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-l
Re: [Xen-devel] [PATCH net v2] xen-netback: fix race condition on XenBus disconnect
> -Original Message-
> From: Igor Druzhinin [mailto:igor.druzhi...@citrix.com]
> Sent: 03 March 2017 20:23
> To: net...@vger.kernel.org; xen-de...@lists.xenproject.org
> Cc: Paul Durrant ; jgr...@suse.com; Wei Liu
> ; Igor Druzhinin
> Subject: [PATCH net v2] xen-netback: fix race condition on XenBus
> disconnect
>
> In some cases during XenBus disconnect event handling and subsequent
> queue resource release there may be some TX handlers active on
> other processors. Use RCU in order to synchronize with them.
>
> Signed-off-by: Igor Druzhinin
> ---
> v2:
> * Add protection for xenvif_get_ethtool_stats
> * Additional comments and fixes
> ---
> drivers/net/xen-netback/interface.c | 29 ++---
> drivers/net/xen-netback/netback.c | 2 +-
> drivers/net/xen-netback/xenbus.c| 20 ++--
> 3 files changed, 33 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-
> netback/interface.c
> index a2d32676..266b7cd 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -164,13 +164,17 @@ static int xenvif_start_xmit(struct sk_buff *skb,
> struct net_device *dev)
> {
> struct xenvif *vif = netdev_priv(dev);
> struct xenvif_queue *queue = NULL;
> - unsigned int num_queues = vif->num_queues;
> + unsigned int num_queues;
> u16 index;
> struct xenvif_rx_cb *cb;
>
> BUG_ON(skb->dev != dev);
>
> - /* Drop the packet if queues are not set up */
> + /* Drop the packet if queues are not set up.
> + * This handler should be called inside an RCU read section
> + * so we don't need to enter it here explicitly.
> + */
> + num_queues = rcu_dereference(vif)->num_queues;
> if (num_queues < 1)
> goto drop;
>
> @@ -221,18 +225,21 @@ static struct net_device_stats
> *xenvif_get_stats(struct net_device *dev)
> {
> struct xenvif *vif = netdev_priv(dev);
> struct xenvif_queue *queue = NULL;
> + unsigned int num_queues;
> u64 rx_bytes = 0;
> u64 rx_packets = 0;
> u64 tx_bytes = 0;
> u64 tx_packets = 0;
> unsigned int index;
>
> - spin_lock(&vif->lock);
> - if (vif->queues == NULL)
> + rcu_read_lock();
> +
> + num_queues = rcu_dereference(vif)->num_queues;
> + if (num_queues < 1)
> goto out;
Is this if clause worth it? All it does is jump over the for loop, which would
not be executed anyway, since the initial test (0 < 0) would fail.
>
> /* Aggregate tx and rx stats from each queue */
> - for (index = 0; index < vif->num_queues; ++index) {
> + for (index = 0; index < num_queues; ++index) {
> queue = &vif->queues[index];
> rx_bytes += queue->stats.rx_bytes;
> rx_packets += queue->stats.rx_packets;
> @@ -241,7 +248,7 @@ static struct net_device_stats
> *xenvif_get_stats(struct net_device *dev)
> }
>
> out:
> - spin_unlock(&vif->lock);
> + rcu_read_unlock();
>
> vif->dev->stats.rx_bytes = rx_bytes;
> vif->dev->stats.rx_packets = rx_packets;
> @@ -377,10 +384,16 @@ static void xenvif_get_ethtool_stats(struct
> net_device *dev,
>struct ethtool_stats *stats, u64 * data)
> {
> struct xenvif *vif = netdev_priv(dev);
> - unsigned int num_queues = vif->num_queues;
> + unsigned int num_queues;
> int i;
> unsigned int queue_index;
>
> + rcu_read_lock();
> +
> + num_queues = rcu_dereference(vif)->num_queues;
> + if (num_queues < 1)
> + goto out;
> +
You have introduced a semantic change with the above if clause. The
xenvif_stats array was previously zeroed if num_queues < 1. It appears that
ethtool does actually allocate a zeroed array to pass in here, but I wonder
whether it is still safer to have this function zero it anyway.
> for (i = 0; i < ARRAY_SIZE(xenvif_stats); i++) {
> unsigned long accum = 0;
> for (queue_index = 0; queue_index < num_queues;
> ++queue_index) {
> @@ -389,6 +402,8 @@ static void xenvif_get_ethtool_stats(struct
> net_device *dev,
> }
> data[i] = accum;
> }
> +out:
> + rcu_read_unlock();
> }
>
> static void xenvif_get_strings(struct net_device *dev, u32 stringset, u8 *
> data)
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-
> netback/netback.c
> index f9bcf4a..62fa74d 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -214,7 +214,7 @@ static void xenvif_fatal_tx_err(struct xenvif *vif)
> netdev_err(vif->dev, "fatal error; disabling device\n");
> vif->disabled = true;
> /* Disable the vif from queue 0's kthread */
> - if (vif->queues)
> + if (vif->num_queues > 0)
num_queues is unsigned so this check should not be > 0. It would be better
simply to do:
if (vif->num_queue
Re: [Xen-devel] [PATCH v2 5/5] xen: use libxendevicemodel when available
> -Original Message- > From: Stefano Stabellini [mailto:sstabell...@kernel.org] > Sent: 03 March 2017 20:43 > To: Stefano Stabellini > Cc: Paul Durrant ; xen-de...@lists.xenproject.org; > qemu-de...@nongnu.org; Anthony Perard > Subject: RE: [PATCH v2 5/5] xen: use libxendevicemodel when available > > On Fri, 3 Mar 2017, Stefano Stabellini wrote: > > On Fri, 3 Mar 2017, Paul Durrant wrote: > > > > -Original Message- > > > > From: Stefano Stabellini [mailto:sstabell...@kernel.org] > > > > Sent: 02 March 2017 22:50 > > > > To: Paul Durrant > > > > Cc: xen-de...@lists.xenproject.org; qemu-de...@nongnu.org; Stefano > > > > Stabellini ; Anthony Perard > > > > > > > > Subject: Re: [PATCH v2 5/5] xen: use libxendevicemodel when available > > > > > > > > On Thu, 2 Mar 2017, Paul Durrant wrote: > > > > > This patch modifies the wrapper functions in xen_common.h to use > the > > > > > new xendevicemodel interface if it is available along with > compatibility > > > > > code to use the old libxenctrl interface if it is not. > > > > > > > > > > Signed-off-by: Paul Durrant > > > > > --- > > > > > Cc: Stefano Stabellini > > > > > Cc: Anthony Perard > > > > > > > > > > v2: > > > > > - Add a compat define for xenforeignmemory_close() since this is > now > > > > > used. > > > > > --- > > > > > include/hw/xen/xen_common.h | 115 > > > > +++- > > > > > xen-common.c| 8 +++ > > > > > 2 files changed, 90 insertions(+), 33 deletions(-) > > > > > > > > > > diff --git a/include/hw/xen/xen_common.h > > > > b/include/hw/xen/xen_common.h > > > > > index 31cf25f..48444e5 100644 > > > > > --- a/include/hw/xen/xen_common.h > > > > > +++ b/include/hw/xen/xen_common.h > > > > > @@ -9,6 +9,7 @@ > > > > > #undef XC_WANT_COMPAT_EVTCHN_API > > > > > #undef XC_WANT_COMPAT_GNTTAB_API > > > > > #undef XC_WANT_COMPAT_MAP_FOREIGN_API > > > > > +#undef XC_WANT_COMPAT_DEVICEMODEL_API > > > > > > > > > > #include > > > > > #include > > > > > @@ -26,48 +27,95 @@ extern xc_interface *xen_xc; > > > > > * We don't support Xen prior to 4.2.0. > > > > > */ > > > > > > > > > > +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 490 > > > > > + > > > > > +typedef xc_interface xendevicemodel_handle; > > > > > + > > > > > +#define xendevicemodel_open(l, f) xen_xc > > > > > + > > > > > +#define xendevicemodel_map_io_range_to_ioreq_server \ > > > > > +xc_hvm_map_io_range_to_ioreq_server > > > > > +#define xendevicemodel_unmap_io_range_from_ioreq_server \ > > > > > +xc_hvm_unmap_io_range_from_ioreq_server > > > > > +#define xendevicemodel_map_pcidev_to_ioreq_server \ > > > > > +xc_hvm_map_pcidev_to_ioreq_server > > > > > +#define xendevicemodel_unmap_pcidev_from_ioreq_server \ > > > > > +xc_hvm_unmap_pcidev_from_ioreq_server > > > > > +#define xendevicemodel_create_ioreq_server \ > > > > > +xc_hvm_create_ioreq_server > > > > > +#define xendevicemodel_destroy_ioreq_server \ > > > > > +xc_hvm_destroy_ioreq_server > > > > > +#define xendevicemodel_get_ioreq_server_info \ > > > > > +xc_hvm_get_ioreq_server_info > > > > > +#define xendevicemodel_set_ioreq_server_state \ > > > > > +xc_hvm_set_ioreq_server_state > > > > > +#define xendevicemodel_set_pci_intx_level \ > > > > > +xc_hvm_set_pci_intx_level > > > > > +#define xendevicemodel_set_pci_link_route \ > > > > > +xc_hvm_set_pci_link_route > > > > > +#define xendevicemodel_set_isa_irq_level \ > > > > > +xc_hvm_set_isa_irq_level > > > > > +#define xendevicemodel_inject_msi \ > > > > > +xc_hvm_inject_msi > > > > > +#define xendevicemodel_set_mem_type \ > > > > > +xc_hvm_set_mem_type > > > > > +#define xendevicemodel_track_dirty_vram \ > > > > > +xc_hvm_track_dirty_vram > > > > > +#define xendevicemodel_modified_memory \ > > > > > +xc_hvm_modified_memory > > > > > > > > It does build correctly now for Xen < 4.9, however it breaks against > > > > xen-unstable: > > > > > > > > ERROR: configure test passed without -Werror but failed with -Werror. > > > > This is probably a bug in the configure script. The failing > > > > command > > > > will be at the bottom of config.log. > > > > You can run configure with --disable-werror to bypass this > > > > check. > > > > > > > > and config.log says: > > > > > > > > config-temp/qemu-conf.c: In function 'main': > > > > config-temp/qemu-conf.c:32:3: error: implicit declaration of function > > > > 'xc_hvm_set_mem_type' [-Werror=implicit-function-declaration] > > > > config-temp/qemu-conf.c:32:3: error: nested extern declaration of > > > > 'xc_hvm_set_mem_type' [-Werror=nested-externs] > > > > config-temp/qemu-conf.c:34:3: error: implicit declaration of function > > > > 'xc_hvm_inject_msi' [-Werror=implicit-function-declaration] > > > > config-temp/qemu-conf.c:34:3: error: nested extern declaration of > > > > 'xc_hvm_inject_msi' [-Werror=nested-externs] > > > > config-temp/qemu-conf.c:35:3: error: implic
Re: [Xen-devel] [PATCH 5/7] x86/shadow: Use the pagewalk reserved bits helpers
At 12:26 + on 02 Mar (1488457613), Andrew Cooper wrote: > On 01/03/17 16:03, Jan Beulich wrote: > On 27.02.17 at 15:03, wrote: > >> The shadow logic should never create a shadow of a guest PTE which contains > >> reserved bits from the guests point of view. Such a shadowed entry might > >> not > >> cause #PF[RSVD] when walked by hardware, thus won't behave architecturally > >> from the guests point of view. > > But are we already or-ing in the RSVD bit accordingly in such cases, > > before handing the #PF to the guest? The patch here certainly > > doesn't make any change towards that, afaics. > > The purpose of this patch is to ensure we never create a shadow which > risks causing hardware to generate #PF[RSVD] when running on the > shadows, other than the one deliberate case (MMIO fastpath). Confusion! AIUI: - Shadows installed on demand in the pagefault handler are already correct. If the guest PTE contained invalid bits we'd have injected a fault instead of shadowing it. - There is no risk of accidentally installing a shadow with reserved bits in it even if the guest pte has reserved bits in it. _sh_propagate() sanity-checks the flags, and the address bits come from the MFN (IOW we'd need a buggy p2m entry). If that were a risk, I don't think this patch would solve it. - The potential bug that this patch tries to fix is: 1. Guest writes a PTE with reserved bits in it. 2. That gets shadowed by a write-to-pagetable path or a prefetch. 3. The shadow is a valid PTE, so the guest gets no #PF, instead of #PF(rsvd). Now by the same logic I used above there's probably no path where a reserved _address_ bit causes a problem, but I see no harm in centralising the logic and using the same code for these paths as for the pt walker. In answering this, I've spotted that the calls to l1e_propagate_from_guest() in sh_resync_l1() and sh_prefetch() aren't updated in this patch and should be. Cheers, Tim. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] x86/mem_access: fixed vm_event emulation check with altp2m enabled
Currently, p2m_mem_access_emulate_check() uses p2m_get_mem_access()
to check if the page restrictions have been lifted between the time
of sending the vm_event out and the reception of the reply - in
which case emulation is no longer required. Unfortunately,
p2m_get_mem_access() uses p2m_get_hostp2m(d) which only checks the
default EPT (view 0 in altp2m parlance). This patch fixes this by
checking the active altp2m view instead, whenever applicable.
Signed-off-by: Razvan Cojocaru
---
xen/arch/x86/mm/mem_access.c | 98 +---
1 file changed, 56 insertions(+), 42 deletions(-)
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 3ebeb4f..29a0c43 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -32,14 +32,68 @@
#include "mm-locks.h"
+/*
+ * Get access type for a gfn.
+ * If gfn == INVALID_GFN, gets the default access type.
+ */
+static int _p2m_get_mem_access(struct p2m_domain *p2m, gfn_t gfn,
+ xenmem_access_t *access)
+{
+p2m_type_t t;
+p2m_access_t a;
+mfn_t mfn;
+
+static const xenmem_access_t memaccess[] = {
+#define ACCESS(ac) [p2m_access_##ac] = XENMEM_access_##ac
+ACCESS(n),
+ACCESS(r),
+ACCESS(w),
+ACCESS(rw),
+ACCESS(x),
+ACCESS(rx),
+ACCESS(wx),
+ACCESS(rwx),
+ACCESS(rx2rw),
+ACCESS(n2rwx),
+#undef ACCESS
+};
+
+/* If request to get default access. */
+if ( gfn_eq(gfn, INVALID_GFN) )
+{
+*access = memaccess[p2m->default_access];
+return 0;
+}
+
+gfn_lock(p2m, gfn, 0);
+mfn = p2m->get_entry(p2m, gfn_x(gfn), &t, &a, 0, NULL, NULL);
+gfn_unlock(p2m, gfn, 0);
+
+if ( mfn_eq(mfn, INVALID_MFN) )
+return -ESRCH;
+
+if ( (unsigned) a >= ARRAY_SIZE(memaccess) )
+return -ERANGE;
+
+*access = memaccess[a];
+return 0;
+}
+
bool p2m_mem_access_emulate_check(struct vcpu *v,
const vm_event_response_t *rsp)
{
xenmem_access_t access;
bool violation = 1;
const struct vm_event_mem_access *data = &rsp->u.mem_access;
+struct domain *d = v->domain;
+struct p2m_domain *p2m = NULL;
-if ( p2m_get_mem_access(v->domain, _gfn(data->gfn), &access) == 0 )
+if ( altp2m_active(d) )
+p2m = p2m_get_altp2m(v);
+if ( !p2m )
+p2m = p2m_get_hostp2m(d);
+
+if ( _p2m_get_mem_access(p2m, _gfn(data->gfn), &access) == 0 )
{
switch ( access )
{
@@ -405,51 +459,11 @@ long p2m_set_mem_access_multi(struct domain *d,
return rc;
}
-/*
- * Get access type for a gfn.
- * If gfn == INVALID_GFN, gets the default access type.
- */
int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t *access)
{
struct p2m_domain *p2m = p2m_get_hostp2m(d);
-p2m_type_t t;
-p2m_access_t a;
-mfn_t mfn;
-
-static const xenmem_access_t memaccess[] = {
-#define ACCESS(ac) [p2m_access_##ac] = XENMEM_access_##ac
-ACCESS(n),
-ACCESS(r),
-ACCESS(w),
-ACCESS(rw),
-ACCESS(x),
-ACCESS(rx),
-ACCESS(wx),
-ACCESS(rwx),
-ACCESS(rx2rw),
-ACCESS(n2rwx),
-#undef ACCESS
-};
-
-/* If request to get default access. */
-if ( gfn_eq(gfn, INVALID_GFN) )
-{
-*access = memaccess[p2m->default_access];
-return 0;
-}
-gfn_lock(p2m, gfn, 0);
-mfn = p2m->get_entry(p2m, gfn_x(gfn), &t, &a, 0, NULL, NULL);
-gfn_unlock(p2m, gfn, 0);
-
-if ( mfn_eq(mfn, INVALID_MFN) )
-return -ESRCH;
-
-if ( (unsigned) a >= ARRAY_SIZE(memaccess) )
-return -ERANGE;
-
-*access = memaccess[a];
-return 0;
+return _p2m_get_mem_access(p2m, gfn, access);
}
/*
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 5/5] xen: use libxendevicemodel when available
> -Original Message- > From: Qemu-devel [mailto:qemu-devel- > bounces+paul.durrant=citrix@nongnu.org] On Behalf Of Paul Durrant > Sent: 06 March 2017 09:15 > To: 'Stefano Stabellini' > Cc: Anthony Perard ; xen- > de...@lists.xenproject.org; qemu-de...@nongnu.org > Subject: Re: [Qemu-devel] [PATCH v2 5/5] xen: use libxendevicemodel when > available > > > -Original Message- > > From: Stefano Stabellini [mailto:sstabell...@kernel.org] > > Sent: 03 March 2017 20:43 > > To: Stefano Stabellini > > Cc: Paul Durrant ; xen- > de...@lists.xenproject.org; > > qemu-de...@nongnu.org; Anthony Perard > > Subject: RE: [PATCH v2 5/5] xen: use libxendevicemodel when available > > > > On Fri, 3 Mar 2017, Stefano Stabellini wrote: > > > On Fri, 3 Mar 2017, Paul Durrant wrote: > > > > > -Original Message- > > > > > From: Stefano Stabellini [mailto:sstabell...@kernel.org] > > > > > Sent: 02 March 2017 22:50 > > > > > To: Paul Durrant > > > > > Cc: xen-de...@lists.xenproject.org; qemu-de...@nongnu.org; > Stefano > > > > > Stabellini ; Anthony Perard > > > > > > > > > > Subject: Re: [PATCH v2 5/5] xen: use libxendevicemodel when > available > > > > > > > > > > On Thu, 2 Mar 2017, Paul Durrant wrote: > > > > > > This patch modifies the wrapper functions in xen_common.h to use > > the > > > > > > new xendevicemodel interface if it is available along with > > compatibility > > > > > > code to use the old libxenctrl interface if it is not. > > > > > > > > > > > > Signed-off-by: Paul Durrant > > > > > > --- > > > > > > Cc: Stefano Stabellini > > > > > > Cc: Anthony Perard > > > > > > > > > > > > v2: > > > > > > - Add a compat define for xenforeignmemory_close() since this is > > now > > > > > > used. > > > > > > --- > > > > > > include/hw/xen/xen_common.h | 115 > > > > > +++- > > > > > > xen-common.c| 8 +++ > > > > > > 2 files changed, 90 insertions(+), 33 deletions(-) > > > > > > > > > > > > diff --git a/include/hw/xen/xen_common.h > > > > > b/include/hw/xen/xen_common.h > > > > > > index 31cf25f..48444e5 100644 > > > > > > --- a/include/hw/xen/xen_common.h > > > > > > +++ b/include/hw/xen/xen_common.h > > > > > > @@ -9,6 +9,7 @@ > > > > > > #undef XC_WANT_COMPAT_EVTCHN_API > > > > > > #undef XC_WANT_COMPAT_GNTTAB_API > > > > > > #undef XC_WANT_COMPAT_MAP_FOREIGN_API > > > > > > +#undef XC_WANT_COMPAT_DEVICEMODEL_API > > > > > > > > > > > > #include > > > > > > #include > > > > > > @@ -26,48 +27,95 @@ extern xc_interface *xen_xc; > > > > > > * We don't support Xen prior to 4.2.0. > > > > > > */ > > > > > > > > > > > > +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 490 > > > > > > + > > > > > > +typedef xc_interface xendevicemodel_handle; > > > > > > + > > > > > > +#define xendevicemodel_open(l, f) xen_xc > > > > > > + > > > > > > +#define xendevicemodel_map_io_range_to_ioreq_server \ > > > > > > +xc_hvm_map_io_range_to_ioreq_server > > > > > > +#define xendevicemodel_unmap_io_range_from_ioreq_server \ > > > > > > +xc_hvm_unmap_io_range_from_ioreq_server > > > > > > +#define xendevicemodel_map_pcidev_to_ioreq_server \ > > > > > > +xc_hvm_map_pcidev_to_ioreq_server > > > > > > +#define xendevicemodel_unmap_pcidev_from_ioreq_server \ > > > > > > +xc_hvm_unmap_pcidev_from_ioreq_server > > > > > > +#define xendevicemodel_create_ioreq_server \ > > > > > > +xc_hvm_create_ioreq_server > > > > > > +#define xendevicemodel_destroy_ioreq_server \ > > > > > > +xc_hvm_destroy_ioreq_server > > > > > > +#define xendevicemodel_get_ioreq_server_info \ > > > > > > +xc_hvm_get_ioreq_server_info > > > > > > +#define xendevicemodel_set_ioreq_server_state \ > > > > > > +xc_hvm_set_ioreq_server_state > > > > > > +#define xendevicemodel_set_pci_intx_level \ > > > > > > +xc_hvm_set_pci_intx_level > > > > > > +#define xendevicemodel_set_pci_link_route \ > > > > > > +xc_hvm_set_pci_link_route > > > > > > +#define xendevicemodel_set_isa_irq_level \ > > > > > > +xc_hvm_set_isa_irq_level > > > > > > +#define xendevicemodel_inject_msi \ > > > > > > +xc_hvm_inject_msi > > > > > > +#define xendevicemodel_set_mem_type \ > > > > > > +xc_hvm_set_mem_type > > > > > > +#define xendevicemodel_track_dirty_vram \ > > > > > > +xc_hvm_track_dirty_vram > > > > > > +#define xendevicemodel_modified_memory \ > > > > > > +xc_hvm_modified_memory > > > > > > > > > > It does build correctly now for Xen < 4.9, however it breaks against > > > > > xen-unstable: > > > > > > > > > > ERROR: configure test passed without -Werror but failed with - > Werror. > > > > > This is probably a bug in the configure script. The failing > > > > > command > > > > > will be at the bottom of config.log. > > > > > You can run configure with --disable-werror to bypass this > > > > > check. > > > > > > > > > > and config.log says: > > > > > > > > > > config-temp/qemu-conf.c: In function 'main': > > > > > config-t
[Xen-devel] [distros-debian-sid test] 68639: regressions - trouble: blocked/broken/fail/pass
flight 68639 distros-debian-sid real [real] http://osstest.xs.citrite.net/~osstest/testlogs/logs/68639/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-amd64-sid-netboot-pygrub 10 guest-start fail REGR. vs. 68617 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-i386-sid-netboot-pygrub 10 guest-start fail blocked in 68617 test-amd64-i386-i386-sid-netboot-pvgrub 10 guest-start fail like 68617 test-amd64-amd64-amd64-sid-netboot-pvgrub 10 guest-start fail like 68617 test-armhf-armhf-armhf-sid-netboot-pygrub 9 debian-di-install fail like 68617 Tests which did not succeed, but are not blocking: test-arm64-arm64-armhf-sid-netboot-pygrub 1 build-check(1)blocked n/a build-arm64-pvops 2 hosts-allocate broken never pass build-arm64 2 hosts-allocate broken never pass build-arm64-pvops 3 capture-logs broken never pass build-arm64 3 capture-logs broken never pass baseline version: flight 68617 jobs: build-amd64 pass build-arm64 broken build-armhf pass build-i386 pass build-amd64-pvopspass build-arm64-pvopsbroken build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-amd64-sid-netboot-pvgrubfail test-amd64-i386-i386-sid-netboot-pvgrub fail test-amd64-i386-amd64-sid-netboot-pygrub fail test-arm64-arm64-armhf-sid-netboot-pygrubblocked test-armhf-armhf-armhf-sid-netboot-pygrubfail test-amd64-amd64-i386-sid-netboot-pygrub fail sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xs.citrite.net/~osstest/testlogs/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/mem_access: fixed vm_event emulation check with altp2m enabled
>>> On 06.03.17 at 10:28, wrote:
> --- a/xen/arch/x86/mm/mem_access.c
> +++ b/xen/arch/x86/mm/mem_access.c
> @@ -32,14 +32,68 @@
>
> #include "mm-locks.h"
>
> +/*
> + * Get access type for a gfn.
> + * If gfn == INVALID_GFN, gets the default access type.
> + */
> +static int _p2m_get_mem_access(struct p2m_domain *p2m, gfn_t gfn,
> + xenmem_access_t *access)
> +{
> +p2m_type_t t;
> +p2m_access_t a;
> +mfn_t mfn;
> +
> +static const xenmem_access_t memaccess[] = {
> +#define ACCESS(ac) [p2m_access_##ac] = XENMEM_access_##ac
> +ACCESS(n),
> +ACCESS(r),
> +ACCESS(w),
> +ACCESS(rw),
> +ACCESS(x),
> +ACCESS(rx),
> +ACCESS(wx),
> +ACCESS(rwx),
> +ACCESS(rx2rw),
> +ACCESS(n2rwx),
> +#undef ACCESS
> +};
> +
> +/* If request to get default access. */
> +if ( gfn_eq(gfn, INVALID_GFN) )
> +{
> +*access = memaccess[p2m->default_access];
> +return 0;
> +}
> +
> +gfn_lock(p2m, gfn, 0);
> +mfn = p2m->get_entry(p2m, gfn_x(gfn), &t, &a, 0, NULL, NULL);
> +gfn_unlock(p2m, gfn, 0);
> +
> +if ( mfn_eq(mfn, INVALID_MFN) )
> +return -ESRCH;
> +
> +if ( (unsigned) a >= ARRAY_SIZE(memaccess) )
Granted you're just moving this code here, but while doing so you
could have removed the stray blank and added the missing "int"
from/to the cast expression. Unless there's a need for a v2 this
could of course be adjusted upon commit.
Reviewed-by: Jan Beulich
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/11] xen/arm: vpl011: Refactor evtchn_send in Xen to allow sending events from a xen bound channel
Hi Konrad,
On 4 March 2017 at 02:43, Konrad Rzeszutek Wilk wrote:
> On Tue, Feb 21, 2017 at 04:56:00PM +0530, Bhupinder Thakur wrote:
>> Breakup evtchn_send() to allow sending events for a Xen bound channel.
>> Currently,
>> there is a check in evtchn_send() i.e. is_consumer_xen() that if the event
>> channel
>> is bound to a xen consumer then event generation is not allowed for that
>> channel.
>> This check is to disallow a guest from raising an event via this channel.
>> However,
>
> Did any code archeology help in idenfitying why this was done this way?
>
> You should explain why it was done - what was the use case , and why
> your change will not change this semantic.
I think there was never a use case earlier to generate events from the
Xen itself to the guest. The event generation always happened on
behalf of a guest via evtchn_send(), which was invoked through a
hypercall by the guest. To disallow a guest from sending an event via
an event channel which is bound to Xen, there is a check in
evtchn_send() to check if it is a xen bound channel and if it is then
disallow sending an event via that channel because that channel is
meant only for Xen.
For pl011 emulation, there is a requirement for Xen to generate the
event to dom0 when there is data for dom0 to read in the ring buffer.
Since I am using a Xen bound channel so that Xen can receive events
from dom0 when there is data for Xen to read, the above mentioned
check would not allow the event to be sent to dom0.
Since this event is required to be generated from Xen only, it is safe
to allow to send the event from a xen bound channel. That is why I
introduced a new function raw_evtchn_send() to allow Xen to do that
while still keeping that restriction for the guests who use
evtchn_send() as it is today.
>> it should allow Xen to send a event via this channel as it is required for
>> sending
>> vpl011 event to the dom0.
>>
>> This change introduces a new function raw_evtchn_send() which sends the event
>> without this check. The current evtchn_send() calls this function after
>> doing the
>
> .. and without taking a lock? Why?
>
The third parameter to raw_evtchn_send() is the event channel
structure pointer. When raw_evtchn_send() is called from inside
evtchn_send(), it would be passed a pointer to the local event channel
structure and lock already taken (which would be case most of the
times). But when it is called directly by Xen to send an event to dom0
(as in case of pl011), the third parameter will be passed as NULL. In
this case, the lock and a reference to local channel structure would
be taken inside raw_evtchn_send().
>
>> xen consumer check. Xen uses the raw_evtchm_send() version to send the event
>> thus
>> bypassing the check.
>>
>> Signed-off-by: Bhupinder Thakur
>> ---
>> xen/common/event_channel.c | 49
>> ++
>> xen/include/xen/event.h| 6 ++
>> 2 files changed, 42 insertions(+), 13 deletions(-)
>>
>> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
>> index 638dc5e..4b039f3 100644
>> --- a/xen/common/event_channel.c
>> +++ b/xen/common/event_channel.c
>> @@ -27,6 +27,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -650,25 +651,21 @@ static long evtchn_close(struct domain *d1, int port1,
>> bool_t guest)
>> return rc;
>> }
>>
>> -int evtchn_send(struct domain *ld, unsigned int lport)
>> +int raw_evtchn_send(struct domain *ld, unsigned int lport, void *data)
>> {
>> struct evtchn *lchn, *rchn;
>> struct domain *rd;
>> -intrport, ret = 0;
>> +int rport, ret=0;
>
> Please that code as is.
>
I will remove these cosmetic changes in the next patch.
>>
>> -if ( !port_is_valid(ld, lport) )
>> -return -EINVAL;
>> -
>> -lchn = evtchn_from_port(ld, lport);
>> -
>> -spin_lock(&lchn->lock);
>> -
>> -/* Guest cannot send via a Xen-attached event channel. */
>> -if ( unlikely(consumer_is_xen(lchn)) )
>> +if ( !data )
>> {
>> -ret = -EINVAL;
>> -goto out;
>> +if ( !port_is_valid(ld, lport) )
>> +return -EINVAL;
>> +lchn = evtchn_from_port(ld, lport);
>> +spin_lock(&lchn->lock);
>
> That won't do. Please keep the format of the old code as much
> as possible (hint: Those newlines).
>
I will remove these cosmetic changes in the next patch.
>> }
>> +else
>> +lchn = (struct evtchn *)data;
>>
>> ret = xsm_evtchn_send(XSM_HOOK, ld, lchn);
>> if ( ret )
>> @@ -696,6 +693,32 @@ int evtchn_send(struct domain *ld, unsigned int lport)
>> }
>>
>> out:
>> +if ( !data )
>> +spin_unlock(&lchn->lock);
>> +
>> +return ret;
>> +}
>> +
>> +int evtchn_send(struct domain *ld, unsigned int lport)
>> +{
>> +struct evtchn *lchn;
>> +int ret;
>> +
>> +if ( !port_is_valid(ld, lport) )
>> +return -EINVAL;
>> +
>> +lchn = evtchn_from_port(ld, lp
[Xen-devel] [PATCH] x86/emul: Correct the decoding of mov to/from cr/dr
The mov to/from cr/dr behave as if they were encoded with Mod = 3. When
encoded with Mod != 3, no displacement or SIB bytes are fetched.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
---
tools/tests/x86_emulator/test_x86_emulator.c | 21 +
xen/arch/x86/x86_emulate/x86_emulate.c | 20 +++-
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c
b/tools/tests/x86_emulator/test_x86_emulator.c
index 37d00f1..b588a8d 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -894,6 +894,27 @@ int main(int argc, char **argv)
}
printf("okay\n");
+printf("%-40s", "Testing mov %%cr4,%%esi (bad ModRM)...");
+/*
+ * Mod = 1, Reg = 4, R/M = 6 would normally encode a memory reference of
+ * disp8(%esi), but mov to/from cr/dr are special and behave as if they
+ * were encoded with Mod == 3.
+ */
+instr[0] = 0x0f; instr[1] = 0x20, instr[2] = 0146;
+instr[3] = 0; /* Supposed disp8. */
+regs.esi = 0;
+regs.eip = (unsigned long)&instr[0];
+rc = x86_emulate(&ctxt, &emulops);
+/*
+ * We don't care precicely what gets read from %cr4 into %esi, just so
+ * long as ModRM is treated as a register operand and 0(%esi) isn't
+ * followed as a memory reference.
+ */
+if ( (rc != X86EMUL_OKAY) ||
+ (regs.eip != (unsigned long)&instr[3]) )
+goto fail;
+printf("okay\n");
+
#define decl_insn(which) extern const unsigned char which[], \
which##_end[] asm ( ".L" #which "_end" )
#define put_insn(which, insn) ".pushsection .test, \"ax\", @progbits\n" \
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
b/xen/arch/x86/x86_emulate/x86_emulate.c
index ad62420..58be737 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2086,7 +2086,8 @@ x86_decode_twobyte(
}
/* fall through */
case 0x21: case 0x23: /* mov to/from dr */
-generate_exception_if(lock_prefix || ea.type != OP_REG, EXC_UD);
+ASSERT(ea.type == OP_REG); /* Early operand adjustment ensures this. */
+generate_exception_if(lock_prefix, EXC_UD);
op_bytes = mode_64bit() ? 8 : 4;
break;
@@ -2427,6 +2428,23 @@ x86_decode(
break;
}
}
+else if ( ext == ext_0f )
+{
+switch ( b )
+{
+case 0x20: /* mov cr,reg */
+case 0x21: /* mov dr,reg */
+case 0x22: /* mov reg,cr */
+case 0x23: /* mov reg,dr */
+/*
+ * Mov to/from cr/dr ignore the encoding of Mod, and behave as
+ * if they were encoded as reg/reg instructions. No futher
+ * disp/SIB bytes are fetched.
+ */
+modrm_mod = 3;
+break;
+}
+}
if ( modrm_mod == 3 )
{
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [ovmf test] 106484: regressions - FAIL
flight 106484 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/106484/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail REGR. vs. 105963 test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail REGR. vs. 105963 version targeted for testing: ovmf 627dcba3528159dedfb12e846840206c2f83ab32 baseline version: ovmf e0307a7dad02aa8c0cd8b3b0b9edce8ddb3fef2e Last test of basis 105963 2017-02-21 21:43:31 Z 12 days Failing since105980 2017-02-22 10:03:53 Z 12 days 33 attempts Testing same since 106484 2017-03-06 07:45:21 Z0 days1 attempts People who touched revisions under test: Anthony PERARD Ard Biesheuvel Brijesh Singh Chao Zhang Chen A Chen Dandan Bi edk2-devel On Behalf Of rthomaiy <[mailto:edk2-devel-boun...@lists.01.org]> Fu Siyuan Hao Wu Hegde Nagaraj P Jeff Fan Jiaxin Wu Jiewen Yao Laszlo Ersek Leo Duran Paolo Bonzini Qin Long Richard Thomaiyar Ruiyu Ni Star Zeng Wu Jiaxin Yonghong Zhu Zhang Lubo Zhang, Chao B jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 fail test-amd64-i386-xl-qemuu-ovmf-amd64 fail sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 2961 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/11] xen/arm: vpl011: Refactor evtchn_send in Xen to allow sending events from a xen bound channel
>>> On 06.03.17 at 11:16, wrote: > On 4 March 2017 at 02:43, Konrad Rzeszutek Wilk > wrote: >> On Tue, Feb 21, 2017 at 04:56:00PM +0530, Bhupinder Thakur wrote: >>> Breakup evtchn_send() to allow sending events for a Xen bound channel. >>> Currently, >>> there is a check in evtchn_send() i.e. is_consumer_xen() that if the event >>> channel >>> is bound to a xen consumer then event generation is not allowed for that >>> channel. >>> This check is to disallow a guest from raising an event via this channel. >>> However, >> >> Did any code archeology help in idenfitying why this was done this way? >> >> You should explain why it was done - what was the use case , and why >> your change will not change this semantic. > > I think there was never a use case earlier to generate events from the > Xen itself to the guest. The event generation always happened on > behalf of a guest via evtchn_send(), which was invoked through a > hypercall by the guest. This is not true: All interrupts being converted to events go this route, without any hypercall involved. Same for signaling qemu to do emulation of an emulated device (memory of port) access. > To disallow a guest from sending an event via > an event channel which is bound to Xen, there is a check in > evtchn_send() to check if it is a xen bound channel and if it is then > disallow sending an event via that channel because that channel is > meant only for Xen. As said in reply to your patch - I think you got directions mixed up here: You talk about Xen sending an event to a guest, but at the same time you fiddle with code preventing guests from sending events to Xen. > For pl011 emulation, there is a requirement for Xen to generate the > event to dom0 when there is data for dom0 to read in the ring buffer. This is not different from hardware signaling the need for service via an interrupt, which - always for PV guests, sometimes for HVM ones - is being converted to an event. > Since I am using a Xen bound channel so that Xen can receive events > from dom0 when there is data for Xen to read, the above mentioned > check would not allow the event to be sent to dom0. Now here you come to talk about the direction which indeed is not allowed. But breaking out part of the function won't help you, as the new (raw) function then mustn't be called as a result of some guest's request (hypercall or whatever), which includes Dom0. > Since this event is required to be generated from Xen only, it is safe > to allow to send the event from a xen bound channel. That is why I > introduced a new function raw_evtchn_send() to allow Xen to do that > while still keeping that restriction for the guests who use > evtchn_send() as it is today. And now it becomes confusing again: Yet another time you talk about Xen sending the event. Please can you cleanly separate both directions of event flow, and talk only about the relevant one in the respective contexts? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/11] xen/arm: vpl011: Refactor evtchn_send in Xen to allow sending events from a xen bound channel
Hi Jan,
On 6 March 2017 at 13:45, Jan Beulich wrote:
On 05.03.17 at 13:39, wrote:
>> My knowledge is limited for this code. So I've just CCed "The REST"
>> maintainers. Please do CC them in the future.
>
> Indeed.
>
I will take care of this in the future.
>> On 02/21/2017 11:26 AM, Bhupinder Thakur wrote:
>>> Breakup evtchn_send() to allow sending events for a Xen bound channel.
>>> Currently,
>>> there is a check in evtchn_send() i.e. is_consumer_xen() that if the event
>>> channel
>>> is bound to a xen consumer then event generation is not allowed for that
>>> channel.
>>> This check is to disallow a guest from raising an event via this channel.
>>> However,
>>> it should allow Xen to send a event via this channel as it is required for
>>> sending
>>> vpl011 event to the dom0.
>>>
>>> This change introduces a new function raw_evtchn_send() which sends the
>>> event
>>> without this check. The current evtchn_send() calls this function after
>>> doing the
>>> xen consumer check. Xen uses the raw_evtchm_send() version to send the
>>> event thus
>>> bypassing the check.
>
> Why would Xen want to send an event it is itself the consumer of?
> Surely there are better ways to communicate state internally? The
> more that you say you want the event sent to Dom0...
>
As a consumer, Xen receives event from dom0. It also needs to send
events to dom0 to indicate that there is data in the ring buffer for
dom0 to read. I am using a xen bound event channel for
sending/receiving events to/from dom0. I added a new function
raw_evtchn_send() to allow Xen to send events to dom0 without doing
the is_xen_consumer check. Note that this check is still there in
evtchn_send() to disallow guests to raise events on the xen bound
channel.
>>> @@ -650,25 +651,21 @@ static long evtchn_close(struct domain *d1, int
>>> port1, bool_t guest)
>>> return rc;
>>> }
>>>
>>> -int evtchn_send(struct domain *ld, unsigned int lport)
>>> +int raw_evtchn_send(struct domain *ld, unsigned int lport, void *data)
>>> {
>>> struct evtchn *lchn, *rchn;
>>> struct domain *rd;
>>> -intrport, ret = 0;
>>> +int rport, ret=0;
>
> There's only whitespace change here, and just the break existing
> formatting. Please avoid stray changes like this.
>
I will fix this in the next version.
>>> @@ -696,6 +693,32 @@ int evtchn_send(struct domain *ld, unsigned int lport)
>>> }
>>>
>>> out:
>>> +if ( !data )
>>> +spin_unlock(&lchn->lock);
>>> +
>>> +return ret;
>>> +}
>>> +
>>> +int evtchn_send(struct domain *ld, unsigned int lport)
>>> +{
>>> +struct evtchn *lchn;
>>> +int ret;
>>> +
>>> +if ( !port_is_valid(ld, lport) )
>>> +return -EINVAL;
>>> +
>>> +lchn = evtchn_from_port(ld, lport);
>>> +
>>> +spin_lock(&lchn->lock);
>>> +
>>> +if ( unlikely(consumer_is_xen(lchn)) )
>>> +{
>>> +printk("evtchn_send failed to send via xen event channel\n");
>>> +return -EINVAL;
>
> As a general remark: Splitting out functionality into a new function
> should _never_ be accompanied by silent behavioral changes for
> pre-existing code paths. In the case here there was no printk()
> before, and I see no justification for one to be added.
>
Ok. I will remove the printk.
> Jan
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] (no subject)
On Mon, Feb 27, 2017 at 6:12 PM, Dmitry Rockosov wrote: > Hi guys, > > Do you know when *recognized* Virtual Interrupt on VM-Entry will be > delivered if Virtual-Interrupt Delivery is enabled and interrupt delivery is > blocking by STI? > > Previously, VMM used Interrupt-Window, but as I see in XEN code, > Interrupt-Window is not used when Virtual Interrupt Delivery is enabled. > > Does it mean, we will get Virtual Interrupt on the next VM-entry? CC'ing the VMX maintainer. -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: Allow a default compiled-in command line using Kconfig
On 06/03/17 02:38, Sky Liu wrote:
> Added two new config entries in common/Kconfig: CMDLINE_BOOL and CMDLINE.
Hello. Thankyou for looking into this task.
As a start, why do you introduce CMDLINE_BOOL? you don't actually use it.
> These two entries enable an embedded command line
> to be compiled in the hypervisor.
> If CMDLINE_BOOL is set to Y, both arm and x86 startup routines
> will call cmdline_parse() first on CMDLINE,
> then on the line passed by the bootloader.
> This allows downstreams to set their defaults
> without modifying the source code all over the place.
> Also probably useful for the embedded space.
>
> See Also:
> https://xenproject.atlassian.net/projects/XEN/issues/XEN-41?filter=allopenissues
If you are going to include this URL, please drop the ?filter=allopenissues.
>
> Signed-off-by: Zhongze Liu
> ---
> xen/arch/arm/setup.c | 5 +
> xen/arch/x86/setup.c | 5 +
> xen/common/Kconfig | 27 +++
> 3 files changed, 37 insertions(+)
>
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 92a2de6b70..55860a1299 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -729,6 +729,11 @@ void __init start_xen(unsigned long boot_phys_offset,
> + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
> fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr);
>
> +#ifdef CONFIG_CMDLINE
> +printk("Compiled-in command line: %s\n", CONFIG_CMDLINE);
> +cmdline_parse(cmdline);
> +#endif
You can be rather more cunning, and avoid the #ifdefary here.
Given something like this:
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 4b87c60..2da9cfe 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -24,6 +24,8 @@ enum system_state system_state = SYS_STATE_early_boot;
xen_commandline_t saved_cmdline;
+const char opt_def_cmdline[] = CONFIG_CMDLINE;
+
static void __init assign_integer_param(
const struct kernel_param *param, uint64_t val)
{
diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index 548b64d..99a428d 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -100,5 +100,7 @@ extern enum system_state {
bool_t is_active_kernel_text(unsigned long addr);
+extern const char opt_def_cmdline[];
+
#endif /* _LINUX_KERNEL_H */
Your code in the various setup.c's can be
if ( strlen(opt_def_cmdline) )
{
printk("Compiled-in command line: %s\n", opt_def_cmdline);
cmdline_parse(opt_def_cmdline);
}
which is rather neater and not liable to code-rot.
> +
> cmdline = boot_fdt_cmdline(device_tree_flattened);
> printk("Command line: %s\n", cmdline);
> cmdline_parse(cmdline);
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index dab67d5491..2b961f95cf 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -690,6 +690,11 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> kextra += 3;
> while ( kextra[1] == ' ' ) kextra++;
> }
> +
> +#ifdef CONFIG_CMDLINE
> +printk("Compiled-in command line: %s\n", CONFIG_CMDLINE);
> +cmdline_parse(CONFIG_CMDLINE);
> +#endif
> cmdline_parse(cmdline);
>
> /* Must be after command line argument parsing and before
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index f2ecbc43d6..1afcd8142c 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -237,4 +237,31 @@ config FAST_SYMBOL_LOOKUP
>The only user of this is Live patching.
>
>If unsure, say Y.
> +
> +config CMDLINE_BOOL
> + bool "Built-in hypervisor command line"
> + default n
> + ---help---
> + Allow for specifying boot arguments to the hypervisor at
> + build time. On some systems (e.g. embedded ones), it is
> + necessary or convenient to provide some or all of the
> + hypervisor boot arguments with the hypervisor itself (that is,
> + to not rely on the boot loader to provide them.)
> +
> + To compile command line arguments into the hypervisor,
> + set this option to 'Y', then fill in the
> + boot arguments in CONFIG_CMDLINE.
> +
> +config CMDLINE
> + string "Built-in hypervisor command string"
> + depends on CMDLINE_BOOL
> + default ""
> + ---help---
> + Enter arguments here that should be compiled into the hypervisor
> + image and used at boot time. If the boot loader provides a
> + command line at boot time, it is appended to this string to
> + form the full hypervisor command line, when the system boots.
As Jan identified, please make the indentation here match the rest of
the file.
You should state that if present, this command line is parsed first, so
parameters from the bootloader will take precedence.
> +
> + However, you can use the CONFIG_CMDLINE_OVERRIDE option to
> + change this behavior.
This line isn't applicable to Xen.
~Andrew
> endmenu
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/11] xen/arm: vpl011: Refactor evtchn_send in Xen to allow sending events from a xen bound channel
>>> On 06.03.17 at 11:44, wrote: > On 6 March 2017 at 13:45, Jan Beulich wrote: > On 05.03.17 at 13:39, wrote: >>> On 02/21/2017 11:26 AM, Bhupinder Thakur wrote: Breakup evtchn_send() to allow sending events for a Xen bound channel. Currently, there is a check in evtchn_send() i.e. is_consumer_xen() that if the event channel is bound to a xen consumer then event generation is not allowed for that channel. This check is to disallow a guest from raising an event via this channel. However, it should allow Xen to send a event via this channel as it is required for sending vpl011 event to the dom0. This change introduces a new function raw_evtchn_send() which sends the event without this check. The current evtchn_send() calls this function after doing the xen consumer check. Xen uses the raw_evtchm_send() version to send the event thus bypassing the check. >> >> Why would Xen want to send an event it is itself the consumer of? >> Surely there are better ways to communicate state internally? The >> more that you say you want the event sent to Dom0... >> > As a consumer, Xen receives event from dom0. It also needs to send > events to dom0 to indicate that there is data in the ring buffer for > dom0 to read. I am using a xen bound event channel for > sending/receiving events to/from dom0. I added a new function > raw_evtchn_send() to allow Xen to send events to dom0 without doing > the is_xen_consumer check. Note that this check is still there in > evtchn_send() to disallow guests to raise events on the xen bound > channel. I can see why Xen needs to send events; I can't see why Dom0 couldn't simply make a hypercall instead of sending an event if it needs to signal something. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] xen: Allow a default compiled-in command line using Kconfig
Added 3 new config entries in common/Kconfig:
CMDLINE_BOOL, CMDLINE and CMDLINE_OVERRIDE
These 3 entries enable an embedded command line
to be compiled in the hypervisor.
If CMDLINE_BOOL is set to y, both arm and x86 startup routines
will combine the compiled-in command line and the boot loader command line
together to form a complete command line before calling cmdline_parse().
And if CMDLINE_OVERRIDE is set to y, boot loader command line
will be ignored.
This allows downstreams to set their defaults
without modifying the source code all over the place.
Also probably useful for the embedded space.
See Also:
https://xenproject.atlassian.net/projects/XEN/issues/XEN-41?filter=allopenissues
Signed-off-by: Zhongze Liu
---
Changed since v1:
* Added the missing CMDLINE_OVERRIDE config entry.
* Combined built-in and boot loader command lines into one command line
before calling cmdline_parse() (instead of calling cmdline_parse() twice).
* Handled dom0 command line options (options after " -- " belong to dom0)
in the x86 setup routine.
---
xen/arch/arm/setup.c | 15 ++--
xen/arch/x86/setup.c | 66 ++--
xen/common/Kconfig | 38 ++
3 files changed, 99 insertions(+), 20 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 92a2de6b70..9a68bda0fe 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -706,6 +706,7 @@ void __init start_xen(unsigned long boot_phys_offset,
int cpus, i;
paddr_t xen_paddr;
const char *cmdline;
+static xen_commandline_t __initdata complete_cmdline;
struct bootmodule *xen_bootmodule;
struct domain *dom0;
struct xen_arch_domainconfig config;
@@ -729,9 +730,19 @@ void __init start_xen(unsigned long boot_phys_offset,
+ (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr);
+#ifdef CONFIG_CMDLINE
+safe_strcpy(complete_cmdline, CONFIG_CMDLINE);
+printk("Compiled-in command line: %s\n", complete_cmdline);
+#endif
+
+#ifndef CONFIG_CMDLINE_OVERRIDE
cmdline = boot_fdt_cmdline(device_tree_flattened);
-printk("Command line: %s\n", cmdline);
-cmdline_parse(cmdline);
+safe_strcat(complete_cmdline, " ");
+safe_strcat(complete_cmdline, cmdline);
+printk("Command line: %s\n", complete_cmdline);
+#endif
+
+cmdline_parse(complete_cmdline);
/* Register Xen's load address as a boot module. */
xen_bootmodule = add_boot_module(BOOTMOD_XEN,
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index dab67d5491..b8899aa443 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -636,10 +636,34 @@ static char * __init cmdline_cook(char *p, const char
*loader_name)
return p;
}
+/*
+ * Extracts dom0 options from the commandline.
+ *
+ * Options after ' -- ' separator belong to dom0.
+ * 1. Orphan dom0's options from Xen's command line.
+ * 2. Skip all but final leading space from dom0's options.
+ */
+
+static inline char* __init extract_dom0_options(char *cmdline)
+{
+char *kextra;
+
+if ( (kextra = strstr(cmdline, " -- ")) != NULL )
+{
+*kextra = '\0';
+kextra += 3;
+while ( kextra[1] == ' ' ) kextra++;
+}
+
+return kextra;
+}
+
void __init noreturn __start_xen(unsigned long mbi_p)
{
char *memmap_type = NULL;
char *cmdline, *kextra, *loader;
+static xen_commandline_t __initdata complete_cmdline;
+static char __initdata complete_kextra[MAX_GUEST_CMDLINE];
unsigned int initrdidx, domcr_flags = DOMCRF_s3_integrity;
multiboot_info_t *mbi = __va(mbi_p);
module_t *mod = (module_t *)__va(mbi->mods_addr);
@@ -672,25 +696,31 @@ void __init noreturn __start_xen(unsigned long mbi_p)
/* Full exception support from here on in. */
+#ifdef CONFIG_CMDLINE
+/* Process the built-in command line options. */
+
+safe_strcpy(complete_cmdline, CONFIG_CMDLINE);
+if ( (kextra = extract_dom0_options(complete_cmdline)) != NULL )
+safe_strcpy(complete_kextra, kextra);
+printk("Compiled-in command line: %s\n", complete_cmdline);
+#endif
+
loader = (mbi->flags & MBI_LOADERNAME)
? (char *)__va(mbi->boot_loader_name) : "unknown";
-/* Parse the command-line options. */
+#ifndef CONFIG_CMDLINE_OVERRIDE
+/* Parse the command-line options passed by the bootloader. */
+
cmdline = cmdline_cook((mbi->flags & MBI_CMDLINE) ?
__va(mbi->cmdline) : NULL,
loader);
-if ( (kextra = strstr(cmdline, " -- ")) != NULL )
-{
-/*
- * Options after ' -- ' separator belong to dom0.
- * 1. Orphan dom0's options from Xen's command line.
- * 2. Skip all but final leading space from dom0's options.
- */
-*kextra = '\0';
-kextra += 3;
-while ( kextra[1] == ' ' ) kextra++;
-}
-cmdline_parse(cmdl
Re: [Xen-devel] [PATCH 03/11] xen/arm: vpl011: Refactor evtchn_send in Xen to allow sending events from a xen bound channel
Hi, On 6 March 2017 at 16:24, Jan Beulich wrote: On 06.03.17 at 11:44, wrote: >> On 6 March 2017 at 13:45, Jan Beulich wrote: >> On 05.03.17 at 13:39, wrote: On 02/21/2017 11:26 AM, Bhupinder Thakur wrote: > Breakup evtchn_send() to allow sending events for a Xen bound channel. > Currently, > there is a check in evtchn_send() i.e. is_consumer_xen() that if the > event channel > is bound to a xen consumer then event generation is not allowed for that > channel. > This check is to disallow a guest from raising an event via this channel. > However, > it should allow Xen to send a event via this channel as it is required > for sending > vpl011 event to the dom0. > > This change introduces a new function raw_evtchn_send() which sends the > event > without this check. The current evtchn_send() calls this function after > doing the > xen consumer check. Xen uses the raw_evtchm_send() version to send the > event thus > bypassing the check. >>> >>> Why would Xen want to send an event it is itself the consumer of? >>> Surely there are better ways to communicate state internally? The >>> more that you say you want the event sent to Dom0... >>> >> As a consumer, Xen receives event from dom0. It also needs to send >> events to dom0 to indicate that there is data in the ring buffer for >> dom0 to read. I am using a xen bound event channel for >> sending/receiving events to/from dom0. I added a new function >> raw_evtchn_send() to allow Xen to send events to dom0 without doing >> the is_xen_consumer check. Note that this check is still there in >> evtchn_send() to disallow guests to raise events on the xen bound >> channel. > > I can see why Xen needs to send events; I can't see why Dom0 couldn't > simply make a hypercall instead of sending an event if it needs to signal > something. > We decided to reuse the same PV console interface for pl011 as used for PV console in xenconsole running on dom0, which is events/ring buffer based. From xenconsole point of view, there is no difference in terms of handling a pl011 console and a PV console. > Jan > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen: Allow a default compiled-in command line using Kconfig
Hello, On 06/03/17 11:01, Zhongze Liu wrote: Added 3 new config entries in common/Kconfig: CMDLINE_BOOL, CMDLINE and CMDLINE_OVERRIDE These 3 entries enable an embedded command line to be compiled in the hypervisor. If CMDLINE_BOOL is set to y, both arm and x86 startup routines will combine the compiled-in command line and the boot loader command line together to form a complete command line before calling cmdline_parse(). And if CMDLINE_OVERRIDE is set to y, boot loader command line will be ignored. This behavior seems a little odd to me. Why would you want to append the two command line? And more importantly if the same option is passed twice, how will this behave? For example the compile command line could contain iommu=off, but the bootloader is passing iommu=on. What would be the final state of the IOMMU? This one is quite simple, but you could have options which set flags. So if you want to override it, the new option would have to unset it. This allows downstreams to set their defaults without modifying the source code all over the place. Also probably useful for the embedded space. See Also: https://xenproject.atlassian.net/projects/XEN/issues/XEN-41?filter=allopenissues A shorter link is https://xenproject.atlassian.net/browse/XEN-41 Signed-off-by: Zhongze Liu Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: Allow a default compiled-in command line using Kconfig
2017-03-06 18:50 GMT+08:00 Andrew Cooper :
> On 06/03/17 02:38, Sky Liu wrote:
>> Added two new config entries in common/Kconfig: CMDLINE_BOOL and CMDLINE.
>
> Hello. Thankyou for looking into this task.
>
Hi, Cooper. Thanks for your suggestions.
>
> As a start, why do you introduce CMDLINE_BOOL? you don't actually use it.
>
My original motivation was to make CMDLINE and CMDLINE_OVERRIDE depends on
CMDLINE_BOOL, which is off by default. But I forget to add
CMDLINE_OVERRIDE in this patch.
>
>> These two entries enable an embedded command line
>> to be compiled in the hypervisor.
>> If CMDLINE_BOOL is set to Y, both arm and x86 startup routines
>> will call cmdline_parse() first on CMDLINE,
>> then on the line passed by the bootloader.
>> This allows downstreams to set their defaults
>> without modifying the source code all over the place.
>> Also probably useful for the embedded space.
>>
>> See Also:
>> https://xenproject.atlassian.net/projects/XEN/issues/XEN-41?filter=allopenissues
>
> If you are going to include this URL, please drop the ?filter=allopenissues.
>
I'll drop that in the next version.
>
>>
>> Signed-off-by: Zhongze Liu
>> ---
>> xen/arch/arm/setup.c | 5 +
>> xen/arch/x86/setup.c | 5 +
>> xen/common/Kconfig | 27 +++
>> 3 files changed, 37 insertions(+)
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 92a2de6b70..55860a1299 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -729,6 +729,11 @@ void __init start_xen(unsigned long boot_phys_offset,
>> + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
>> fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr);
>>
>> +#ifdef CONFIG_CMDLINE
>> +printk("Compiled-in command line: %s\n", CONFIG_CMDLINE);
>> +cmdline_parse(cmdline);
>> +#endif
>
> You can be rather more cunning, and avoid the #ifdefary here.
>
> Given something like this:
>
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index 4b87c60..2da9cfe 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -24,6 +24,8 @@ enum system_state system_state = SYS_STATE_early_boot;
>
> xen_commandline_t saved_cmdline;
>
> +const char opt_def_cmdline[] = CONFIG_CMDLINE;
> +
> static void __init assign_integer_param(
> const struct kernel_param *param, uint64_t val)
> {
> diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
> index 548b64d..99a428d 100644
> --- a/xen/include/xen/kernel.h
> +++ b/xen/include/xen/kernel.h
> @@ -100,5 +100,7 @@ extern enum system_state {
>
> bool_t is_active_kernel_text(unsigned long addr);
>
> +extern const char opt_def_cmdline[];
> +
> #endif /* _LINUX_KERNEL_H */
>
>
> Your code in the various setup.c's can be
>
> if ( strlen(opt_def_cmdline) )
> {
> printk("Compiled-in command line: %s\n", opt_def_cmdline);
> cmdline_parse(opt_def_cmdline);
> }
>
> which is rather neater and not liable to code-rot.
>
>> +
>> cmdline = boot_fdt_cmdline(device_tree_flattened);
>> printk("Command line: %s\n", cmdline);
>> cmdline_parse(cmdline);
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index dab67d5491..2b961f95cf 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -690,6 +690,11 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>> kextra += 3;
>> while ( kextra[1] == ' ' ) kextra++;
>> }
>> +
>> +#ifdef CONFIG_CMDLINE
>> +printk("Compiled-in command line: %s\n", CONFIG_CMDLINE);
>> +cmdline_parse(CONFIG_CMDLINE);
>> +#endif
>> cmdline_parse(cmdline);
>>
>> /* Must be after command line argument parsing and before
>> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
>> index f2ecbc43d6..1afcd8142c 100644
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -237,4 +237,31 @@ config FAST_SYMBOL_LOOKUP
>>The only user of this is Live patching.
>>
>>If unsure, say Y.
>> +
>> +config CMDLINE_BOOL
>> + bool "Built-in hypervisor command line"
>> + default n
>> + ---help---
>> + Allow for specifying boot arguments to the hypervisor at
>> + build time. On some systems (e.g. embedded ones), it is
>> + necessary or convenient to provide some or all of the
>> + hypervisor boot arguments with the hypervisor itself (that is,
>> + to not rely on the boot loader to provide them.)
>> +
>> + To compile command line arguments into the hypervisor,
>> + set this option to 'Y', then fill in the
>> + boot arguments in CONFIG_CMDLINE.
>> +
>> +config CMDLINE
>> + string "Built-in hypervisor command string"
>> + depends on CMDLINE_BOOL
>> + default ""
>> + ---help---
>> + Enter arguments here that should be compiled into the hypervisor
>> + image and used at boot time. If the boot loader provides a
>> + command line at boot time, it is appended to this string to
>> + form the full hypervisor command line, when the system boots.
>
> As Jan identified, please make the indentation here match the rest of
> the file.
>
Re: [Xen-devel] [PATCH] xen: Allow a default compiled-in command line using Kconfig
>>> On 06.03.17 at 11:50, wrote:
> On 06/03/17 02:38, Sky Liu wrote:
>> Added two new config entries in common/Kconfig: CMDLINE_BOOL and CMDLINE.
>
> Hello. Thankyou for looking into this task.
>
> As a start, why do you introduce CMDLINE_BOOL? you don't actually use it.
>
>> These two entries enable an embedded command line
>> to be compiled in the hypervisor.
>> If CMDLINE_BOOL is set to Y, both arm and x86 startup routines
>> will call cmdline_parse() first on CMDLINE,
>> then on the line passed by the bootloader.
>> This allows downstreams to set their defaults
>> without modifying the source code all over the place.
>> Also probably useful for the embedded space.
>>
>> See Also:
> https://xenproject.atlassian.net/projects/XEN/issues/XEN-41?filter=allopeniss
> ues
>
> If you are going to include this URL, please drop the ?filter=allopenissues.
>
>>
>> Signed-off-by: Zhongze Liu
>> ---
>> xen/arch/arm/setup.c | 5 +
>> xen/arch/x86/setup.c | 5 +
>> xen/common/Kconfig | 27 +++
>> 3 files changed, 37 insertions(+)
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 92a2de6b70..55860a1299 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -729,6 +729,11 @@ void __init start_xen(unsigned long boot_phys_offset,
>> + (fdt_paddr & ((1 << SECOND_SHIFT) - 1));
>> fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr);
>>
>> +#ifdef CONFIG_CMDLINE
>> +printk("Compiled-in command line: %s\n", CONFIG_CMDLINE);
>> +cmdline_parse(cmdline);
>> +#endif
>
> You can be rather more cunning, and avoid the #ifdefary here.
>
> Given something like this:
>
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index 4b87c60..2da9cfe 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -24,6 +24,8 @@ enum system_state system_state = SYS_STATE_early_boot;
>
> xen_commandline_t saved_cmdline;
>
> +const char opt_def_cmdline[] = CONFIG_CMDLINE;
> +
> static void __init assign_integer_param(
> const struct kernel_param *param, uint64_t val)
> {
> diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
> index 548b64d..99a428d 100644
> --- a/xen/include/xen/kernel.h
> +++ b/xen/include/xen/kernel.h
> @@ -100,5 +100,7 @@ extern enum system_state {
>
> bool_t is_active_kernel_text(unsigned long addr);
>
> +extern const char opt_def_cmdline[];
> +
> #endif /* _LINUX_KERNEL_H */
>
>
> Your code in the various setup.c's can be
>
> if ( strlen(opt_def_cmdline) )
> {
> printk("Compiled-in command line: %s\n", opt_def_cmdline);
> cmdline_parse(opt_def_cmdline);
> }
>
> which is rather neater and not liable to code-rot.
I would go even farther and have cmdline_parse() invoke itself with
the static string, instead of repeating the same code per arch. This
also avoids the need to make opt_def_cmdline[] non-static.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen: Allow a default compiled-in command line using Kconfig
>>> On 06.03.17 at 12:15, wrote: > On 06/03/17 11:01, Zhongze Liu wrote: >> Added 3 new config entries in common/Kconfig: >> CMDLINE_BOOL, CMDLINE and CMDLINE_OVERRIDE >> >> These 3 entries enable an embedded command line >> to be compiled in the hypervisor. >> >> If CMDLINE_BOOL is set to y, both arm and x86 startup routines >> will combine the compiled-in command line and the boot loader command line >> together to form a complete command line before calling cmdline_parse(). >> And if CMDLINE_OVERRIDE is set to y, boot loader command line >> will be ignored. > > This behavior seems a little odd to me. Why would you want to append the > two command line? And more importantly if the same option is passed > twice, how will this behave? > > For example the compile command line could contain iommu=off, but the > bootloader is passing iommu=on. What would be the final state of the IOMMU? > > This one is quite simple, but you could have options which set flags. So > if you want to override it, the new option would have to unset it. I don't understand this concern - overriding an earlier option by a later one is (supposed to be) well defined, and we even suggest this to people when wanting to try out options without having to edit bootloader (or whatever) config files. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen: Allow a default compiled-in command line using Kconfig
On 06/03/17 11:32, Jan Beulich wrote: On 06.03.17 at 12:15, wrote: On 06/03/17 11:01, Zhongze Liu wrote: Added 3 new config entries in common/Kconfig: CMDLINE_BOOL, CMDLINE and CMDLINE_OVERRIDE These 3 entries enable an embedded command line to be compiled in the hypervisor. If CMDLINE_BOOL is set to y, both arm and x86 startup routines will combine the compiled-in command line and the boot loader command line together to form a complete command line before calling cmdline_parse(). And if CMDLINE_OVERRIDE is set to y, boot loader command line will be ignored. This behavior seems a little odd to me. Why would you want to append the two command line? And more importantly if the same option is passed twice, how will this behave? For example the compile command line could contain iommu=off, but the bootloader is passing iommu=on. What would be the final state of the IOMMU? This one is quite simple, but you could have options which set flags. So if you want to override it, the new option would have to unset it. I don't understand this concern - overriding an earlier option by a later one is (supposed to be) well defined, and we even suggest this to people when wanting to try out options without having to edit bootloader (or whatever) config files. My concern is not about the override itself but about appending two commandlines option without any explanation whether the built-in commandline will be parsed first. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 02/11] xen/arm: vpl011: Add new hvm params in Xen for ring buffer/event setup
Hi Jan, On 06/03/17 08:06, Jan Beulich wrote: On 05.03.17 at 13:35, wrote: On 02/21/2017 11:25 AM, Bhupinder Thakur wrote: --- a/xen/include/public/hvm/params.h +++ b/xen/include/public/hvm/params.h @@ -203,10 +203,17 @@ */ #define HVM_PARAM_ACPI_IOPORTS_LOCATION 19 -/* Deprecated */ +#if defined(__arm__) || defined(__aarch64__) +#define HVM_PARAM_VPL011_CONSOLE_PFN20 +#define HVM_PARAM_VPL011_CONSOLE_EVTCHN 21 +#define HVM_PARAM_VPL011_VIRQ 22 +#else #define HVM_PARAM_MEMORY_EVENT_CR0 20 #define HVM_PARAM_MEMORY_EVENT_CR3 21 #define HVM_PARAM_MEMORY_EVENT_CR4 22 Those parameters are still deprecated but you drop the comment stating that. +#endif + Those params are x86 specific so should have never been set on ARM. But I am not sure if it is fine to re-purpose deprecated number. I have CCed "The REST" maintainers to have their input here. I think re-purposing something that was never (meant to be) used is fine in a case like this. However, the question is moot with your suggestion to not use params here in the first place. I suggested to drop HVM_PARAM_VPL011_VIRQ because we can hardcode the guest interrupt number for the UART as we already do for the MMIO region. The 2 other HVM_PARAM looks sensible to me. I thought a bit more about those params. I think the name should be generic and not tie to pl011 because we may want to emulate different UART for the guest in the future. Also, by re-using deprecated encoding it means that it will not be possible to use those parameters on x86 if you ever decide to emulate UART in Xen. I am not sure whether if you are happy with that? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable test] 106482: tolerable FAIL
flight 106482 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/106482/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 9 debian-hvm-install fail in 106469 pass in 106482 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 15 guest-localmigrate/x10 fail pass in 106469 test-armhf-armhf-libvirt 18 leak-check/check fail pass in 106469 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 106469 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 106469 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 106469 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 106469 test-armhf-armhf-libvirt 13 saverestore-support-checkfail like 106469 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail like 106469 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail like 106469 test-amd64-amd64-xl-rtds 9 debian-install fail like 106469 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass build-arm64 5 xen-buildfail never pass build-arm64-xsm 5 xen-buildfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass build-arm64-pvops 5 kernel-build fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 saverestore-support-checkfail never pass version targeted for testing: xen 6d55c0c316357a412526b9dccd45d3c3abb75227 baseline version: xen 6d55c0c316357a412526b9dccd45d3c3abb75227 Last test of basis 106482 2017-03-06 01:57:48 Z0 days Testing same since0 1970-01-01 00:00:00 Z 17231 days0 attempts jobs: build-amd64-xsm pass build-arm64-xsm fail build-armhf-xsm
Re: [Xen-devel] [PATCH v2] xen: Allow a default compiled-in command line using Kconfig
>>> On 06.03.17 at 12:38, wrote: > > On 06/03/17 11:32, Jan Beulich wrote: > On 06.03.17 at 12:15, wrote: >>> On 06/03/17 11:01, Zhongze Liu wrote: Added 3 new config entries in common/Kconfig: CMDLINE_BOOL, CMDLINE and CMDLINE_OVERRIDE These 3 entries enable an embedded command line to be compiled in the hypervisor. If CMDLINE_BOOL is set to y, both arm and x86 startup routines will combine the compiled-in command line and the boot loader command line together to form a complete command line before calling cmdline_parse(). And if CMDLINE_OVERRIDE is set to y, boot loader command line will be ignored. >>> >>> This behavior seems a little odd to me. Why would you want to append the >>> two command line? And more importantly if the same option is passed >>> twice, how will this behave? >>> >>> For example the compile command line could contain iommu=off, but the >>> bootloader is passing iommu=on. What would be the final state of the IOMMU? >>> >>> This one is quite simple, but you could have options which set flags. So >>> if you want to override it, the new option would have to unset it. >> >> I don't understand this concern - overriding an earlier option by a >> later one is (supposed to be) well defined, and we even suggest this >> to people when wanting to try out options without having to edit >> bootloader (or whatever) config files. > > My concern is not about the override itself but about appending two > commandlines option without any explanation whether the built-in > commandline will be parsed first. Oh, agreed - as Andrew has also pointed out, the ordering (and hence override behavior) needs to be clearly spelled out here. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 0/2] Build fixes for clang 4.0
Hello, This series makes Xen (both hypervisor and tools) build with clang 4.0. Note that patch #2 is well... unfortunate, but I couldn't find any other way to solve this (although I'm open to suggestions). I'm also afraid that issues similar to this one might come up in future compiler versions with XSM. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from
working as expected, and vpmu.o ends up with a reference to
__xsm_action_mismatch_detected which makes the build fail:
[...]
ld-melf_x86_64_fbsd -T xen.lds -N prelink.o \
xen/common/symbols-dummy.o -o xen/.xen-syms.0
prelink.o: In function `xsm_default_action':
xen/include/xsm/dummy.h:80: undefined reference to
`__xsm_action_mismatch_detected'
xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32
against undefined symbol `__xsm_action_mismatch_detected'
ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' isn't
defined
Then doing a search in the objects files:
# find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \
'for filename; do nm "$filename" | \
grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash
xen/arch/x86/prelink.o
xen/arch/x86/cpu/vpmu.o
xen/arch/x86/cpu/built_in.o
xen/arch/x86/built_in.o
The current patch is the only way I've found to fix this so far, by simply
moving the XSM_PRIV check into the default case in xsm_pmu_op. This also fixes
the behavior of do_xenpmu_op, which will now return -EINVAL for unknown
XENPMU_* operations, instead of -EPERM when called by a privileged domain.
Signed-off-by: Roger Pau Monné
---
Cc: Daniel De Graaf
---
xen/include/xsm/dummy.h | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 4b27ae7..ff73039 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -682,18 +682,13 @@ static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG struct
domain *d, unsigned int
XSM_ASSERT_ACTION(XSM_OTHER);
switch ( op )
{
-case XENPMU_mode_set:
-case XENPMU_mode_get:
-case XENPMU_feature_set:
-case XENPMU_feature_get:
-return xsm_default_action(XSM_PRIV, d, current->domain);
case XENPMU_init:
case XENPMU_finish:
case XENPMU_lvtpc_set:
case XENPMU_flush:
return xsm_default_action(XSM_HOOK, d, current->domain);
default:
-return -EPERM;
+return xsm_default_action(XSM_PRIV, d, current->domain);
}
}
--
2.10.1 (Apple Git-78)
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 1/2] build/clang: remove the address-of-packed-member warning
Clang 4.0 has added a new check that triggers when taking the address of a field of a packed structure. I've explained our situation to the llvm/clang folks, but nobody seem to care: https://reviews.llvm.org/D20561 So simply disable that check if present. Signed-off-by: Roger Pau Monné --- Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Tim Deegan Cc: Wei Liu --- Config.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/Config.mk b/Config.mk index 81550a7..e9f60c7 100644 --- a/Config.mk +++ b/Config.mk @@ -216,6 +216,7 @@ $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement) $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement) $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable) $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs) +$(call cc-option-add,CFLAGS,CC,-Wno-address-of-packed-member) LDFLAGS += $(foreach i, $(EXTRA_LIB), -L$(i)) CFLAGS += $(foreach i, $(EXTRA_INCLUDES), -I$(i)) -- 2.10.1 (Apple Git-78) ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 02/11] xen/arm: vpl011: Add new hvm params in Xen for ring buffer/event setup
>>> On 06.03.17 at 12:42, wrote: > I thought a bit more about those params. I think the name should be > generic and not tie to pl011 because we may want to emulate different > UART for the guest in the future. That's reasonable, but I continue to have reservations against the underlying approach here, namely ... > Also, by re-using deprecated encoding it means that it will not be > possible to use those parameters on x86 if you ever decide to emulate > UART in Xen. I am not sure whether if you are happy with that? ... with this in mind: If we wanted to do this for HVM guests, we'd indeed want the parameters. If we wanted this for PV guests, the model wouldn't fit at all. Plus what I'm not understanding (perhaps because most of the discussion around this seemed to be ARM-specific, and hence I've skipped reading through it) is - why is this UART emulation needed in the first place? We've never had a need for such on x86, afaia. Furthermore - how does this scale? I.e. what if other devices pop up wanting to be emulated? Or multiple UARTs? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] build/clang: remove the address-of-packed-member warning
>>> On 06.03.17 at 13:31, wrote: > Clang 4.0 has added a new check that triggers when taking the address of a > field of a packed structure. I've explained our situation to the llvm/clang > folks, but nobody seem to care: > > https://reviews.llvm.org/D20561 > > So simply disable that check if present. > > Signed-off-by: Roger Pau Monné Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
>>> On 06.03.17 at 13:31, wrote: > There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from > working as expected, and vpmu.o ends up with a reference to > __xsm_action_mismatch_detected which makes the build fail: > > [...] > ld-melf_x86_64_fbsd -T xen.lds -N prelink.o \ > xen/common/symbols-dummy.o -o xen/.xen-syms.0 > prelink.o: In function `xsm_default_action': > xen/include/xsm/dummy.h:80: undefined reference to > `__xsm_action_mismatch_detected' > xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 > against undefined symbol `__xsm_action_mismatch_detected' > ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' > isn't defined > > Then doing a search in the objects files: > > # find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \ > 'for filename; do nm "$filename" | \ > grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash > xen/arch/x86/prelink.o > xen/arch/x86/cpu/vpmu.o > xen/arch/x86/cpu/built_in.o > xen/arch/x86/built_in.o > > The current patch is the only way I've found to fix this so far, by simply > moving the XSM_PRIV check into the default case in xsm_pmu_op. This also fixes > the behavior of do_xenpmu_op, which will now return -EINVAL for unknown > XENPMU_* operations, instead of -EPERM when called by a privileged domain. Especially from this perspective I think the patch is fine (but also Cc-ing Boris), yet I still think the compilation aspect needs to be got to the bottom of, to have a complete picture in case the problem shows up in a slightly different way elsewhere. Did you report this to clang folks? Did they have any explanation of what's going on? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/emul: Correct the decoding of mov to/from cr/dr
>>> On 06.03.17 at 11:30, wrote:
> The mov to/from cr/dr behave as if they were encoded with Mod = 3. When
> encoded with Mod != 3, no displacement or SIB bytes are fetched.
Would mind letting us know how you became aware of this oddity?
It's clearly both unexpected and undocumented, and I wonder
whether there are any other opcodes behaving the same. In any
even I think we want to make at least an attempt at having HW
vendors confirm this (and to address the other-opcodes concern),
so I'm extending the Cc list.
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -894,6 +894,27 @@ int main(int argc, char **argv)
> }
> printf("okay\n");
>
> +printf("%-40s", "Testing mov %%cr4,%%esi (bad ModRM)...");
> +/*
> + * Mod = 1, Reg = 4, R/M = 6 would normally encode a memory reference of
> + * disp8(%esi), but mov to/from cr/dr are special and behave as if they
> + * were encoded with Mod == 3.
> + */
> +instr[0] = 0x0f; instr[1] = 0x20, instr[2] = 0146;
I can guess why you've done it this way, but I'd prefer if we didn't
start using octal numbers. I for one am very used to reading ModRM
bytes in their hex representation.
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -2086,7 +2086,8 @@ x86_decode_twobyte(
> }
> /* fall through */
> case 0x21: case 0x23: /* mov to/from dr */
> -generate_exception_if(lock_prefix || ea.type != OP_REG, EXC_UD);
> +ASSERT(ea.type == OP_REG); /* Early operand adjustment ensures this.
> */
> +generate_exception_if(lock_prefix, EXC_UD);
> op_bytes = mode_64bit() ? 8 : 4;
> break;
>
> @@ -2427,6 +2428,23 @@ x86_decode(
> break;
> }
> }
> +else if ( ext == ext_0f )
> +{
> +switch ( b )
> +{
> +case 0x20: /* mov cr,reg */
> +case 0x21: /* mov dr,reg */
> +case 0x22: /* mov reg,cr */
> +case 0x23: /* mov reg,dr */
> +/*
> + * Mov to/from cr/dr ignore the encoding of Mod, and behave
> as
> + * if they were encoded as reg/reg instructions. No futher
> + * disp/SIB bytes are fetched.
> + */
> +modrm_mod = 3;
> +break;
> +}
> +}
Just like done by
https://lists.xenproject.org/archives/html/xen-devel/2017-03/msg00588.html
this calls for the outer if/else to be morphed into a switch().
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 02/11] xen/arm: vpl011: Add new hvm params in Xen for ring buffer/event setup
Hi Jan, On 06/03/17 12:41, Jan Beulich wrote: On 06.03.17 at 12:42, wrote: I thought a bit more about those params. I think the name should be generic and not tie to pl011 because we may want to emulate different UART for the guest in the future. That's reasonable, but I continue to have reservations against the underlying approach here, namely ... Also, by re-using deprecated encoding it means that it will not be possible to use those parameters on x86 if you ever decide to emulate UART in Xen. I am not sure whether if you are happy with that? ... with this in mind: If we wanted to do this for HVM guests, we'd indeed want the parameters. If we wanted this for PV guests, the model wouldn't fit at all. Plus what I'm not understanding (perhaps because most of the discussion around this seemed to be ARM-specific, and hence I've skipped reading through it) is - why is this UART emulation needed in the first place? We've never had a need for such on x86, afaia. Linaro has published a set of guidelines to guarantee a same guest image can run on multiple hypervisors (see [1]). This specification mandates the presence of a SBSA-compliant UART. I just realized the cover letter does not explain why we need to emulate a PL011 on ARM. Bhupinder can you detail it on the next version? Furthermore - how does this scale? I.e. what if other devices pop up wanting to be emulated? Or multiple UARTs? I think you can appreciate that using QEMU for emulating an UART is quite overkill. Also, we are expecting more people to look at providing mediation (such as Firmware access) within Xen as they are concerned about performance and certification. An idea to ensure the integrity of Xen would be to run this code at a lower level. Cheers, [1] https://www.linaro.org/app/resources/WhitePaper/VMSystemSpecificationForARM-v2.0.pdf -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/5] x86/ioapic: add prototype for apic_gsi_base to io_apic.h
>>> On 23.02.17 at 12:52, wrote: > So that the function can be called from other files without adding prototypes > to each of them. If you plan to add more callers, this is the time to name the function properly (an apic_ prefix rather suggests the LAPIC) and to convert its parameter to unsigned int. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/emul: Correct the decoding of mov to/from cr/dr
On 06/03/17 13:10, Jan Beulich wrote:
On 06.03.17 at 11:30, wrote:
>> The mov to/from cr/dr behave as if they were encoded with Mod = 3. When
>> encoded with Mod != 3, no displacement or SIB bytes are fetched.
> Would mind letting us know how you became aware of this oddity?
> It's clearly both unexpected
Most definitely.
> and undocumented,
Actually, the contrary. It is explicitly called out in both Intel and
AMDs instruction manual, which is why I noticed it.
I have confirmed that hardware doesn't fetch disp or SIB bytes on my
oldest and newest Intel and AMD boxes.
> and I wonder whether there are any other opcodes behaving the same.
Not that I have spotted.
> In any even I think we want to make at least an attempt at having HW
> vendors confirm this (and to address the other-opcodes concern),
> so I'm extending the Cc list.
>
>> --- a/tools/tests/x86_emulator/test_x86_emulator.c
>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
>> @@ -894,6 +894,27 @@ int main(int argc, char **argv)
>> }
>> printf("okay\n");
>>
>> +printf("%-40s", "Testing mov %%cr4,%%esi (bad ModRM)...");
>> +/*
>> + * Mod = 1, Reg = 4, R/M = 6 would normally encode a memory reference of
>> + * disp8(%esi), but mov to/from cr/dr are special and behave as if they
>> + * were encoded with Mod == 3.
>> + */
>> +instr[0] = 0x0f; instr[1] = 0x20, instr[2] = 0146;
> I can guess why you've done it this way, but I'd prefer if we didn't
> start using octal numbers. I for one am very used to reading ModRM
> bytes in their hex representation.
The argument ought to be for which representation is easier. I'd accept
an argument for consistency with the rest of the code, but are you
seriously saying you think that hex is easier to read/manipulate than
octal for ModRM/SIB bytes?
>
>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>> @@ -2086,7 +2086,8 @@ x86_decode_twobyte(
>> }
>> /* fall through */
>> case 0x21: case 0x23: /* mov to/from dr */
>> -generate_exception_if(lock_prefix || ea.type != OP_REG, EXC_UD);
>> +ASSERT(ea.type == OP_REG); /* Early operand adjustment ensures
>> this. */
>> +generate_exception_if(lock_prefix, EXC_UD);
>> op_bytes = mode_64bit() ? 8 : 4;
>> break;
>>
>> @@ -2427,6 +2428,23 @@ x86_decode(
>> break;
>> }
>> }
>> +else if ( ext == ext_0f )
>> +{
>> +switch ( b )
>> +{
>> +case 0x20: /* mov cr,reg */
>> +case 0x21: /* mov dr,reg */
>> +case 0x22: /* mov reg,cr */
>> +case 0x23: /* mov reg,dr */
>> +/*
>> + * Mov to/from cr/dr ignore the encoding of Mod, and behave
>> as
>> + * if they were encoded as reg/reg instructions. No futher
>> + * disp/SIB bytes are fetched.
>> + */
>> +modrm_mod = 3;
>> +break;
>> +}
>> +}
> Just like done by
> https://lists.xenproject.org/archives/html/xen-devel/2017-03/msg00588.html
> this calls for the outer if/else to be morphed into a switch().
I did that first, but it was rather complicated to read. I can switch
back to that if you'd prefer.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen: Allow a default compiled-in command line using Kconfig
2017-03-06 19:42 GMT+08:00 Jan Beulich : On 06.03.17 at 12:38, wrote: > >> >> On 06/03/17 11:32, Jan Beulich wrote: >> On 06.03.17 at 12:15, wrote: On 06/03/17 11:01, Zhongze Liu wrote: > Added 3 new config entries in common/Kconfig: > CMDLINE_BOOL, CMDLINE and CMDLINE_OVERRIDE > > These 3 entries enable an embedded command line > to be compiled in the hypervisor. > > If CMDLINE_BOOL is set to y, both arm and x86 startup routines > will combine the compiled-in command line and the boot loader command line > together to form a complete command line before calling cmdline_parse(). > And if CMDLINE_OVERRIDE is set to y, boot loader command line > will be ignored. This behavior seems a little odd to me. Why would you want to append the two command line? And more importantly if the same option is passed twice, how will this behave? For example the compile command line could contain iommu=off, but the bootloader is passing iommu=on. What would be the final state of the IOMMU? This one is quite simple, but you could have options which set flags. So if you want to override it, the new option would have to unset it. >>> >>> I don't understand this concern - overriding an earlier option by a >>> later one is (supposed to be) well defined, and we even suggest this >>> to people when wanting to try out options without having to edit >>> bootloader (or whatever) config files. >> >> My concern is not about the override itself but about appending two >> commandlines option without any explanation whether the built-in >> commandline will be parsed first. > > Oh, agreed - as Andrew has also pointed out, the ordering (and hence > override behavior) needs to be clearly spelled out here. > > Jan > Yes, the expression here is indeed quite ambiguous. I'll try to clarify the ordering of the two commandlines and override behavior. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] build/clang: remove the address-of-packed-member warning
On 06/03/17 12:31, Roger Pau Monne wrote: > Clang 4.0 has added a new check that triggers when taking the address of a > field of a packed structure. I've explained our situation to the llvm/clang > folks, but nobody seem to care: > > https://reviews.llvm.org/D20561 > > So simply disable that check if present. > > Signed-off-by: Roger Pau Monné In this case, I'd prefer that we drop the unnecessary packed on segment_attributes and segment_register, perhaps adding some BUILD_BUG_ON()'s in the SVM code to check that the structure still matches hardwares representation. Or were there other structures which ended up in a similar situation? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 02/11] xen/arm: vpl011: Add new hvm params in Xen for ring buffer/event setup
>>> On 06.03.17 at 14:21, wrote: > On 06/03/17 12:41, Jan Beulich wrote: >> Furthermore - how does this scale? I.e. what if other devices pop >> up wanting to be emulated? Or multiple UARTs? > > I think you can appreciate that using QEMU for emulating an UART is > quite overkill. Sure, but this doesn't answer my questions. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [xen-unstable test] 106435: regressions - FAIL
> -Original Message- > From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of > Paul Durrant > Sent: 06 March 2017 08:28 > To: Andrew Cooper > Cc: xen-de...@lists.xensource.com; osstest service owner ad...@xenproject.org>; Jan Beulich > Subject: Re: [Xen-devel] [xen-unstable test] 106435: regressions - FAIL > > > -Original Message- > > From: Andrew Cooper [mailto:am...@hermes.cam.ac.uk] On Behalf Of > > Andrew Cooper > > Sent: 04 March 2017 15:45 > > To: Paul Durrant > > Cc: osstest service owner ; xen- > > de...@lists.xensource.com; Jan Beulich > > Subject: Re: [Xen-devel] [xen-unstable test] 106435: regressions - FAIL > > > > On 04/03/2017 15:19, osstest service owner wrote: > > > flight 106435 xen-unstable real [real] > > > http://logs.test-lab.xenproject.org/osstest/logs/106435/ > > > > > > Regressions :-( > > > > > > Tests which did not succeed and are blocking, > > > including tests which could not be run: > > > test-amd64-amd64-xl-qemuu-win7-amd64 9 windows-install fail REGR. > vs. > > 106412 > > > test-armhf-armhf-libvirt-raw 9 debian-di-installfail REGR. vs. > > > 106412 > > > > From > > http://logs.test-lab.xenproject.org/osstest/logs/106435/test-amd64- > amd64- > > xl-qemuu-win7-amd64/9.ts-windows-install.log > > > > Mar 4 12:49:08.069641 (d3) Booting from DVD/CD... > > Mar 4 12:49:08.069670 (d3) Booting from :7c00 > > Mar 4 12:49:08.069697 (XEN) stdvga.c:178:d3v0 leaving stdvga mode > > Mar 4 12:49:13.045676 (XEN) d3: VIRIDIAN GUEST_OS_ID: vendor: 1 os: 4 > > major: 6 minor: 1 sp: 0 build: 1db0 > > Mar 4 12:49:41.461683 (XEN) d3: VIRIDIAN HYPERCALL: enabled: 1 pfn: 3 > > Mar 4 12:49:41.461731 (XEN) d3v0: VIRIDIAN APIC_ASSIST: enabled: 1 pfn: > > 3fffe > > Mar 4 12:49:41.469596 (XEN) domain_crash called from viridian.c:306 > > Mar 4 12:49:41.477595 (XEN) Domain 3 (vcpu#0) crashed on cpu#1: > > Mar 4 12:49:41.477633 (XEN) [ Xen-4.9-unstable x86_64 debug=y Not > > tainted ] > > Mar 4 12:49:41.485589 (XEN) CPU:1 > > Mar 4 12:49:41.485620 (XEN) RIP:0010:[] > > Mar 4 12:49:41.485650 (XEN) RFLAGS: 0286 CONTEXT: hvm > > guest (d3v0) > > Mar 4 12:49:41.493591 (XEN) rax: rbx: f800027ede80 > > rcx: 0001 > > Mar 4 12:49:41.501560 (XEN) rdx: rsi: fa8001291040 > > rdi: f800027fbc40 > > Mar 4 12:49:41.509588 (XEN) rbp: 0080 rsp: f880009b0d80 > > r8: > > Mar 4 12:49:41.517584 (XEN) r9: f800027ede80 r10: fa8001291040 > > r11: f800027ede90 > > Mar 4 12:49:41.517624 (XEN) r12: f88129c0 r13: f800028afbe0 > > r14: fa800122db30 > > Mar 4 12:49:41.525586 (XEN) r15: f8b96080 cr0: 80050031 > > cr4: 06b8 > > Mar 4 12:49:41.533582 (XEN) cr3: 00187000 cr2: > > > Mar 4 12:49:41.541578 (XEN) ds: 002b es: 002b fs: 0053 gs: 002b > > ss: 0018 > > cs: 0010 > > > > > > Looks like this intermittent bug is still biting. Should we kill > > VIRIDIAN APIC_ASSIST until we have got to the bottom of it? > > Alternatively, add some printk() to actually give us useful information > > when it does go wrong. > > I actually think this may be a bug in Windows 7. I normally run with this > enlightenment on and have seen this very occasionally and only with > Windows 7. > > I think it would probably be useful to stash info about the last interrupt > injected into the guest and then reduce the domain_crash() to a gprintk() > dumping that info plus info about the interrupt that's about to be injected. > I'll > post a patch. Actually I think I may have figured this out... If a buggy version of Windows enables APIC assist (by setting up the shared page) but then performs an EOI without checking/clearing the bit in the page this will result in a domain_crash(). The reasoning is as follows: - Performing the EOI will clear the vector from the ISR but will not clear the pending assist information. - The next call to vlapic_has_pending_irq() will also not clear the pending assist information (because the assist bit is still set), but it will return a valid vector. - The subsequent call to vlapic_ack_pending_irq() will then call viridian_start_apic_assist() which will invoke domain_crash() because it detects that there is a pending assist. The OS is at fault because, by setting up the APIC assist page, it is guaranteeing to the hypervisor this it will check/clear the bit. But it's easy enough to work around the bug by simply aborting any pending APIC assist in the EOI handler. I'll test a patch that does this and post later. Paul > > Paul > > > > > ~Andrew > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/emul: Correct the decoding of mov to/from cr/dr
>>> On 06.03.17 at 14:30, wrote:
> On 06/03/17 13:10, Jan Beulich wrote:
> On 06.03.17 at 11:30, wrote:
>>> The mov to/from cr/dr behave as if they were encoded with Mod = 3. When
>>> encoded with Mod != 3, no displacement or SIB bytes are fetched.
>> Would mind letting us know how you became aware of this oddity?
>> It's clearly both unexpected
>
> Most definitely.
>
>> and undocumented,
>
> Actually, the contrary. It is explicitly called out in both Intel and
> AMDs instruction manual, which is why I noticed it.
Ah, well hidden in the middle of text. Using standard opcode
representation for non-standard encodings is far from helpful -
that's what I look at mainly (and the text becomes important
only when there _are_ oddities there).
> I have confirmed that hardware doesn't fetch disp or SIB bytes on my
> oldest and newest Intel and AMD boxes.
So did I.
>> and I wonder whether there are any other opcodes behaving the same.
>
> Not that I have spotted.
Neither me, but there must be something in the decoder special
casing those insns, and it's easily imaginable for such special cases
to also apply elsewhere.
>>> --- a/tools/tests/x86_emulator/test_x86_emulator.c
>>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
>>> @@ -894,6 +894,27 @@ int main(int argc, char **argv)
>>> }
>>> printf("okay\n");
>>>
>>> +printf("%-40s", "Testing mov %%cr4,%%esi (bad ModRM)...");
>>> +/*
>>> + * Mod = 1, Reg = 4, R/M = 6 would normally encode a memory reference
>>> of
>>> + * disp8(%esi), but mov to/from cr/dr are special and behave as if they
>>> + * were encoded with Mod == 3.
>>> + */
>>> +instr[0] = 0x0f; instr[1] = 0x20, instr[2] = 0146;
>> I can guess why you've done it this way, but I'd prefer if we didn't
>> start using octal numbers. I for one am very used to reading ModRM
>> bytes in their hex representation.
>
> The argument ought to be for which representation is easier. I'd accept
> an argument for consistency with the rest of the code, but are you
> seriously saying you think that hex is easier to read/manipulate than
> octal for ModRM/SIB bytes?
Yes, and very seriously in fact. Nevertheless I admit that this is a
matter of what one is used to. The only alternative to hex I'd
freely accept would be to use bit fields to fill the pieces individually.
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -2086,7 +2086,8 @@ x86_decode_twobyte(
>>> }
>>> /* fall through */
>>> case 0x21: case 0x23: /* mov to/from dr */
>>> -generate_exception_if(lock_prefix || ea.type != OP_REG, EXC_UD);
>>> +ASSERT(ea.type == OP_REG); /* Early operand adjustment ensures
>>> this. */
>>> +generate_exception_if(lock_prefix, EXC_UD);
>>> op_bytes = mode_64bit() ? 8 : 4;
>>> break;
>>>
>>> @@ -2427,6 +2428,23 @@ x86_decode(
>>> break;
>>> }
>>> }
>>> +else if ( ext == ext_0f )
>>> +{
>>> +switch ( b )
>>> +{
>>> +case 0x20: /* mov cr,reg */
>>> +case 0x21: /* mov dr,reg */
>>> +case 0x22: /* mov reg,cr */
>>> +case 0x23: /* mov reg,dr */
>>> +/*
>>> + * Mov to/from cr/dr ignore the encoding of Mod, and
>>> behave as
>>> + * if they were encoded as reg/reg instructions. No futher
>>> + * disp/SIB bytes are fetched.
>>> + */
>>> +modrm_mod = 3;
>>> +break;
>>> +}
>>> +}
>> Just like done by
>> https://lists.xenproject.org/archives/html/xen-devel/2017-03/msg00588.html
>> this calls for the outer if/else to be morphed into a switch().
>
> I did that first, but it was rather complicated to read. I can switch
> back to that if you'd prefer.
Well, if you don't here then that other patch will in any case.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH net v2] xen-netback: fix race condition on XenBus disconnect
On 06/03/17 08:58, Paul Durrant wrote:
>> -Original Message-
>> From: Igor Druzhinin [mailto:igor.druzhi...@citrix.com]
>> Sent: 03 March 2017 20:23
>> To: net...@vger.kernel.org; xen-de...@lists.xenproject.org
>> Cc: Paul Durrant ; jgr...@suse.com; Wei Liu
>> ; Igor Druzhinin
>> Subject: [PATCH net v2] xen-netback: fix race condition on XenBus
>> disconnect
>>
>> In some cases during XenBus disconnect event handling and subsequent
>> queue resource release there may be some TX handlers active on
>> other processors. Use RCU in order to synchronize with them.
>>
>> Signed-off-by: Igor Druzhinin
>> ---
>> v2:
>> * Add protection for xenvif_get_ethtool_stats
>> * Additional comments and fixes
>> ---
>> drivers/net/xen-netback/interface.c | 29 ++---
>> drivers/net/xen-netback/netback.c | 2 +-
>> drivers/net/xen-netback/xenbus.c| 20 ++--
>> 3 files changed, 33 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-
>> netback/interface.c
>> index a2d32676..266b7cd 100644
>> --- a/drivers/net/xen-netback/interface.c
>> +++ b/drivers/net/xen-netback/interface.c
>> @@ -164,13 +164,17 @@ static int xenvif_start_xmit(struct sk_buff *skb,
>> struct net_device *dev)
>> {
>> struct xenvif *vif = netdev_priv(dev);
>> struct xenvif_queue *queue = NULL;
>> -unsigned int num_queues = vif->num_queues;
>> +unsigned int num_queues;
>> u16 index;
>> struct xenvif_rx_cb *cb;
>>
>> BUG_ON(skb->dev != dev);
>>
>> -/* Drop the packet if queues are not set up */
>> +/* Drop the packet if queues are not set up.
>> + * This handler should be called inside an RCU read section
>> + * so we don't need to enter it here explicitly.
>> + */
>> +num_queues = rcu_dereference(vif)->num_queues;
>> if (num_queues < 1)
>> goto drop;
>>
>> @@ -221,18 +225,21 @@ static struct net_device_stats
>> *xenvif_get_stats(struct net_device *dev)
>> {
>> struct xenvif *vif = netdev_priv(dev);
>> struct xenvif_queue *queue = NULL;
>> +unsigned int num_queues;
>> u64 rx_bytes = 0;
>> u64 rx_packets = 0;
>> u64 tx_bytes = 0;
>> u64 tx_packets = 0;
>> unsigned int index;
>>
>> -spin_lock(&vif->lock);
>> -if (vif->queues == NULL)
>> +rcu_read_lock();
>> +
>> +num_queues = rcu_dereference(vif)->num_queues;
>> +if (num_queues < 1)
>> goto out;
>
> Is this if clause worth it? All it does is jump over the for loop, which
> would not be executed anyway, since the initial test (0 < 0) would fail.
Probably not needed here, but it does make it consistent with other
similar checks across the file. Just looks more descriptive.
>
>>
>> /* Aggregate tx and rx stats from each queue */
>> -for (index = 0; index < vif->num_queues; ++index) {
>> +for (index = 0; index < num_queues; ++index) {
>> queue = &vif->queues[index];
>> rx_bytes += queue->stats.rx_bytes;
>> rx_packets += queue->stats.rx_packets;
>> @@ -241,7 +248,7 @@ static struct net_device_stats
>> *xenvif_get_stats(struct net_device *dev)
>> }
>>
>> out:
>> -spin_unlock(&vif->lock);
>> +rcu_read_unlock();
>>
>> vif->dev->stats.rx_bytes = rx_bytes;
>> vif->dev->stats.rx_packets = rx_packets;
>> @@ -377,10 +384,16 @@ static void xenvif_get_ethtool_stats(struct
>> net_device *dev,
>> struct ethtool_stats *stats, u64 * data)
>> {
>> struct xenvif *vif = netdev_priv(dev);
>> -unsigned int num_queues = vif->num_queues;
>> +unsigned int num_queues;
>> int i;
>> unsigned int queue_index;
>>
>> +rcu_read_lock();
>> +
>> +num_queues = rcu_dereference(vif)->num_queues;
>> +if (num_queues < 1)
>> +goto out;
>> +
>
> You have introduced a semantic change with the above if clause. The
> xenvif_stats array was previously zeroed if num_queues < 1. It appears that
> ethtool does actually allocate a zeroed array to pass in here, but I wonder
> whether it is still safer to have this function zero it anyway.
Agree. Should at least zero out data array before exiting.
>
>> for (i = 0; i < ARRAY_SIZE(xenvif_stats); i++) {
>> unsigned long accum = 0;
>> for (queue_index = 0; queue_index < num_queues;
>> ++queue_index) {
>> @@ -389,6 +402,8 @@ static void xenvif_get_ethtool_stats(struct
>> net_device *dev,
>> }
>> data[i] = accum;
>> }
>> +out:
>> +rcu_read_unlock();
>> }
>>
>> static void xenvif_get_strings(struct net_device *dev, u32 stringset, u8 *
>> data)
>> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-
>> netback/netback.c
>> index f9bcf4a..62fa74d 100644
>> --- a/drivers/net/xen-netback/netback.c
>> +++ b/drivers/net/xen-netback/netback.c
>> @@ -214,7 +214,7 @@ static void xenvif_fatal_tx_err(struct xenvif *vif)
>> net
Re: [Xen-devel] [PATCH 1/2] build/clang: remove the address-of-packed-member warning
>>> On 06.03.17 at 13:31, wrote: > --- a/Config.mk > +++ b/Config.mk > @@ -216,6 +216,7 @@ $(call > cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement) > $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement) > $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable) > $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs) > +$(call cc-option-add,CFLAGS,CC,-Wno-address-of-packed-member) Actually, having thought some more about this, the warning should be suppressed only for x86 imo. ARM wants aligned accesses after all. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/md/md.c | 6 +++---
drivers/md/md.h | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 985374f..94c8ebf 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -449,7 +449,7 @@ EXPORT_SYMBOL(md_unplug);
static inline struct mddev *mddev_get(struct mddev *mddev)
{
- atomic_inc(&mddev->active);
+ refcount_inc(&mddev->active);
return mddev;
}
@@ -459,7 +459,7 @@ static void mddev_put(struct mddev *mddev)
{
struct bio_set *bs = NULL;
- if (!atomic_dec_and_lock(&mddev->active, &all_mddevs_lock))
+ if (!refcount_dec_and_lock(&mddev->active, &all_mddevs_lock))
return;
if (!mddev->raid_disks && list_empty(&mddev->disks) &&
mddev->ctime == 0 && !mddev->hold_active) {
@@ -495,7 +495,7 @@ void mddev_init(struct mddev *mddev)
INIT_LIST_HEAD(&mddev->all_mddevs);
setup_timer(&mddev->safemode_timer, md_safemode_timeout,
(unsigned long) mddev);
- atomic_set(&mddev->active, 1);
+ refcount_set(&mddev->active, 1);
atomic_set(&mddev->openers, 0);
atomic_set(&mddev->active_io, 0);
spin_lock_init(&mddev->lock);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index b8859cb..4811663 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -360,7 +361,7 @@ struct mddev {
*/
struct mutexopen_mutex;
struct mutexreconfig_mutex;
- atomic_tactive; /* general refcount */
+ refcount_t active; /* general refcount */
atomic_topeners;/* number of active
opens */
int changed;/* True if we might
need to
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/md/raid5-cache.c | 8 +++---
drivers/md/raid5.c | 66
drivers/md/raid5.h | 3 ++-
3 files changed, 39 insertions(+), 38 deletions(-)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 3f307be..6c05e12 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -979,7 +979,7 @@ int r5l_write_stripe(struct r5l_log *log, struct
stripe_head *sh)
* don't delay.
*/
clear_bit(STRIPE_DELAYED, &sh->state);
- atomic_inc(&sh->count);
+ refcount_inc(&sh->count);
mutex_lock(&log->io_mutex);
/* meta + data */
@@ -1321,7 +1321,7 @@ static void r5c_flush_stripe(struct r5conf *conf, struct
stripe_head *sh)
assert_spin_locked(&conf->device_lock);
list_del_init(&sh->lru);
- atomic_inc(&sh->count);
+ refcount_inc(&sh->count);
set_bit(STRIPE_HANDLE, &sh->state);
atomic_inc(&conf->active_stripes);
@@ -1424,7 +1424,7 @@ static void r5c_do_reclaim(struct r5conf *conf)
*/
if (!list_empty(&sh->lru) &&
!test_bit(STRIPE_HANDLE, &sh->state) &&
- atomic_read(&sh->count) == 0) {
+ refcount_read(&sh->count) == 0) {
r5c_flush_stripe(conf, sh);
if (count++ >= R5C_RECLAIM_STRIPE_GROUP)
break;
@@ -2650,7 +2650,7 @@ r5c_cache_data(struct r5l_log *log, struct stripe_head
*sh,
* don't delay.
*/
clear_bit(STRIPE_DELAYED, &sh->state);
- atomic_inc(&sh->count);
+ refcount_inc(&sh->count);
mutex_lock(&log->io_mutex);
/* meta + data */
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 2ce23b0..30c96a8 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -296,7 +296,7 @@ static void do_release_stripe(struct r5conf *conf, struct
stripe_head *sh,
static void __release_stripe(struct r5conf *conf, struct stripe_head *sh,
struct list_head *temp_inactive_list)
{
- if (atomic_dec_and_test(&sh->count))
+ if (refcount_dec_and_test(&sh->count))
do_release_stripe(conf, sh, temp_inactive_list);
}
@@ -388,7 +388,7 @@ void raid5_release_stripe(struct stripe_head *sh)
/* Avoid release_list until the last reference.
*/
- if (atomic_add_unless(&sh->count, -1, 1))
+ if (refcount_dec_not_one(&sh->count))
return;
if (unlikely(!conf->mddev->thread) ||
@@ -401,7 +401,7 @@ void raid5_release_stripe(struct stripe_head *sh)
slow_path:
local_irq_save(flags);
/* we are ok here if STRIPE_ON_RELEASE_LIST is set or not */
- if (atomic_dec_and_lock(&sh->count, &conf->device_lock)) {
+ if (refcount_dec_and_lock(&sh->count, &conf->device_lock)) {
INIT_LIST_HEAD(&list);
hash = sh->hash_lock_index;
do_release_stripe(conf, sh, &list);
@@ -491,7 +491,7 @@ static void init_stripe(struct stripe_head *sh, sector_t
sector, int previous)
struct r5conf *conf = sh->raid_conf;
int i, seq;
- BUG_ON(atomic_read(&sh->count) != 0);
+ BUG_ON(refcount_read(&sh->count) != 0);
BUG_ON(test_bit(STRIPE_HANDLE, &sh->state));
BUG_ON(stripe_operations_active(sh));
BUG_ON(sh->batch_head);
@@ -668,11 +668,11 @@ raid5_get_active_stripe(struct r5conf *conf, sector_t
sector,
&conf->cache_state);
} else {
init_stripe(sh, sector, previous);
- atomic_inc(&sh->count);
+ refcount_inc(&sh->count);
}
- } else if (!atomic_inc_not_zero(&sh->count)) {
+ } else if (!refcount_inc_not_zero(&sh->count)) {
spin_lock(&conf->device_lock);
- if (!atomic_read(&sh->count)) {
+ if (!refcount_read(&sh->count)) {
if (!test_bit(STRIPE_HANDLE, &sh->state))
atomic_inc(&conf->active_stripes);
BUG_ON(list_empty(&sh->lru) &&
@@ -688,7 +688,7 @@ raid5_get_active_stripe(struct r5conf *conf, sector_t
sector,
sh->group = NULL;
}
}
- atomic_inc(&sh->count);
+
[Xen-devel] [PATCH 16/29] drivers, media: convert vb2_vmalloc_buf.refcount from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/media/v4l2-core/videobuf2-vmalloc.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c
b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index 3f77814..f83253a 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -26,7 +27,7 @@ struct vb2_vmalloc_buf {
struct frame_vector *vec;
enum dma_data_direction dma_dir;
unsigned long size;
- atomic_trefcount;
+ refcount_t refcount;
struct vb2_vmarea_handler handler;
struct dma_buf *dbuf;
};
@@ -56,7 +57,7 @@ static void *vb2_vmalloc_alloc(struct device *dev, unsigned
long attrs,
return ERR_PTR(-ENOMEM);
}
- atomic_inc(&buf->refcount);
+ refcount_set(&buf->refcount, 1);
return buf;
}
@@ -64,7 +65,7 @@ static void vb2_vmalloc_put(void *buf_priv)
{
struct vb2_vmalloc_buf *buf = buf_priv;
- if (atomic_dec_and_test(&buf->refcount)) {
+ if (refcount_dec_and_test(&buf->refcount)) {
vfree(buf->vaddr);
kfree(buf);
}
@@ -161,7 +162,7 @@ static void *vb2_vmalloc_vaddr(void *buf_priv)
static unsigned int vb2_vmalloc_num_users(void *buf_priv)
{
struct vb2_vmalloc_buf *buf = buf_priv;
- return atomic_read(&buf->refcount);
+ return refcount_read(&buf->refcount);
}
static int vb2_vmalloc_mmap(void *buf_priv, struct vm_area_struct *vma)
@@ -368,7 +369,7 @@ static struct dma_buf *vb2_vmalloc_get_dmabuf(void
*buf_priv, unsigned long flag
return NULL;
/* dmabuf keeps reference to vb2 buffer */
- atomic_inc(&buf->refcount);
+ refcount_inc(&buf->refcount);
return dbuf;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 09/29] drivers, md: convert table_device.count from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/md/dm.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 9f37d7f..cba91c3 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#define DM_MSG_PREFIX "core"
@@ -96,7 +97,7 @@ struct dm_md_mempools {
struct table_device {
struct list_head list;
- atomic_t count;
+ refcount_t count;
struct dm_dev dm_dev;
};
@@ -680,10 +681,11 @@ int dm_get_table_device(struct mapped_device *md, dev_t
dev, fmode_t mode,
format_dev_t(td->dm_dev.name, dev);
- atomic_set(&td->count, 0);
+ refcount_set(&td->count, 1);
list_add(&td->list, &md->table_devices);
+ } else {
+ refcount_inc(&td->count);
}
- atomic_inc(&td->count);
mutex_unlock(&md->table_devices_lock);
*result = &td->dm_dev;
@@ -696,7 +698,7 @@ void dm_put_table_device(struct mapped_device *md, struct
dm_dev *d)
struct table_device *td = container_of(d, struct table_device, dm_dev);
mutex_lock(&md->table_devices_lock);
- if (atomic_dec_and_test(&td->count)) {
+ if (refcount_dec_and_test(&td->count)) {
close_table_device(td, md);
list_del(&td->list);
kfree(td);
@@ -713,7 +715,7 @@ static void free_table_devices(struct list_head *devices)
struct table_device *td = list_entry(tmp, struct table_device,
list);
DMWARN("dm_destroy: %s still exists with %d references",
- td->dm_dev.name, atomic_read(&td->count));
+ td->dm_dev.name, refcount_read(&td->count));
kfree(td);
}
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 12/29] drivers, media: convert s2255_dev.num_channels from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/media/usb/s2255/s2255drv.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/media/usb/s2255/s2255drv.c
b/drivers/media/usb/s2255/s2255drv.c
index a9d4484..2b4b009 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -36,6 +36,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -256,7 +257,7 @@ struct s2255_vc {
struct s2255_dev {
struct s2255_vc vc[MAX_CHANNELS];
struct v4l2_device v4l2_dev;
- atomic_tnum_channels;
+ refcount_tnum_channels;
int frames;
struct mutexlock; /* channels[].vdev.lock */
struct mutexcmdlock; /* protects cmdbuf */
@@ -1581,11 +1582,11 @@ static void s2255_video_device_release(struct
video_device *vdev)
container_of(vdev, struct s2255_vc, vdev);
dprintk(dev, 4, "%s, chnls: %d\n", __func__,
- atomic_read(&dev->num_channels));
+ refcount_read(&dev->num_channels));
v4l2_ctrl_handler_free(&vc->hdl);
- if (atomic_dec_and_test(&dev->num_channels))
+ if (refcount_dec_and_test(&dev->num_channels))
s2255_destroy(dev);
return;
}
@@ -1688,7 +1689,7 @@ static int s2255_probe_v4l(struct s2255_dev *dev)
"failed to register video device!\n");
break;
}
- atomic_inc(&dev->num_channels);
+ refcount_set(&dev->num_channels, 1);
v4l2_info(&dev->v4l2_dev, "V4L2 device registered as %s\n",
video_device_node_name(&vc->vdev));
@@ -1696,11 +1697,11 @@ static int s2255_probe_v4l(struct s2255_dev *dev)
pr_info("Sensoray 2255 V4L driver Revision: %s\n",
S2255_VERSION);
/* if no channels registered, return error and probe will fail*/
- if (atomic_read(&dev->num_channels) == 0) {
+ if (refcount_read(&dev->num_channels) == 0) {
v4l2_device_unregister(&dev->v4l2_dev);
return ret;
}
- if (atomic_read(&dev->num_channels) != MAX_CHANNELS)
+ if (refcount_read(&dev->num_channels) != MAX_CHANNELS)
pr_warn("s2255: Not all channels available.\n");
return 0;
}
@@ -2248,7 +2249,7 @@ static int s2255_probe(struct usb_interface *interface,
goto errorFWDATA1;
}
- atomic_set(&dev->num_channels, 0);
+ refcount_set(&dev->num_channels, 0);
dev->pid = id->idProduct;
dev->fw_data = kzalloc(sizeof(struct s2255_fw), GFP_KERNEL);
if (!dev->fw_data)
@@ -2368,12 +2369,12 @@ static void s2255_disconnect(struct usb_interface
*interface)
{
struct s2255_dev *dev = to_s2255_dev(usb_get_intfdata(interface));
int i;
- int channels = atomic_read(&dev->num_channels);
+ int channels = refcount_read(&dev->num_channels);
mutex_lock(&dev->lock);
v4l2_device_disconnect(&dev->v4l2_dev);
mutex_unlock(&dev->lock);
/*see comments in the uvc_driver.c usb disconnect function */
- atomic_inc(&dev->num_channels);
+ refcount_inc(&dev->num_channels);
/* unregister each video device. */
for (i = 0; i < channels; i++)
video_unregister_device(&dev->vc[i].vdev);
@@ -2386,7 +2387,7 @@ static void s2255_disconnect(struct usb_interface
*interface)
dev->vc[i].vidstatus_ready = 1;
wake_up(&dev->vc[i].wait_vidstatus);
}
- if (atomic_dec_and_test(&dev->num_channels))
+ if (refcount_dec_and_test(&dev->num_channels))
s2255_destroy(dev);
dev_info(&interface->dev, "%s\n", __func__);
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 14/29] drivers, media: convert vb2_dc_buf.refcount from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/media/v4l2-core/videobuf2-dma-contig.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index fb6a177..d29a07f 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -12,6 +12,7 @@
#include
#include
+#include
#include
#include
#include
@@ -34,7 +35,7 @@ struct vb2_dc_buf {
/* MMAP related */
struct vb2_vmarea_handler handler;
- atomic_trefcount;
+ refcount_t refcount;
struct sg_table *sgt_base;
/* DMABUF related */
@@ -86,7 +87,7 @@ static unsigned int vb2_dc_num_users(void *buf_priv)
{
struct vb2_dc_buf *buf = buf_priv;
- return atomic_read(&buf->refcount);
+ return refcount_read(&buf->refcount);
}
static void vb2_dc_prepare(void *buf_priv)
@@ -122,7 +123,7 @@ static void vb2_dc_put(void *buf_priv)
{
struct vb2_dc_buf *buf = buf_priv;
- if (!atomic_dec_and_test(&buf->refcount))
+ if (!refcount_dec_and_test(&buf->refcount))
return;
if (buf->sgt_base) {
@@ -170,7 +171,7 @@ static void *vb2_dc_alloc(struct device *dev, unsigned long
attrs,
buf->handler.put = vb2_dc_put;
buf->handler.arg = buf;
- atomic_inc(&buf->refcount);
+ refcount_set(&buf->refcount, 1);
return buf;
}
@@ -407,7 +408,7 @@ static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv,
unsigned long flags)
return NULL;
/* dmabuf keeps reference to vb2 buffer */
- atomic_inc(&buf->refcount);
+ refcount_inc(&buf->refcount);
return dbuf;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 01/11] xen/arm: vpl011: Add pl011 uart emulation in Xen
On Sun, Mar 05, 2017 at 01:04:54AM +, Julien Grall wrote:
> Hi Konrad,
>
> On 03/03/2017 07:59 PM, Konrad Rzeszutek Wilk wrote:
> > > +}
> > > +
> > > +static int vpl011_data_avail(struct domain *d)
> > > +{
> > > +int rc=0;
> > > +unsigned long flags;
> > > +
> > > +struct console_interface *intf=(struct console_interface
> > > *)d->arch.vpl011.ring_buf;
> >
> > Can you have an macro for this?
> > > +
> > > +VPL011_LOCK(d, flags);
> >
> > Please don't. Just use normal spin_lock invocation.
>
> This is really a matter of taste. We've been using macro on the vgic
> emulation for the lock and I find it clearer and less error-prone. At least
> you are sure that all critical paths protected by the local will have IRQ
> disable.
OK. I am more of the other way, but then I can see the beaty of having
an macro that requires two parameters so there is no way to mess this
up.
Bhupinder, please ignore my request.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 03/29] drivers, char: convert vma_data.refcnt from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/char/mspec.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/char/mspec.c b/drivers/char/mspec.c
index a9c2fa3..7b75669 100644
--- a/drivers/char/mspec.c
+++ b/drivers/char/mspec.c
@@ -43,6 +43,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -89,7 +90,7 @@ static int is_sn2;
* protect in fork case where multiple tasks share the vma_data.
*/
struct vma_data {
- atomic_t refcnt;/* Number of vmas sharing the data. */
+ refcount_t refcnt; /* Number of vmas sharing the data. */
spinlock_t lock;/* Serialize access to this structure. */
int count; /* Number of pages allocated. */
enum mspec_page_type type; /* Type of pages allocated. */
@@ -144,7 +145,7 @@ mspec_open(struct vm_area_struct *vma)
struct vma_data *vdata;
vdata = vma->vm_private_data;
- atomic_inc(&vdata->refcnt);
+ refcount_inc(&vdata->refcnt);
}
/*
@@ -162,7 +163,7 @@ mspec_close(struct vm_area_struct *vma)
vdata = vma->vm_private_data;
- if (!atomic_dec_and_test(&vdata->refcnt))
+ if (!refcount_dec_and_test(&vdata->refcnt))
return;
last_index = (vdata->vm_end - vdata->vm_start) >> PAGE_SHIFT;
@@ -274,7 +275,7 @@ mspec_mmap(struct file *file, struct vm_area_struct *vma,
vdata->vm_end = vma->vm_end;
vdata->type = type;
spin_lock_init(&vdata->lock);
- atomic_set(&vdata->refcnt, 1);
+ refcount_set(&vdata->refcnt, 1);
vma->vm_private_data = vdata;
vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 15/29] drivers, media: convert vb2_dma_sg_buf.refcount from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/media/v4l2-core/videobuf2-dma-sg.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index ecff8f4..29fde1a 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -12,6 +12,7 @@
#include
#include
+#include
#include
#include
#include
@@ -46,7 +47,7 @@ struct vb2_dma_sg_buf {
struct sg_table *dma_sgt;
size_t size;
unsigned intnum_pages;
- atomic_trefcount;
+ refcount_t refcount;
struct vb2_vmarea_handler handler;
struct dma_buf_attachment *db_attach;
@@ -150,7 +151,7 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned
long dma_attrs,
buf->handler.put = vb2_dma_sg_put;
buf->handler.arg = buf;
- atomic_inc(&buf->refcount);
+ refcount_set(&buf->refcount, 1);
dprintk(1, "%s: Allocated buffer of %d pages\n",
__func__, buf->num_pages);
@@ -176,7 +177,7 @@ static void vb2_dma_sg_put(void *buf_priv)
struct sg_table *sgt = &buf->sg_table;
int i = buf->num_pages;
- if (atomic_dec_and_test(&buf->refcount)) {
+ if (refcount_dec_and_test(&buf->refcount)) {
dprintk(1, "%s: Freeing buffer of %d pages\n", __func__,
buf->num_pages);
dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
@@ -320,7 +321,7 @@ static unsigned int vb2_dma_sg_num_users(void *buf_priv)
{
struct vb2_dma_sg_buf *buf = buf_priv;
- return atomic_read(&buf->refcount);
+ return refcount_read(&buf->refcount);
}
static int vb2_dma_sg_mmap(void *buf_priv, struct vm_area_struct *vma)
@@ -530,7 +531,7 @@ static struct dma_buf *vb2_dma_sg_get_dmabuf(void
*buf_priv, unsigned long flags
return NULL;
/* dmabuf keeps reference to vb2 buffer */
- atomic_inc(&buf->refcount);
+ refcount_inc(&buf->refcount);
return dbuf;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 07/29] drivers, md: convert dm_dev_internal.count from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/md/dm-table.c | 6 +++---
drivers/md/dm.h | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 3ad16d9..d2e2741 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -416,15 +416,15 @@ int dm_get_device(struct dm_target *ti, const char *path,
fmode_t mode,
return r;
}
- atomic_set(&dd->count, 0);
+ refcount_set(&dd->count, 1);
list_add(&dd->list, &t->devices);
} else if (dd->dm_dev->mode != (mode | dd->dm_dev->mode)) {
r = upgrade_mode(dd, mode, t->md);
if (r)
return r;
+ refcount_inc(&dd->count);
}
- atomic_inc(&dd->count);
*result = dd->dm_dev;
return 0;
@@ -478,7 +478,7 @@ void dm_put_device(struct dm_target *ti, struct dm_dev *d)
dm_device_name(ti->table->md), d->name);
return;
}
- if (atomic_dec_and_test(&dd->count)) {
+ if (refcount_dec_and_test(&dd->count)) {
dm_put_table_device(ti->table->md, d);
list_del(&dd->list);
kfree(dd);
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index f298b01..63b8142 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include "dm-stats.h"
@@ -38,7 +39,7 @@
*/
struct dm_dev_internal {
struct list_head list;
- atomic_t count;
+ refcount_t count;
struct dm_dev *dm_dev;
};
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 02/29] drivers, firewire: convert fw_node.ref_count from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/firewire/core-topology.c | 2 +-
drivers/firewire/core.h | 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
index 0de8350..939d259 100644
--- a/drivers/firewire/core-topology.c
+++ b/drivers/firewire/core-topology.c
@@ -124,7 +124,7 @@ static struct fw_node *fw_node_create(u32 sid, int
port_count, int color)
node->initiated_reset = SELF_ID_PHY_INITIATOR(sid);
node->port_count = port_count;
- atomic_set(&node->ref_count, 1);
+ refcount_set(&node->ref_count, 1);
INIT_LIST_HEAD(&node->link);
return node;
diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h
index e1480ff6..c07962e 100644
--- a/drivers/firewire/core.h
+++ b/drivers/firewire/core.h
@@ -12,7 +12,7 @@
#include
#include
-#include
+#include
struct device;
struct fw_card;
@@ -184,7 +184,7 @@ struct fw_node {
* local node to this node. */
u8 max_depth:4; /* Maximum depth to any leaf node */
u8 max_hops:4; /* Max hops in this sub tree */
- atomic_t ref_count;
+ refcount_t ref_count;
/* For serializing node topology into a list. */
struct list_head link;
@@ -197,14 +197,14 @@ struct fw_node {
static inline struct fw_node *fw_node_get(struct fw_node *node)
{
- atomic_inc(&node->ref_count);
+ refcount_inc(&node->ref_count);
return node;
}
static inline void fw_node_put(struct fw_node *node)
{
- if (atomic_dec_and_test(&node->ref_count))
+ if (refcount_dec_and_test(&node->ref_count))
kfree(node);
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 05/29] drivers, md, bcache: convert cached_dev.count from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/md/bcache/bcache.h| 7 ---
drivers/md/bcache/super.c | 6 +++---
drivers/md/bcache/writeback.h | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
index c3ea03c..de2be28 100644
--- a/drivers/md/bcache/bcache.h
+++ b/drivers/md/bcache/bcache.h
@@ -184,6 +184,7 @@
#include
#include
#include
+#include
#include
#include
@@ -299,7 +300,7 @@ struct cached_dev {
struct semaphoresb_write_mutex;
/* Refcount on the cache set. Always nonzero when we're caching. */
- atomic_tcount;
+ refcount_t count;
struct work_struct detach;
/*
@@ -805,13 +806,13 @@ do {
\
static inline void cached_dev_put(struct cached_dev *dc)
{
- if (atomic_dec_and_test(&dc->count))
+ if (refcount_dec_and_test(&dc->count))
schedule_work(&dc->detach);
}
static inline bool cached_dev_get(struct cached_dev *dc)
{
- if (!atomic_inc_not_zero(&dc->count))
+ if (!refcount_inc_not_zero(&dc->count))
return false;
/* Paired with the mb in cached_dev_attach */
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 85e3f21..cc36ce4 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -891,7 +891,7 @@ static void cached_dev_detach_finish(struct work_struct *w)
closure_init_stack(&cl);
BUG_ON(!test_bit(BCACHE_DEV_DETACHING, &dc->disk.flags));
- BUG_ON(atomic_read(&dc->count));
+ BUG_ON(refcount_read(&dc->count));
mutex_lock(&bch_register_lock);
@@ -1018,7 +1018,7 @@ int bch_cached_dev_attach(struct cached_dev *dc, struct
cache_set *c)
* dc->c must be set before dc->count != 0 - paired with the mb in
* cached_dev_get()
*/
- atomic_set(&dc->count, 1);
+ refcount_set(&dc->count, 1);
/* Block writeback thread, but spawn it */
down_write(&dc->writeback_lock);
@@ -1030,7 +1030,7 @@ int bch_cached_dev_attach(struct cached_dev *dc, struct
cache_set *c)
if (BDEV_STATE(&dc->sb) == BDEV_STATE_DIRTY) {
bch_sectors_dirty_init(dc);
atomic_set(&dc->has_dirty, 1);
- atomic_inc(&dc->count);
+ refcount_inc(&dc->count);
bch_writeback_queue(dc);
}
diff --git a/drivers/md/bcache/writeback.h b/drivers/md/bcache/writeback.h
index 629bd1a..5bac1b0 100644
--- a/drivers/md/bcache/writeback.h
+++ b/drivers/md/bcache/writeback.h
@@ -70,7 +70,7 @@ static inline void bch_writeback_add(struct cached_dev *dc)
{
if (!atomic_read(&dc->has_dirty) &&
!atomic_xchg(&dc->has_dirty, 1)) {
- atomic_inc(&dc->count);
+ refcount_inc(&dc->count);
if (BDEV_STATE(&dc->sb) != BDEV_STATE_DIRTY) {
SET_BDEV_STATE(&dc->sb, BDEV_STATE_DIRTY);
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 00/29] drivers, mics refcount conversions
This series, for various different drivers, replaces atomic_t reference counters with the new refcount_t type and API (see include/linux/refcount.h). By doing this we prevent intentional or accidental underflows or overflows that can led to use-after-free vulnerabilities. The below patches are fully independent and can be cherry-picked separately*. Since we convert all kernel subsystems in the same fashion, resulting in about 300 patches, we have to group them for sending at least in some fashion to be manageable. Please excuse the long cc list. *with the exception of the media/vb2-related patches that depend on vb2_vmarea_handler.refcount conversions. Not run-time tested beyond booting and using kernel with refcount conversions for my daily work. If there are no objections to these patches, I think they can go via Greg's drivers tree, as he suggested before. Elena Reshetova (29): drivers, block: convert xen_blkif.refcnt from atomic_t to refcount_t drivers, firewire: convert fw_node.ref_count from atomic_t to refcount_t drivers, char: convert vma_data.refcnt from atomic_t to refcount_t drivers, connector: convert cn_callback_entry.refcnt from atomic_t to refcount_t drivers, md, bcache: convert cached_dev.count from atomic_t to refcount_t drivers, md: convert dm_cache_metadata.ref_count from atomic_t to refcount_t drivers, md: convert dm_dev_internal.count from atomic_t to refcount_t drivers, md: convert mddev.active from atomic_t to refcount_t drivers, md: convert table_device.count from atomic_t to refcount_t drivers, md: convert stripe_head.count from atomic_t to refcount_t drivers, media: convert cx88_core.refcount from atomic_t to refcount_t drivers, media: convert s2255_dev.num_channels from atomic_t to refcount_t drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t drivers, media: convert vb2_dc_buf.refcount from atomic_t to refcount_t drivers, media: convert vb2_dma_sg_buf.refcount from atomic_t to refcount_t drivers, media: convert vb2_vmalloc_buf.refcount from atomic_t to refcount_t drivers, pci: convert hv_pci_dev.refs from atomic_t to refcount_t drivers, s390: convert urdev.ref_count from atomic_t to refcount_t drivers, s390: convert lcs_reply.refcnt from atomic_t to refcount_t drivers, s390: convert qeth_reply.refcnt from atomic_t to refcount_t drivers, s390: convert fc_fcp_pkt.ref_cnt from atomic_t to refcount_t drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t drivers: convert vme_user_vma_priv.refcnt from atomic_t to refcount_t drivers: convert iblock_req.pending from atomic_t to refcount_t drivers, usb: convert ffs_data.ref from atomic_t to refcount_t drivers, usb: convert dev_data.count from atomic_t to refcount_t drivers, usb: convert ep_data.count from atomic_t to refcount_t drivers: convert sbd_duart.map_guard from atomic_t to refcount_t drivers, xen: convert grant_map.users from atomic_t to refcount_t drivers/block/xen-blkback/common.h | 7 +-- drivers/block/xen-blkback/xenbus.c | 2 +- drivers/char/mspec.c | 9 ++-- drivers/connector/cn_queue.c | 4 +- drivers/connector/connector.c | 2 +- drivers/firewire/core-topology.c | 2 +- drivers/firewire/core.h| 8 ++-- drivers/md/bcache/bcache.h | 7 +-- drivers/md/bcache/super.c | 6 +-- drivers/md/bcache/writeback.h | 2 +- drivers/md/dm-cache-metadata.c | 9 ++-- drivers/md/dm-table.c | 6 +-- drivers/md/dm.c| 12 +++-- drivers/md/dm.h| 3 +- drivers/md/md.c| 6 +-- drivers/md/md.h| 3 +- drivers/md/raid5-cache.c | 8 ++-- drivers/md/raid5.c | 66 +- drivers/md/raid5.h | 3 +- drivers/media/pci/cx88/cx88-cards.c| 2 +- drivers/media/pci/cx88/cx88-core.c | 4 +- drivers/media/pci/cx88/cx88.h | 3 +- drivers/media/usb/s2255/s2255drv.c | 21 drivers/media/v4l2-core/videobuf2-dma-contig.c | 11 +++-- drivers/media/v4l2-core/videobuf2-dma-sg.c | 11 +++-- drivers/media/v4l2-core/videobuf2-memops.c | 6 +-- drivers/media/v4l2-core/videobuf2-vmalloc.c| 11 +++-- drivers/pci/host/pci-hyperv.c | 9 ++-- drivers/s390/char/vmur.c | 8 ++-- drivers/s390/char/vmur.h | 4 +- drivers/s390/net/lcs.c | 8 ++-- drivers/s390/net/lcs.h | 3 +- drivers/s390/net/qeth_core.h | 3 +- drivers/s390/net/qeth_core_main.c | 8 ++-- drivers/
[Xen-devel] [PATCH 17/29] drivers, pci: convert hv_pci_dev.refs from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/pci/host/pci-hyperv.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index cd114c6..870deed 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -56,6 +56,7 @@
#include
#include
#include
+#include
#include
/*
@@ -421,7 +422,7 @@ enum hv_pcidev_ref_reason {
struct hv_pci_dev {
/* List protected by pci_rescan_remove_lock */
struct list_head list_entry;
- atomic_t refs;
+ refcount_t refs;
enum hv_pcichild_state state;
struct pci_function_description desc;
bool reported_missing;
@@ -1254,13 +1255,13 @@ static void q_resource_requirements(void *context,
struct pci_response *resp,
static void get_pcichild(struct hv_pci_dev *hpdev,
enum hv_pcidev_ref_reason reason)
{
- atomic_inc(&hpdev->refs);
+ refcount_inc(&hpdev->refs);
}
static void put_pcichild(struct hv_pci_dev *hpdev,
enum hv_pcidev_ref_reason reason)
{
- if (atomic_dec_and_test(&hpdev->refs))
+ if (refcount_dec_and_test(&hpdev->refs))
kfree(hpdev);
}
@@ -1314,7 +1315,7 @@ static struct hv_pci_dev *new_pcichild_device(struct
hv_pcibus_device *hbus,
wait_for_completion(&comp_pkt.host_event);
hpdev->desc = *desc;
- get_pcichild(hpdev, hv_pcidev_ref_initial);
+ refcount_set(&hpdev->refs, 1);
get_pcichild(hpdev, hv_pcidev_ref_childlist);
spin_lock_irqsave(&hbus->device_list_lock, flags);
list_add_tail(&hpdev->list_entry, &hbus->children);
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 11/29] drivers, media: convert cx88_core.refcount from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/media/pci/cx88/cx88-cards.c | 2 +-
drivers/media/pci/cx88/cx88-core.c | 4 ++--
drivers/media/pci/cx88/cx88.h | 3 ++-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/media/pci/cx88/cx88-cards.c
b/drivers/media/pci/cx88/cx88-cards.c
index cdfbde2..7fc5f5f 100644
--- a/drivers/media/pci/cx88/cx88-cards.c
+++ b/drivers/media/pci/cx88/cx88-cards.c
@@ -3670,7 +3670,7 @@ struct cx88_core *cx88_core_create(struct pci_dev *pci,
int nr)
if (!core)
return NULL;
- atomic_inc(&core->refcount);
+ refcount_set(&core->refcount, 1);
core->pci_bus = pci->bus->number;
core->pci_slot = PCI_SLOT(pci->devfn);
core->pci_irqmask = PCI_INT_RISC_RD_BERRINT | PCI_INT_RISC_WR_BERRINT |
diff --git a/drivers/media/pci/cx88/cx88-core.c
b/drivers/media/pci/cx88/cx88-core.c
index 973a9cd4..8bfa5b7 100644
--- a/drivers/media/pci/cx88/cx88-core.c
+++ b/drivers/media/pci/cx88/cx88-core.c
@@ -1052,7 +1052,7 @@ struct cx88_core *cx88_core_get(struct pci_dev *pci)
mutex_unlock(&devlist);
return NULL;
}
- atomic_inc(&core->refcount);
+ refcount_inc(&core->refcount);
mutex_unlock(&devlist);
return core;
}
@@ -1073,7 +1073,7 @@ void cx88_core_put(struct cx88_core *core, struct pci_dev
*pci)
release_mem_region(pci_resource_start(pci, 0),
pci_resource_len(pci, 0));
- if (!atomic_dec_and_test(&core->refcount))
+ if (!refcount_dec_and_test(&core->refcount))
return;
mutex_lock(&devlist);
diff --git a/drivers/media/pci/cx88/cx88.h b/drivers/media/pci/cx88/cx88.h
index 115414c..16c1313 100644
--- a/drivers/media/pci/cx88/cx88.h
+++ b/drivers/media/pci/cx88/cx88.h
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
@@ -339,7 +340,7 @@ struct cx8802_dev;
struct cx88_core {
struct list_head devlist;
- atomic_t refcount;
+ refcount_t refcount;
/* board name */
intnr;
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 06/29] drivers, md: convert dm_cache_metadata.ref_count from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/md/dm-cache-metadata.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
index e4c2c1a..6d26e71 100644
--- a/drivers/md/dm-cache-metadata.c
+++ b/drivers/md/dm-cache-metadata.c
@@ -13,6 +13,7 @@
#include "persistent-data/dm-transaction-manager.h"
#include
+#include
/**/
@@ -102,7 +103,7 @@ struct cache_disk_superblock {
} __packed;
struct dm_cache_metadata {
- atomic_t ref_count;
+ refcount_t ref_count;
struct list_head list;
unsigned version;
@@ -756,7 +757,7 @@ static struct dm_cache_metadata *metadata_open(struct
block_device *bdev,
}
cmd->version = metadata_version;
- atomic_set(&cmd->ref_count, 1);
+ refcount_set(&cmd->ref_count, 1);
init_rwsem(&cmd->root_lock);
cmd->bdev = bdev;
cmd->data_block_size = data_block_size;
@@ -794,7 +795,7 @@ static struct dm_cache_metadata *lookup(struct block_device
*bdev)
list_for_each_entry(cmd, &table, list)
if (cmd->bdev == bdev) {
- atomic_inc(&cmd->ref_count);
+ refcount_inc(&cmd->ref_count);
return cmd;
}
@@ -865,7 +866,7 @@ struct dm_cache_metadata *dm_cache_metadata_open(struct
block_device *bdev,
void dm_cache_metadata_close(struct dm_cache_metadata *cmd)
{
- if (atomic_dec_and_test(&cmd->ref_count)) {
+ if (refcount_dec_and_test(&cmd->ref_count)) {
mutex_lock(&table_lock);
list_del(&cmd->list);
mutex_unlock(&table_lock);
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/media/v4l2-core/videobuf2-memops.c | 6 +++---
include/media/videobuf2-memops.h | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-memops.c
b/drivers/media/v4l2-core/videobuf2-memops.c
index 1cd322e..4bb8424 100644
--- a/drivers/media/v4l2-core/videobuf2-memops.c
+++ b/drivers/media/v4l2-core/videobuf2-memops.c
@@ -96,10 +96,10 @@ static void vb2_common_vm_open(struct vm_area_struct *vma)
struct vb2_vmarea_handler *h = vma->vm_private_data;
pr_debug("%s: %p, refcount: %d, vma: %08lx-%08lx\n",
- __func__, h, atomic_read(h->refcount), vma->vm_start,
+ __func__, h, refcount_read(h->refcount), vma->vm_start,
vma->vm_end);
- atomic_inc(h->refcount);
+ refcount_inc(h->refcount);
}
/**
@@ -114,7 +114,7 @@ static void vb2_common_vm_close(struct vm_area_struct *vma)
struct vb2_vmarea_handler *h = vma->vm_private_data;
pr_debug("%s: %p, refcount: %d, vma: %08lx-%08lx\n",
- __func__, h, atomic_read(h->refcount), vma->vm_start,
+ __func__, h, refcount_read(h->refcount), vma->vm_start,
vma->vm_end);
h->put(h->arg);
diff --git a/include/media/videobuf2-memops.h b/include/media/videobuf2-memops.h
index 36565c7a..a6ed091 100644
--- a/include/media/videobuf2-memops.h
+++ b/include/media/videobuf2-memops.h
@@ -16,6 +16,7 @@
#include
#include
+#include
/**
* struct vb2_vmarea_handler - common vma refcount tracking handler
@@ -25,7 +26,7 @@
* @arg: argument for @put callback
*/
struct vb2_vmarea_handler {
- atomic_t*refcount;
+ refcount_t *refcount;
void(*put)(void *arg);
void*arg;
};
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 18/29] drivers, s390: convert urdev.ref_count from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/s390/char/vmur.c | 8
drivers/s390/char/vmur.h | 4 +++-
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/s390/char/vmur.c b/drivers/s390/char/vmur.c
index 04aceb6..ced8151 100644
--- a/drivers/s390/char/vmur.c
+++ b/drivers/s390/char/vmur.c
@@ -110,7 +110,7 @@ static struct urdev *urdev_alloc(struct ccw_device *cdev)
mutex_init(&urd->io_mutex);
init_waitqueue_head(&urd->wait);
spin_lock_init(&urd->open_lock);
- atomic_set(&urd->ref_count, 1);
+ refcount_set(&urd->ref_count, 1);
urd->cdev = cdev;
get_device(&cdev->dev);
return urd;
@@ -126,7 +126,7 @@ static void urdev_free(struct urdev *urd)
static void urdev_get(struct urdev *urd)
{
- atomic_inc(&urd->ref_count);
+ refcount_inc(&urd->ref_count);
}
static struct urdev *urdev_get_from_cdev(struct ccw_device *cdev)
@@ -159,7 +159,7 @@ static struct urdev *urdev_get_from_devno(u16 devno)
static void urdev_put(struct urdev *urd)
{
- if (atomic_dec_and_test(&urd->ref_count))
+ if (refcount_dec_and_test(&urd->ref_count))
urdev_free(urd);
}
@@ -946,7 +946,7 @@ static int ur_set_offline_force(struct ccw_device *cdev,
int force)
rc = -EBUSY;
goto fail_urdev_put;
}
- if (!force && (atomic_read(&urd->ref_count) > 2)) {
+ if (!force && (refcount_read(&urd->ref_count) > 2)) {
/* There is still a user of urd (e.g. ur_open) */
TRACE("ur_set_offline: BUSY\n");
rc = -EBUSY;
diff --git a/drivers/s390/char/vmur.h b/drivers/s390/char/vmur.h
index fa320ad..35ea9d1 100644
--- a/drivers/s390/char/vmur.h
+++ b/drivers/s390/char/vmur.h
@@ -11,6 +11,8 @@
#ifndef _VMUR_H_
#define _VMUR_H_
+#include
+
#define DEV_CLASS_UR_I 0x20 /* diag210 unit record input device class */
#define DEV_CLASS_UR_O 0x10 /* diag210 unit record output device class */
/*
@@ -69,7 +71,7 @@ struct urdev {
size_t reclen; /* Record length for *write* CCWs */
int class; /* VM device class */
int io_request_rc; /* return code from I/O request */
- atomic_t ref_count; /* reference counter */
+ refcount_t ref_count; /* reference counter */
wait_queue_head_t wait; /* wait queue to serialize open */
int open_flag; /* "urdev is open" flag */
spinlock_t open_lock; /* serialize critical sections */
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 01/29] drivers, block: convert xen_blkif.refcnt from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/block/xen-blkback/common.h | 7 ---
drivers/block/xen-blkback/xenbus.c | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/block/xen-blkback/common.h
b/drivers/block/xen-blkback/common.h
index dea61f6..2ccfd62 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -333,7 +334,7 @@ struct xen_blkif {
struct xen_vbd vbd;
/* Back pointer to the backend_info. */
struct backend_info *be;
- atomic_trefcnt;
+ refcount_t refcnt;
/* for barrier (drain) requests */
struct completion drain_complete;
atomic_tdrain;
@@ -386,10 +387,10 @@ struct pending_req {
(_v)->bdev->bd_part->nr_sects : \
get_capacity((_v)->bdev->bd_disk))
-#define xen_blkif_get(_b) (atomic_inc(&(_b)->refcnt))
+#define xen_blkif_get(_b) (refcount_inc(&(_b)->refcnt))
#define xen_blkif_put(_b) \
do {\
- if (atomic_dec_and_test(&(_b)->refcnt)) \
+ if (refcount_dec_and_test(&(_b)->refcnt)) \
schedule_work(&(_b)->free_work);\
} while (0)
diff --git a/drivers/block/xen-blkback/xenbus.c
b/drivers/block/xen-blkback/xenbus.c
index 8fe61b5..9f89be3 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -176,7 +176,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
return ERR_PTR(-ENOMEM);
blkif->domid = domid;
- atomic_set(&blkif->refcnt, 1);
+ refcount_set(&blkif->refcnt, 1);
init_completion(&blkif->drain_complete);
INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 20/29] drivers, s390: convert qeth_reply.refcnt from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/s390/net/qeth_core.h | 3 ++-
drivers/s390/net/qeth_core_main.c | 8 +++-
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index e7addea..e2c81d21 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -641,7 +642,7 @@ struct qeth_reply {
int rc;
void *param;
struct qeth_card *card;
- atomic_t refcnt;
+ refcount_t refcnt;
};
diff --git a/drivers/s390/net/qeth_core_main.c
b/drivers/s390/net/qeth_core_main.c
index 315d8a2..a2bf13f 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -555,7 +555,7 @@ static struct qeth_reply *qeth_alloc_reply(struct qeth_card
*card)
reply = kzalloc(sizeof(struct qeth_reply), GFP_ATOMIC);
if (reply) {
- atomic_set(&reply->refcnt, 1);
+ refcount_set(&reply->refcnt, 1);
atomic_set(&reply->received, 0);
reply->card = card;
}
@@ -564,14 +564,12 @@ static struct qeth_reply *qeth_alloc_reply(struct
qeth_card *card)
static void qeth_get_reply(struct qeth_reply *reply)
{
- WARN_ON(atomic_read(&reply->refcnt) <= 0);
- atomic_inc(&reply->refcnt);
+ refcount_inc(&reply->refcnt);
}
static void qeth_put_reply(struct qeth_reply *reply)
{
- WARN_ON(atomic_read(&reply->refcnt) <= 0);
- if (atomic_dec_and_test(&reply->refcnt))
+ if (refcount_dec_and_test(&reply->refcnt))
kfree(reply);
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 19/29] drivers, s390: convert lcs_reply.refcnt from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/s390/net/lcs.c | 8 +++-
drivers/s390/net/lcs.h | 3 ++-
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/s390/net/lcs.c b/drivers/s390/net/lcs.c
index 211b31d..18dc787 100644
--- a/drivers/s390/net/lcs.c
+++ b/drivers/s390/net/lcs.c
@@ -774,15 +774,13 @@ lcs_get_lancmd(struct lcs_card *card, int count)
static void
lcs_get_reply(struct lcs_reply *reply)
{
- WARN_ON(atomic_read(&reply->refcnt) <= 0);
- atomic_inc(&reply->refcnt);
+ refcount_inc(&reply->refcnt);
}
static void
lcs_put_reply(struct lcs_reply *reply)
{
-WARN_ON(atomic_read(&reply->refcnt) <= 0);
-if (atomic_dec_and_test(&reply->refcnt)) {
+if (refcount_dec_and_test(&reply->refcnt)) {
kfree(reply);
}
@@ -798,7 +796,7 @@ lcs_alloc_reply(struct lcs_cmd *cmd)
reply = kzalloc(sizeof(struct lcs_reply), GFP_ATOMIC);
if (!reply)
return NULL;
- atomic_set(&reply->refcnt,1);
+ refcount_set(&reply->refcnt,1);
reply->sequence_no = cmd->sequence_no;
reply->received = 0;
reply->rc = 0;
diff --git a/drivers/s390/net/lcs.h b/drivers/s390/net/lcs.h
index 150fcb4..3802f4f 100644
--- a/drivers/s390/net/lcs.h
+++ b/drivers/s390/net/lcs.h
@@ -4,6 +4,7 @@
#include
#include
#include
+#include
#include
#define LCS_DBF_TEXT(level, name, text) \
@@ -270,7 +271,7 @@ struct lcs_buffer {
struct lcs_reply {
struct list_head list;
__u16 sequence_no;
- atomic_t refcnt;
+ refcount_t refcnt;
/* Callback for completion notification. */
void (*callback)(struct lcs_card *, struct lcs_cmd *);
wait_queue_head_t wait_q;
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 21/29] drivers, s390: convert fc_fcp_pkt.ref_cnt from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/scsi/libfc/fc_fcp.c | 6 +++---
include/scsi/libfc.h| 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 0e67621..a808e8e 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -154,7 +154,7 @@ static struct fc_fcp_pkt *fc_fcp_pkt_alloc(struct fc_lport
*lport, gfp_t gfp)
memset(fsp, 0, sizeof(*fsp));
fsp->lp = lport;
fsp->xfer_ddp = FC_XID_UNKNOWN;
- atomic_set(&fsp->ref_cnt, 1);
+ refcount_set(&fsp->ref_cnt, 1);
init_timer(&fsp->timer);
fsp->timer.data = (unsigned long)fsp;
INIT_LIST_HEAD(&fsp->list);
@@ -175,7 +175,7 @@ static struct fc_fcp_pkt *fc_fcp_pkt_alloc(struct fc_lport
*lport, gfp_t gfp)
*/
static void fc_fcp_pkt_release(struct fc_fcp_pkt *fsp)
{
- if (atomic_dec_and_test(&fsp->ref_cnt)) {
+ if (refcount_dec_and_test(&fsp->ref_cnt)) {
struct fc_fcp_internal *si = fc_get_scsi_internal(fsp->lp);
mempool_free(fsp, si->scsi_pkt_pool);
@@ -188,7 +188,7 @@ static void fc_fcp_pkt_release(struct fc_fcp_pkt *fsp)
*/
static void fc_fcp_pkt_hold(struct fc_fcp_pkt *fsp)
{
- atomic_inc(&fsp->ref_cnt);
+ refcount_inc(&fsp->ref_cnt);
}
/**
diff --git a/include/scsi/libfc.h b/include/scsi/libfc.h
index da5033d..2109844 100644
--- a/include/scsi/libfc.h
+++ b/include/scsi/libfc.h
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
@@ -321,7 +322,7 @@ struct fc_seq_els_data {
*/
struct fc_fcp_pkt {
spinlock_tscsi_pkt_lock;
- atomic_t ref_cnt;
+ refcount_tref_cnt;
/* SCSI command and data transfer information */
u32 data_len;
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/scsi/libiscsi.c| 8
drivers/scsi/qedi/qedi_iscsi.c | 2 +-
include/scsi/libiscsi.h| 3 ++-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 834d121..7eb1d2c 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -516,13 +516,13 @@ static void iscsi_free_task(struct iscsi_task *task)
void __iscsi_get_task(struct iscsi_task *task)
{
- atomic_inc(&task->refcount);
+ refcount_inc(&task->refcount);
}
EXPORT_SYMBOL_GPL(__iscsi_get_task);
void __iscsi_put_task(struct iscsi_task *task)
{
- if (atomic_dec_and_test(&task->refcount))
+ if (refcount_dec_and_test(&task->refcount))
iscsi_free_task(task);
}
EXPORT_SYMBOL_GPL(__iscsi_put_task);
@@ -744,7 +744,7 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct
iscsi_hdr *hdr,
* released by the lld when it has transmitted the task for
* pdus we do not expect a response for.
*/
- atomic_set(&task->refcount, 1);
+ refcount_set(&task->refcount, 1);
task->conn = conn;
task->sc = NULL;
INIT_LIST_HEAD(&task->running);
@@ -1616,7 +1616,7 @@ static inline struct iscsi_task *iscsi_alloc_task(struct
iscsi_conn *conn,
sc->SCp.phase = conn->session->age;
sc->SCp.ptr = (char *) task;
- atomic_set(&task->refcount, 1);
+ refcount_set(&task->refcount, 1);
task->state = ISCSI_TASK_PENDING;
task->conn = conn;
task->sc = sc;
diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
index b9f79d3..3895bd5 100644
--- a/drivers/scsi/qedi/qedi_iscsi.c
+++ b/drivers/scsi/qedi/qedi_iscsi.c
@@ -1372,7 +1372,7 @@ static void qedi_cleanup_task(struct iscsi_task *task)
{
if (!task->sc || task->state == ISCSI_TASK_PENDING) {
QEDI_INFO(NULL, QEDI_LOG_IO, "Returning ref_cnt=%d\n",
- atomic_read(&task->refcount));
+ refcount_read(&task->refcount));
return;
}
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index b0e275d..24d74b5 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -139,7 +140,7 @@ struct iscsi_task {
/* state set/tested under session->lock */
int state;
- atomic_trefcount;
+ refcount_t refcount;
struct list_headrunning;/* running cmd list */
void*dd_data; /* driver/transport data */
};
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 24/29] drivers: convert iblock_req.pending from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/target/target_core_iblock.c | 12 ++--
drivers/target/target_core_iblock.h | 3 ++-
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/target/target_core_iblock.c
b/drivers/target/target_core_iblock.c
index d316ed5..bb069eb 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -279,7 +279,7 @@ static void iblock_complete_cmd(struct se_cmd *cmd)
struct iblock_req *ibr = cmd->priv;
u8 status;
- if (!atomic_dec_and_test(&ibr->pending))
+ if (!refcount_dec_and_test(&ibr->pending))
return;
if (atomic_read(&ibr->ib_bio_err_cnt))
@@ -487,7 +487,7 @@ iblock_execute_write_same(struct se_cmd *cmd)
bio_list_init(&list);
bio_list_add(&list, bio);
- atomic_set(&ibr->pending, 1);
+ refcount_set(&ibr->pending, 1);
while (sectors) {
while (bio_add_page(bio, sg_page(sg), sg->length, sg->offset)
@@ -498,7 +498,7 @@ iblock_execute_write_same(struct se_cmd *cmd)
if (!bio)
goto fail_put_bios;
- atomic_inc(&ibr->pending);
+ refcount_inc(&ibr->pending);
bio_list_add(&list, bio);
}
@@ -706,7 +706,7 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist
*sgl, u32 sgl_nents,
cmd->priv = ibr;
if (!sgl_nents) {
- atomic_set(&ibr->pending, 1);
+ refcount_set(&ibr->pending, 1);
iblock_complete_cmd(cmd);
return 0;
}
@@ -719,7 +719,7 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist
*sgl, u32 sgl_nents,
bio_list_init(&list);
bio_list_add(&list, bio);
- atomic_set(&ibr->pending, 2);
+ refcount_set(&ibr->pending, 2);
bio_cnt = 1;
for_each_sg(sgl, sg, sgl_nents, i) {
@@ -740,7 +740,7 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist
*sgl, u32 sgl_nents,
if (!bio)
goto fail_put_bios;
- atomic_inc(&ibr->pending);
+ refcount_inc(&ibr->pending);
bio_list_add(&list, bio);
bio_cnt++;
}
diff --git a/drivers/target/target_core_iblock.h
b/drivers/target/target_core_iblock.h
index 718d3fc..f2a5797 100644
--- a/drivers/target/target_core_iblock.h
+++ b/drivers/target/target_core_iblock.h
@@ -2,6 +2,7 @@
#define TARGET_CORE_IBLOCK_H
#include
+#include
#include
#define IBLOCK_VERSION "4.0"
@@ -10,7 +11,7 @@
#define IBLOCK_LBA_SHIFT 9
struct iblock_req {
- atomic_t pending;
+ refcount_t pending;
atomic_t ib_bio_err_cnt;
} cacheline_aligned;
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 28/29] drivers: convert sbd_duart.map_guard from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/tty/serial/sb1250-duart.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/sb1250-duart.c
b/drivers/tty/serial/sb1250-duart.c
index 771f361..041625c 100644
--- a/drivers/tty/serial/sb1250-duart.c
+++ b/drivers/tty/serial/sb1250-duart.c
@@ -41,7 +41,7 @@
#include
#include
-#include
+#include
#include
#include
@@ -103,7 +103,7 @@ struct sbd_port {
struct sbd_duart {
struct sbd_port sport[2];
unsigned long mapctrl;
- atomic_tmap_guard;
+ refcount_t map_guard;
};
#define to_sport(uport) container_of(uport, struct sbd_port, port)
@@ -654,15 +654,13 @@ static void sbd_release_port(struct uart_port *uport)
{
struct sbd_port *sport = to_sport(uport);
struct sbd_duart *duart = sport->duart;
- int map_guard;
iounmap(sport->memctrl);
sport->memctrl = NULL;
iounmap(uport->membase);
uport->membase = NULL;
- map_guard = atomic_add_return(-1, &duart->map_guard);
- if (!map_guard)
+ if(refcount_dec_and_test(&duart->map_guard))
release_mem_region(duart->mapctrl, DUART_CHANREG_SPACING);
release_mem_region(uport->mapbase, DUART_CHANREG_SPACING);
}
@@ -698,7 +696,6 @@ static int sbd_request_port(struct uart_port *uport)
{
const char *err = KERN_ERR "sbd: Unable to reserve MMIO resource\n";
struct sbd_duart *duart = to_sport(uport)->duart;
- int map_guard;
int ret = 0;
if (!request_mem_region(uport->mapbase, DUART_CHANREG_SPACING,
@@ -706,11 +703,11 @@ static int sbd_request_port(struct uart_port *uport)
printk(err);
return -EBUSY;
}
- map_guard = atomic_add_return(1, &duart->map_guard);
- if (map_guard == 1) {
+ refcount_inc(&duart->map_guard);
+ if (refcount_read(&duart->map_guard) == 1) {
if (!request_mem_region(duart->mapctrl, DUART_CHANREG_SPACING,
"sb1250-duart")) {
- atomic_add(-1, &duart->map_guard);
+ refcount_dec(&duart->map_guard);
printk(err);
ret = -EBUSY;
}
@@ -718,8 +715,7 @@ static int sbd_request_port(struct uart_port *uport)
if (!ret) {
ret = sbd_map_port(uport);
if (ret) {
- map_guard = atomic_add_return(-1, &duart->map_guard);
- if (!map_guard)
+ if (refcount_dec_and_test(&duart->map_guard))
release_mem_region(duart->mapctrl,
DUART_CHANREG_SPACING);
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 25/29] drivers, usb: convert ffs_data.ref from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/usb/gadget/function/f_fs.c | 8
drivers/usb/gadget/function/u_fs.h | 3 ++-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index 87fccf6..3cdeb91 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1570,14 +1570,14 @@ static void ffs_data_get(struct ffs_data *ffs)
{
ENTER();
- atomic_inc(&ffs->ref);
+ refcount_inc(&ffs->ref);
}
static void ffs_data_opened(struct ffs_data *ffs)
{
ENTER();
- atomic_inc(&ffs->ref);
+ refcount_inc(&ffs->ref);
if (atomic_add_return(1, &ffs->opened) == 1 &&
ffs->state == FFS_DEACTIVATED) {
ffs->state = FFS_CLOSING;
@@ -1589,7 +1589,7 @@ static void ffs_data_put(struct ffs_data *ffs)
{
ENTER();
- if (unlikely(atomic_dec_and_test(&ffs->ref))) {
+ if (unlikely(refcount_dec_and_test(&ffs->ref))) {
pr_info("%s(): freeing\n", __func__);
ffs_data_clear(ffs);
BUG_ON(waitqueue_active(&ffs->ev.waitq) ||
@@ -1634,7 +1634,7 @@ static struct ffs_data *ffs_data_new(void)
ENTER();
- atomic_set(&ffs->ref, 1);
+ refcount_set(&ffs->ref, 1);
atomic_set(&ffs->opened, 0);
ffs->state = FFS_READ_DESCRIPTORS;
mutex_init(&ffs->mutex);
diff --git a/drivers/usb/gadget/function/u_fs.h
b/drivers/usb/gadget/function/u_fs.h
index 4b69694..abfca48 100644
--- a/drivers/usb/gadget/function/u_fs.h
+++ b/drivers/usb/gadget/function/u_fs.h
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#ifdef VERBOSE_DEBUG
#ifndef pr_vdebug
@@ -177,7 +178,7 @@ struct ffs_data {
struct completion ep0req_completion; /* P: mutex */
/* reference counter */
- atomic_tref;
+ refcount_t ref;
/* how many files are opened (EP0 and others) */
atomic_topened;
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 26/29] drivers, usb: convert dev_data.count from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/usb/gadget/legacy/inode.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/legacy/inode.c
b/drivers/usb/gadget/legacy/inode.c
index 79a2d8f..81d76f3 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
@@ -114,7 +115,7 @@ enum ep0_state {
struct dev_data {
spinlock_t lock;
- atomic_tcount;
+ refcount_t count;
enum ep0_state state; /* P: lock */
struct usb_gadgetfs_event event [N_EVENT];
unsignedev_next;
@@ -150,12 +151,12 @@ struct dev_data {
static inline void get_dev (struct dev_data *data)
{
- atomic_inc (&data->count);
+ refcount_inc (&data->count);
}
static void put_dev (struct dev_data *data)
{
- if (likely (!atomic_dec_and_test (&data->count)))
+ if (likely (!refcount_dec_and_test (&data->count)))
return;
/* needs no more cleanup */
BUG_ON (waitqueue_active (&data->wait));
@@ -170,7 +171,7 @@ static struct dev_data *dev_new (void)
if (!dev)
return NULL;
dev->state = STATE_DEV_DISABLED;
- atomic_set (&dev->count, 1);
+ refcount_set (&dev->count, 1);
spin_lock_init (&dev->lock);
INIT_LIST_HEAD (&dev->epfiles);
init_waitqueue_head (&dev->wait);
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 23/29] drivers: convert vme_user_vma_priv.refcnt from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/staging/vme/devices/vme_user.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 69e9a770..a3d4610 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -17,7 +17,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include
+#include
#include
#include
#include
@@ -118,7 +118,7 @@ static const int type[VME_DEVS] = { MASTER_MINOR,
MASTER_MINOR,
struct vme_user_vma_priv {
unsigned int minor;
- atomic_t refcnt;
+ refcount_t refcnt;
};
static ssize_t resource_to_user(int minor, char __user *buf, size_t count,
@@ -430,7 +430,7 @@ static void vme_user_vm_open(struct vm_area_struct *vma)
{
struct vme_user_vma_priv *vma_priv = vma->vm_private_data;
- atomic_inc(&vma_priv->refcnt);
+ refcount_inc(&vma_priv->refcnt);
}
static void vme_user_vm_close(struct vm_area_struct *vma)
@@ -438,7 +438,7 @@ static void vme_user_vm_close(struct vm_area_struct *vma)
struct vme_user_vma_priv *vma_priv = vma->vm_private_data;
unsigned int minor = vma_priv->minor;
- if (!atomic_dec_and_test(&vma_priv->refcnt))
+ if (!refcount_dec_and_test(&vma_priv->refcnt))
return;
mutex_lock(&image[minor].mutex);
@@ -473,7 +473,7 @@ static int vme_user_master_mmap(unsigned int minor, struct
vm_area_struct *vma)
}
vma_priv->minor = minor;
- atomic_set(&vma_priv->refcnt, 1);
+ refcount_set(&vma_priv->refcnt, 1);
vma->vm_ops = &vme_user_vm_ops;
vma->vm_private_data = vma_priv;
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 29/29] drivers, xen: convert grant_map.users from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/xen/gntdev.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 2ef2b61..b183cb2 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
#include
#include
@@ -85,7 +86,7 @@ struct grant_map {
int index;
int count;
int flags;
- atomic_t users;
+ refcount_t users;
struct unmap_notify notify;
struct ioctl_gntdev_grant_ref *grants;
struct gnttab_map_grant_ref *map_ops;
@@ -165,7 +166,7 @@ static struct grant_map *gntdev_alloc_map(struct
gntdev_priv *priv, int count)
add->index = 0;
add->count = count;
- atomic_set(&add->users, 1);
+ refcount_set(&add->users, 1);
return add;
@@ -211,7 +212,7 @@ static void gntdev_put_map(struct gntdev_priv *priv, struct
grant_map *map)
if (!map)
return;
- if (!atomic_dec_and_test(&map->users))
+ if (!refcount_dec_and_test(&map->users))
return;
atomic_sub(map->count, &pages_mapped);
@@ -399,7 +400,7 @@ static void gntdev_vma_open(struct vm_area_struct *vma)
struct grant_map *map = vma->vm_private_data;
pr_debug("gntdev_vma_open %p\n", vma);
- atomic_inc(&map->users);
+ refcount_inc(&map->users);
}
static void gntdev_vma_close(struct vm_area_struct *vma)
@@ -1003,7 +1004,7 @@ static int gntdev_mmap(struct file *flip, struct
vm_area_struct *vma)
goto unlock_out;
}
- atomic_inc(&map->users);
+ refcount_inc(&map->users);
vma->vm_ops = &gntdev_vmops;
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 27/29] drivers, usb: convert ep_data.count from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/usb/gadget/legacy/inode.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/legacy/inode.c
b/drivers/usb/gadget/legacy/inode.c
index 81d76f3..d21a5f8 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -191,7 +191,7 @@ enum ep_state {
struct ep_data {
struct mutexlock;
enum ep_state state;
- atomic_tcount;
+ refcount_t count;
struct dev_data *dev;
/* must hold dev->lock before accessing ep or req */
struct usb_ep *ep;
@@ -206,12 +206,12 @@ struct ep_data {
static inline void get_ep (struct ep_data *data)
{
- atomic_inc (&data->count);
+ refcount_inc (&data->count);
}
static void put_ep (struct ep_data *data)
{
- if (likely (!atomic_dec_and_test (&data->count)))
+ if (likely (!refcount_dec_and_test (&data->count)))
return;
put_dev (data->dev);
/* needs no more cleanup */
@@ -1562,7 +1562,7 @@ static int activate_ep_files (struct dev_data *dev)
init_waitqueue_head (&data->wait);
strncpy (data->name, ep->name, sizeof (data->name) - 1);
- atomic_set (&data->count, 1);
+ refcount_set (&data->count, 1);
data->dev = dev;
get_dev (dev);
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 01/10] x86: assembly, ENTRY for fn, GLOBAL for data
On 03/03/2017, 07:20 PM, h...@zytor.com wrote: > On March 1, 2017 2:27:54 AM PST, Ingo Molnar wrote: >> >> * Thomas Gleixner wrote: >> >>> On Wed, 1 Mar 2017, Ingo Molnar wrote: * Jiri Slaby wrote: > This is a start of series to unify use of ENTRY, ENDPROC, GLOBAL, >> END, > and other macros across x86. When we have all this sorted out, >> this will > help to inject DWARF unwinding info by objtool later. > > So, let us use the macros this way: > * ENTRY -- start of a global function > * ENDPROC -- end of a local/global function > * GLOBAL -- start of a globally visible data symbol > * END -- end of local/global data symbol So how about using macro names that actually show the purpose, >> instead of importing all the crappy, historic, essentially randomly chosen >> debug symbol macro names from the binutils and older kernels? Something sane, like: SYM__FUNCTION_START >>> >>> Sane would be: >>> >>> SYM_FUNCTION_START >>> >>> The double underscore is just not giving any value. >> >> So the double underscore (at least in my view) has two advantages: >> >> 1) it helps separate the prefix from the postfix. >> >> I.e. it's a 'symbols' namespace, and a 'function start', not the >> 'start' of a >> 'symbol function'. >> >> 2) It also helps easy greppability. >> >> Try this in latest -tip: >> >> git grep e820__ >> >> To see all the E820 API calls - with no false positives! >> >> 'git grep e820_' on the other hand is a lot less reliable... >> >> But no strong feelings either way, I just try to sneak in these small >> namespace >> structure tricks when nobody's looking! ;-) >> >> Thanks, >> >> Ingo > > This seems needlessly verbose to me and clutters the code. > > How about: > > PROC..ENDPROC, LOCALPROC..ENDPROC and DATA..ENDDATA. Clear, unambiguous and > balanced. I tried this, but: arch/x86/kernel/relocate_kernel_64.S:27:0: warning: "DATA" redefined #define DATA(offset) (KEXEC_CONTROL_CODE_MAX_SIZE+(offset)) I am not saying that I cannot fix it up. I just want to say, that these names might be too generic, especially "PROC" and "DATA". So should I really stick to these? thanks, -- js suse labs ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 04/29] drivers, connector: convert cn_callback_entry.refcnt from atomic_t to refcount_t
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
---
drivers/connector/cn_queue.c | 4 ++--
drivers/connector/connector.c | 2 +-
include/linux/connector.h | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/connector/cn_queue.c b/drivers/connector/cn_queue.c
index 1f8bf05..9c54fdf 100644
--- a/drivers/connector/cn_queue.c
+++ b/drivers/connector/cn_queue.c
@@ -45,7 +45,7 @@ cn_queue_alloc_callback_entry(struct cn_queue_dev *dev, const
char *name,
return NULL;
}
- atomic_set(&cbq->refcnt, 1);
+ refcount_set(&cbq->refcnt, 1);
atomic_inc(&dev->refcnt);
cbq->pdev = dev;
@@ -58,7 +58,7 @@ cn_queue_alloc_callback_entry(struct cn_queue_dev *dev, const
char *name,
void cn_queue_release_callback(struct cn_callback_entry *cbq)
{
- if (!atomic_dec_and_test(&cbq->refcnt))
+ if (!refcount_dec_and_test(&cbq->refcnt))
return;
atomic_dec(&cbq->pdev->refcnt);
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 25693b0..8615594b 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -157,7 +157,7 @@ static int cn_call_callback(struct sk_buff *skb)
spin_lock_bh(&dev->cbdev->queue_lock);
list_for_each_entry(i, &dev->cbdev->queue_list, callback_entry) {
if (cn_cb_equal(&i->id.id, &msg->id)) {
- atomic_inc(&i->refcnt);
+ refcount_inc(&i->refcnt);
cbq = i;
break;
}
diff --git a/include/linux/connector.h b/include/linux/connector.h
index f8fe863..032102b 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -22,7 +22,7 @@
#define __CONNECTOR_H
-#include
+#include
#include
#include
@@ -49,7 +49,7 @@ struct cn_callback_id {
struct cn_callback_entry {
struct list_head callback_entry;
- atomic_t refcnt;
+ refcount_t refcnt;
struct cn_queue_dev *pdev;
struct cn_callback_id id;
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V4 0/8] COLO-Proxy: Make Xen COLO use userspace colo-proxy
On Mon, Mar 06, 2017 at 10:59:18AM +0800, Zhang Chen wrote: > Because of some reason, We no longer support COLO kernel proxy. However the #7 talks about kernel colo proxy? Is this description out-dated? > V4: One usually adds these comments to the patches. That is right after the '---' you say: v4: New patch (for the new patches) Or for the older patches (like #7): v4: Fix typo. Fix 'some bug' (perhaps be specific). >- Because the origin 3/7 has been merged, > remove it in this series. >- Add new patch "COLO-Proxy: Add colo-compare notify args" > to support qemu side patch: > https://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg07265.html > >- Add colo_userspace_proxy macro as a separate patch. >- Add some comments about COLO and fix some typo in patch 2/7. >- Fix some bug and typo in patch 7/7. But the subject is 0/8? So it can't be 7/7. Did you mean 7/8? or 8/8/? >- Rebase codes on upstream Xen. Thanks! ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] build/clang: remove the address-of-packed-member warning
On 06/03/17 13:58, Jan Beulich wrote: On 06.03.17 at 13:31, wrote: >> --- a/Config.mk >> +++ b/Config.mk >> @@ -216,6 +216,7 @@ $(call >> cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement) >> $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement) >> $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable) >> $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs) >> +$(call cc-option-add,CFLAGS,CC,-Wno-address-of-packed-member) > > Actually, having thought some more about this, the warning > should be suppressed only for x86 imo. ARM wants aligned > accesses after all. Looking at Roger's complaint, it appears that the warning is issued even if the member actually is aligned, if *on some unknown system*, it might someday be un-aligned. -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
On 03/06/2017 07:52 AM, Jan Beulich wrote: On 06.03.17 at 13:31, wrote: >> There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from >> working as expected, and vpmu.o ends up with a reference to >> __xsm_action_mismatch_detected which makes the build fail: >> >> [...] >> ld-melf_x86_64_fbsd -T xen.lds -N prelink.o \ >> xen/common/symbols-dummy.o -o xen/.xen-syms.0 >> prelink.o: In function `xsm_default_action': >> xen/include/xsm/dummy.h:80: undefined reference to >> `__xsm_action_mismatch_detected' >> xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 >> against undefined symbol `__xsm_action_mismatch_detected' >> ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' >> isn't defined >> >> Then doing a search in the objects files: >> >> # find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \ >> 'for filename; do nm "$filename" | \ >> grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash >> xen/arch/x86/prelink.o >> xen/arch/x86/cpu/vpmu.o >> xen/arch/x86/cpu/built_in.o >> xen/arch/x86/built_in.o >> >> The current patch is the only way I've found to fix this so far, by simply >> moving the XSM_PRIV check into the default case in xsm_pmu_op. This also >> fixes >> the behavior of do_xenpmu_op, which will now return -EINVAL for unknown >> XENPMU_* operations, instead of -EPERM when called by a privileged domain. How will these changes make do_xenpmu_op() return -EINVAL? -boris > Especially from this perspective I think the patch is fine (but also > Cc-ing Boris), yet I still think the compilation aspect needs to be got > to the bottom of, to have a complete picture in case the problem > shows up in a slightly different way elsewhere. Did you report this > to clang folks? Did they have any explanation of what's going on? > > Jan > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [xtf test] 106490: all pass - PUSHED
flight 106490 xtf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/106490/
Perfect :-)
All tests in this flight passed as required
version targeted for testing:
xtf 4d06734fbb48879be65bab9fefda51dcb253541c
baseline version:
xtf f02259db8c737220b4e6ae5564a8f6da4fba2949
Last test of basis 106365 2017-03-02 10:44:47 Z4 days
Testing same since 106490 2017-03-06 11:15:38 Z0 days1 attempts
People who touched revisions under test:
Andrew Cooper
jobs:
build-amd64-xtf pass
build-amd64 pass
build-amd64-pvopspass
test-xtf-amd64-amd64-1 pass
test-xtf-amd64-amd64-2 pass
test-xtf-amd64-amd64-3 pass
test-xtf-amd64-amd64-4 pass
test-xtf-amd64-amd64-5 pass
sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images
Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs
Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master
Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary
Pushing revision :
+ branch=xtf
+ revision=4d06734fbb48879be65bab9fefda51dcb253541c
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
getconfig Repos
perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xtf
4d06734fbb48879be65bab9fefda51dcb253541c
+ branch=xtf
+ revision=4d06734fbb48879be65bab9fefda51dcb253541c
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
getconfig Repos
perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xtf
+ xenbranch=xen-unstable
+ '[' xxtf = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.8-testing
+ '[' x4d06734fbb48879be65bab9fefda51dcb253541c = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git
++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/xen/git/linux-pvops.git
++ : git://xenbits.xen.org/linux-pvops.git
++ : tested/linux-3.14
++ : tested/linux-arm-xen
++ '[' xgit://xenbits.xen.org/linux-pvops.git = x
Re: [Xen-devel] [PATCH 02/11] xen/arm: vpl011: Add new hvm params in Xen for ring buffer/event setup
On 06/03/17 13:21, Julien Grall wrote: > Hi Jan, > > On 06/03/17 12:41, Jan Beulich wrote: > On 06.03.17 at 12:42, wrote: >>> I thought a bit more about those params. I think the name should be >>> generic and not tie to pl011 because we may want to emulate different >>> UART for the guest in the future. >> >> That's reasonable, but I continue to have reservations against the >> underlying approach here, namely ... >> >>> Also, by re-using deprecated encoding it means that it will not be >>> possible to use those parameters on x86 if you ever decide to emulate >>> UART in Xen. I am not sure whether if you are happy with that? >> >> ... with this in mind: If we wanted to do this for HVM guests, we'd >> indeed want the parameters. If we wanted this for PV guests, the >> model wouldn't fit at all. >> >> Plus what I'm not understanding (perhaps because most of the >> discussion around this seemed to be ARM-specific, and hence I've >> skipped reading through it) is - why is this UART emulation needed >> in the first place? We've never had a need for such on x86, afaia. > > Linaro has published a set of guidelines to guarantee a same guest image > can run on multiple hypervisors (see [1]). This specification mandates > the presence of a SBSA-compliant UART. > > I just realized the cover letter does not explain why we need to emulate > a PL011 on ARM. Bhupinder can you detail it on the next version? Right, but in order to evaluate your question about whether we're "happy" with not being able to use these parameters if we ever decide to emulate UART on x86, we need to know a reason that one might decide to add a UART *on x86*, not on ARM. I mean, I don't think we're running out of bits, so I don't think it's a problem allocating new HVM_PARAM numbers so that we can re-use them if we ever decide to implement UARTs on x86. On the other hand, given that nobody has ever suggested doing so or knows why it might be useful, maybe it makes more sense to just re-use some x86 params and allocate extra numbers if / when we decide we want to implement UART on x86. -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v16 5/9] x86: change default load address from 1 MiB to 2 MiB
On Wed, Mar 01, 2017 at 04:21:35AM -0700, Jan Beulich wrote: > >>> On 01.03.17 at 11:51, wrote: > > On Wed, Mar 01, 2017 at 03:34:47AM -0700, Jan Beulich wrote: > >> >>> On 01.03.17 at 11:13, wrote: > >> > On Wed, Mar 01, 2017 at 02:05:39AM -0700, Jan Beulich wrote: > >> >> >>> On 21.02.17 at 20:19, wrote: > >> >> > Subsequent patches introducing relocatable early boot code play with > >> >> > page tables using 2 MiB huge pages. If load address is not aligned at > >> >> > 2 MiB then code touching such page tables must have special cases for > >> >> > start and end of Xen image memory region. So, let's make life easier > >> >> > and move default load address from 1 MiB to 2 MiB. This way page table > >> >> > code will be nice and easy. Hence, there is a chance that it will be > >> >> > less error prone too... :-))) > >> >> > > >> >> > Additionally, drop first 2 MiB mapping from Xen image mapping. > >> >> > It is no longer needed. > >> >> > >> >> What about the memory allocated for it? Aiui at least in the xen.efi > >> >> case there would be a 2Mb chunk left unused, but also never freed. > >> > > >> > Memory is not allocated for it. > >> > >> Are you sure? Where would the PE32+ header live in that case? > > > > Is the PE32+ header loaded into memory? I think that only code and data > > stuff > > is put there. Header is useful only for loader. Am I missing something? > > I think EFI's loader simply follows Windows' one(s). And yes, the Sadly, I do not know how Windows' one(s) work(s). > headers can be useful - to the loader itself (albeit then the loader > assumes that no-one fiddles with them). I have looked at OVMF. It looks that OVMF loads PE32+ header aside, parses it, puts Xen code and data into allocated memory (EfiLoaderCode type), does relocations and jumps into entry point. So, more or less as I expected. It looks that it does not load whole file at once. Hence, taking into account that memory allocated for Xen image is marked as EfiLoaderCode it means that from Xen point of view this region is free. Of course later everything between _start and _end is reserved. So, it seems to me that everything between __image_base__ and _start is free and available for Xen memory allocator. Am I missing something here? Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH] mm, hotplug: get rid of auto_online_blocks
On Fri 03-03-17 18:34:22, Igor Mammedov wrote:
> On Fri, 3 Mar 2017 09:27:23 +0100
> Michal Hocko wrote:
>
> > On Thu 02-03-17 18:03:15, Igor Mammedov wrote:
> > > On Thu, 2 Mar 2017 15:28:16 +0100
> > > Michal Hocko wrote:
> > >
> > > > On Thu 02-03-17 14:53:48, Igor Mammedov wrote:
> > > > [...]
> > > > > When trying to support memory unplug on guest side in RHEL7,
> > > > > experience shows otherwise. Simplistic udev rule which onlines
> > > > > added block doesn't work in case one wants to online it as movable.
> > > > >
> > > > > Hotplugged blocks in current kernel should be onlined in reverse
> > > > > order to online blocks as movable depending on adjacent blocks zone.
> > > > >
> > > >
> > > > Could you be more specific please? Setting online_movable from the udev
> > > > rule should just work regardless of the ordering or the state of other
> > > > memblocks. If that doesn't work I would call it a bug.
> > > It's rather an implementation constrain than a bug
> > > for details and workaround patch see
> > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1314306#c7
> >
> > "You are not authorized to access bug #1314306"
> Sorry,
> I've made it public, related comments and patch should be accessible now
> (code snippets in BZ are based on older kernel but logic is still the same
> upstream)
>
> > could you paste the reasoning here please?
> sure here is reproducer:
> start VM with CLI:
> qemu-system-x86_64 -enable-kvm -m size=1G,slots=2,maxmem=4G -numa node \
> -object memory-backend-ram,id=m1,size=1G -device pc-dimm,node=0,memdev=m1 \
> /path/to/guest_image
>
> then in guest dimm1 blocks are from 32-39
>
> echo online_movable > /sys/devices/system/memory/memory32/state
> -bash: echo: write error: Invalid argument
>
> in current mainline kernel it triggers following code path:
>
> online_pages()
> ...
>if (online_type == MMOP_ONLINE_KERNEL) {
>
> if (!zone_can_shift(pfn, nr_pages, ZONE_NORMAL, &zone_shift))
>
> return -EINVAL;
Are you sure? I would expect MMOP_ONLINE_MOVABLE here
> zone_can_shift()
> ...
> if (idx < target) {
>
> /* pages must be at end of current zone */
>
> if (pfn + nr_pages != zone_end_pfn(zone))
>
> return false;
>
> since we are trying to online as movable not the last section in
> ZONE_NORMAL.
>
> Here is what makes hotplugged memory end up in ZONE_NORMAL:
> acpi_memory_enable_device() -> add_memory -> add_memory_resource ->
>-> arch/x86/mm/init_64.c
>
> /*
> * Memory is added always to NORMAL zone. This means you will never get
> * additional DMA/DMA32 memory.
> */
> int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> {
> ...
> struct zone *zone = pgdat->node_zones +
> zone_for_memory(nid, start, size, ZONE_NORMAL, for_device);
>
> i.e. all hot-plugged memory modules always go to ZONE_NORMAL
> and only the first/last block in zone is allowed to be moved
> to another zone. Patch [1] tries to fix issue by assigning
> removable memory resource to movable zone so hotplugged+removable
> blocks look like:
> movable normal, movable, movable
> instead of current:
> normal, normal, normal movable
Hmm, this code is confusing and clean as mud. I have to stare there some
more but AFAIK zones shouldn't have problems with holes so the only
thing we have to guarantee is that different zones do not overlap. So
this smells like a bug rather than the ineherent implementation
limitation.
[...]
> > > > > Which means simple udev rule isn't usable since it gets event from
> > > > > the first to the last hotplugged block order. So now we would have
> > > > > to write a daemon that would
> > > > > - watch for all blocks in hotplugged memory appear (how would it
> > > > > know)
> > > > > - online them in right order (order might also be different depending
> > > > >on kernel version)
> > > > >-- it becomes even more complicated in NUMA case when there are
> > > > > multiple zones and kernel would have to provide user-space
> > > > > with information about zone maps
> > > > >
> > > > > In short current experience shows that userspace approach
> > > > > - doesn't solve issues that Vitaly has been fixing (i.e. onlining
> > > > >fast and/or under memory pressure) when udev (or something else
> > > > >might be killed)
> > > >
> > > > yeah and that is why the patch does the onlining from the kernel.
> > > onlining in this patch is limited to hyperv and patch breaks
> > > auto-online on x86 kvm/vmware/baremetal as they reuse the same
> > > hotplug path.
> >
> > Those can use the udev or do you see any reason why they couldn't?
>
> Reasons are above, under >>>
Re: [Xen-devel] [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
>>> On 06.03.17 at 15:42, wrote: > On 03/06/2017 07:52 AM, Jan Beulich wrote: > On 06.03.17 at 13:31, wrote: >>> There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from >>> working as expected, and vpmu.o ends up with a reference to >>> __xsm_action_mismatch_detected which makes the build fail: >>> >>> [...] >>> ld-melf_x86_64_fbsd -T xen.lds -N prelink.o \ >>> xen/common/symbols-dummy.o -o xen/.xen-syms.0 >>> prelink.o: In function `xsm_default_action': >>> xen/include/xsm/dummy.h:80: undefined reference to >>> `__xsm_action_mismatch_detected' >>> xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 >>> against undefined symbol `__xsm_action_mismatch_detected' >>> ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' >>> isn't defined >>> >>> Then doing a search in the objects files: >>> >>> # find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \ >>> 'for filename; do nm "$filename" | \ >>> grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash >>> xen/arch/x86/prelink.o >>> xen/arch/x86/cpu/vpmu.o >>> xen/arch/x86/cpu/built_in.o >>> xen/arch/x86/built_in.o >>> >>> The current patch is the only way I've found to fix this so far, by simply >>> moving the XSM_PRIV check into the default case in xsm_pmu_op. This also >>> fixes >>> the behavior of do_xenpmu_op, which will now return -EINVAL for unknown >>> XENPMU_* operations, instead of -EPERM when called by a privileged domain. > > How will these changes make do_xenpmu_op() return -EINVAL? Since the XSM check no longer returns -EPERM, the handling inside do_vpmu_op() will now reach the default case there. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v16 5/9] x86: change default load address from 1 MiB to 2 MiB
>>> On 06.03.17 at 15:51, wrote: > On Wed, Mar 01, 2017 at 04:21:35AM -0700, Jan Beulich wrote: >> >>> On 01.03.17 at 11:51, wrote: >> > On Wed, Mar 01, 2017 at 03:34:47AM -0700, Jan Beulich wrote: >> >> >>> On 01.03.17 at 11:13, wrote: >> >> > On Wed, Mar 01, 2017 at 02:05:39AM -0700, Jan Beulich wrote: >> >> >> >>> On 21.02.17 at 20:19, wrote: >> >> >> > Subsequent patches introducing relocatable early boot code play with >> >> >> > page tables using 2 MiB huge pages. If load address is not aligned at >> >> >> > 2 MiB then code touching such page tables must have special cases for >> >> >> > start and end of Xen image memory region. So, let's make life easier >> >> >> > and move default load address from 1 MiB to 2 MiB. This way page >> >> >> > table >> >> >> > code will be nice and easy. Hence, there is a chance that it will be >> >> >> > less error prone too... :-))) >> >> >> > >> >> >> > Additionally, drop first 2 MiB mapping from Xen image mapping. >> >> >> > It is no longer needed. >> >> >> >> >> >> What about the memory allocated for it? Aiui at least in the xen.efi >> >> >> case there would be a 2Mb chunk left unused, but also never freed. >> >> > >> >> > Memory is not allocated for it. >> >> >> >> Are you sure? Where would the PE32+ header live in that case? >> > >> > Is the PE32+ header loaded into memory? I think that only code and data > stuff >> > is put there. Header is useful only for loader. Am I missing something? >> >> I think EFI's loader simply follows Windows' one(s). And yes, the > > Sadly, I do not know how Windows' one(s) work(s). > >> headers can be useful - to the loader itself (albeit then the loader >> assumes that no-one fiddles with them). > > I have looked at OVMF. It looks that OVMF loads PE32+ header aside, parses > it, puts > Xen code and data into allocated memory (EfiLoaderCode type), does > relocations and > jumps into entry point. So, more or less as I expected. It looks that it does > not > load whole file at once. Hence, taking into account that memory allocated for > Xen > image is marked as EfiLoaderCode it means that from Xen point of view this > region > is free. Of course later everything between _start and _end is reserved. So, > it > seems to me that everything between __image_base__ and _start is free and > available for Xen memory allocator. Am I missing something here? Oh, right, the EFI memory type is what removes the need to free anything here. I'm sorry for the noise. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
On 03/06/2017 09:54 AM, Jan Beulich wrote: On 06.03.17 at 15:42, wrote: >> On 03/06/2017 07:52 AM, Jan Beulich wrote: >> On 06.03.17 at 13:31, wrote: There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from working as expected, and vpmu.o ends up with a reference to __xsm_action_mismatch_detected which makes the build fail: [...] ld-melf_x86_64_fbsd -T xen.lds -N prelink.o \ xen/common/symbols-dummy.o -o xen/.xen-syms.0 prelink.o: In function `xsm_default_action': xen/include/xsm/dummy.h:80: undefined reference to `__xsm_action_mismatch_detected' xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 against undefined symbol `__xsm_action_mismatch_detected' ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' isn't defined Then doing a search in the objects files: # find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \ 'for filename; do nm "$filename" | \ grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash xen/arch/x86/prelink.o xen/arch/x86/cpu/vpmu.o xen/arch/x86/cpu/built_in.o xen/arch/x86/built_in.o The current patch is the only way I've found to fix this so far, by simply moving the XSM_PRIV check into the default case in xsm_pmu_op. This also fixes the behavior of do_xenpmu_op, which will now return -EINVAL for unknown XENPMU_* operations, instead of -EPERM when called by a privileged domain. >> How will these changes make do_xenpmu_op() return -EINVAL? > Since the XSM check no longer returns -EPERM, the handling inside > do_vpmu_op() will now reach the default case there. Oh, I was staring at xsm_default_action() and trying to see where -EINVAL would be coming from. Sorry for the noise. -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] build/clang: remove the address-of-packed-member warning
At 14:36 + on 06 Mar (1488811016), George Dunlap wrote:
> On 06/03/17 13:58, Jan Beulich wrote:
> On 06.03.17 at 13:31, wrote:
> >> --- a/Config.mk
> >> +++ b/Config.mk
> >> @@ -216,6 +216,7 @@ $(call
> >> cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
> >> $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
> >> $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable)
> >> $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs)
> >> +$(call cc-option-add,CFLAGS,CC,-Wno-address-of-packed-member)
> >
> > Actually, having thought some more about this, the warning
> > should be suppressed only for x86 imo. ARM wants aligned
> > accesses after all.
>
> Looking at Roger's complaint, it appears that the warning is issued even
> if the member actually is aligned, if *on some unknown system*, it might
> someday be un-aligned.
AIUI the complaint is (based on the simplified example from the ticket):
struct __attribute__((__packed__)) bar {
uint16_t x1;
uint16_t x2;
} b;
&b.x2;
Because the struct is packed, it has alignment 1, and so do its
fields. &b.x2 is a pointer to a uint16_t, but it _isn't_ 16-bit
aligned (because the whole struct is only byte-aligned).
So I guess that one fix would be to declare that the struct has
appropriate alignment? I don't know whether that would suppress the
warning, but the clang devs might be more receptive to seeing it as
a false positive.
Tim.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] build/clang: fix XSM dummy policy when using clang 4.0
On Mon, Mar 06, 2017 at 05:52:14AM -0700, Jan Beulich wrote: > >>> On 06.03.17 at 13:31, wrote: > > There seems to be some weird bug in clang 4.0 that prevents xsm_pmu_op from > > working as expected, and vpmu.o ends up with a reference to > > __xsm_action_mismatch_detected which makes the build fail: > > > > [...] > > ld-melf_x86_64_fbsd -T xen.lds -N prelink.o \ > > xen/common/symbols-dummy.o -o xen/.xen-syms.0 > > prelink.o: In function `xsm_default_action': > > xen/include/xsm/dummy.h:80: undefined reference to > > `__xsm_action_mismatch_detected' > > xen/xen/include/xsm/dummy.h:80: relocation truncated to fit: R_X86_64_PC32 > > against undefined symbol `__xsm_action_mismatch_detected' > > ld: xen/xen/.xen-syms.0: hidden symbol `__xsm_action_mismatch_detected' > > isn't defined > > > > Then doing a search in the objects files: > > > > # find xen/ -type f -name '*.o' -print0 | xargs -0 bash -c \ > > 'for filename; do nm "$filename" | \ > > grep -q __xsm_action_mismatch_detected && echo "$filename"; done' bash > > xen/arch/x86/prelink.o > > xen/arch/x86/cpu/vpmu.o > > xen/arch/x86/cpu/built_in.o > > xen/arch/x86/built_in.o > > > > The current patch is the only way I've found to fix this so far, by simply > > moving the XSM_PRIV check into the default case in xsm_pmu_op. This also > > fixes > > the behavior of do_xenpmu_op, which will now return -EINVAL for unknown > > XENPMU_* operations, instead of -EPERM when called by a privileged domain. > > Especially from this perspective I think the patch is fine (but also > Cc-ing Boris), yet I still think the compilation aspect needs to be got > to the bottom of, to have a complete picture in case the problem > shows up in a slightly different way elsewhere. Did you report this > to clang folks? Did they have any explanation of what's going on? I've just reported this upstream, for the record the ticket is at: http://bugs.llvm.org/show_bug.cgi?id=32150 Roger. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 21/29] drivers, s390: convert fc_fcp_pkt.ref_cnt from atomic_t to refcount_t
On 03/06/2017 03:21 PM, Elena Reshetova wrote: > refcount_t type and corresponding API should be > used instead of atomic_t when the variable is used as > a reference counter. This allows to avoid accidental > refcounter overflows that might lead to use-after-free > situations. The subject is wrong, should be something like "scsi: libfc convert fc_fcp_pkt.ref_cnt from atomic_t to refcount_t" but not s390. Other than that Acked-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] build/clang: remove the address-of-packed-member warning
On 06/03/17 15:16, Tim Deegan wrote:
> At 14:36 + on 06 Mar (1488811016), George Dunlap wrote:
>> On 06/03/17 13:58, Jan Beulich wrote:
>> On 06.03.17 at 13:31, wrote:
--- a/Config.mk
+++ b/Config.mk
@@ -216,6 +216,7 @@ $(call
cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
$(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
$(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable)
$(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs)
+$(call cc-option-add,CFLAGS,CC,-Wno-address-of-packed-member)
>>> Actually, having thought some more about this, the warning
>>> should be suppressed only for x86 imo. ARM wants aligned
>>> accesses after all.
>> Looking at Roger's complaint, it appears that the warning is issued even
>> if the member actually is aligned, if *on some unknown system*, it might
>> someday be un-aligned.
> AIUI the complaint is (based on the simplified example from the ticket):
>
> struct __attribute__((__packed__)) bar {
> uint16_t x1;
> uint16_t x2;
> } b;
>
> &b.x2;
>
> Because the struct is packed, it has alignment 1, and so do its
> fields. &b.x2 is a pointer to a uint16_t, but it _isn't_ 16-bit
> aligned (because the whole struct is only byte-aligned).
>
> So I guess that one fix would be to declare that the struct has
> appropriate alignment? I don't know whether that would suppress the
> warning, but the clang devs might be more receptive to seeing it as
> a false positive.
The structs in question (segment_attributes and segment_register) have
proper natural alignment with no padding anyway, so don't need to be
__packed__ to be correct.
It would be better to remove the unnecessary __packed__.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [GSoC 2017] Rust bindings for libxl
Hello, I'm Saurav Sachidanand, and I'm a CS sophomore studying in India. For more than year I've been programming in Rust and have published some personal projects in it (few involving the Rust-C FFI) and have contributed a some code to Servo (github.com/saurvs). I've also played around a bit with kernel modules in NetBSD. I'm interested in Xen's project for creating Rust bindings for libxl. Since I'm new to Xen, I'll spend time reading the docs, building and testing out Xen, and researching on the how to go about the implementing the bindings. I'd greatly appreciate any guidance and pointers you can give regarding this project. And if you could point me to some small coding tasks, I can start working it to get familiar with Xen's code base. Thanks, Saurav ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 5/5] x86/vioapic: allow PVHv2 Dom0 to have more than one IO APIC
>>> On 23.02.17 at 12:52, wrote:
> int vioapic_init(struct domain *d)
> {
> +unsigned int i, nr_vioapics = is_hardware_domain(d) ? nr_ioapics : 1;
Considering this ...
> @@ -646,24 +656,41 @@ int vioapic_init(struct domain *d)
>
> if ( (d->arch.hvm_domain.vioapic == NULL) &&
> ((d->arch.hvm_domain.vioapic =
> - xmalloc(struct hvm_hw_vioapic)) == NULL) )
> + xzalloc_array(struct hvm_hw_vioapic, nr_vioapics)) == NULL) )
> return -ENOMEM;
>
> -domain_vioapic(d, 0)->redirtbl = xmalloc_array(union vioapic_redir_entry,
> - VIOAPIC_NUM_PINS);
> -if ( !domain_vioapic(d, 0)->redirtbl )
> +if ( !is_hardware_domain(d) )
> {
> -xfree(d->arch.hvm_domain.vioapic);
> -return -ENOMEM;
> +ASSERT(nr_vioapics == 1);
... this is kind of strange a check.
Everything else looks fine, but this will see some changes due to
the intended data structure adjustments in an earlier patch.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/mem_access: fixed vm_event emulation check with altp2m enabled
On Mon, Mar 6, 2017 at 2:28 AM, Razvan Cojocaru
wrote:
> Currently, p2m_mem_access_emulate_check() uses p2m_get_mem_access()
> to check if the page restrictions have been lifted between the time
> of sending the vm_event out and the reception of the reply - in
> which case emulation is no longer required. Unfortunately,
> p2m_get_mem_access() uses p2m_get_hostp2m(d) which only checks the
> default EPT (view 0 in altp2m parlance). This patch fixes this by
> checking the active altp2m view instead, whenever applicable.
>
> Signed-off-by: Razvan Cojocaru
Acked-by: Tamas K Lengyel
> ---
> xen/arch/x86/mm/mem_access.c | 98
> +---
> 1 file changed, 56 insertions(+), 42 deletions(-)
>
> diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
> index 3ebeb4f..29a0c43 100644
> --- a/xen/arch/x86/mm/mem_access.c
> +++ b/xen/arch/x86/mm/mem_access.c
> @@ -32,14 +32,68 @@
>
> #include "mm-locks.h"
>
> +/*
> + * Get access type for a gfn.
> + * If gfn == INVALID_GFN, gets the default access type.
> + */
> +static int _p2m_get_mem_access(struct p2m_domain *p2m, gfn_t gfn,
> + xenmem_access_t *access)
> +{
> +p2m_type_t t;
> +p2m_access_t a;
> +mfn_t mfn;
> +
> +static const xenmem_access_t memaccess[] = {
> +#define ACCESS(ac) [p2m_access_##ac] = XENMEM_access_##ac
> +ACCESS(n),
> +ACCESS(r),
> +ACCESS(w),
> +ACCESS(rw),
> +ACCESS(x),
> +ACCESS(rx),
> +ACCESS(wx),
> +ACCESS(rwx),
> +ACCESS(rx2rw),
> +ACCESS(n2rwx),
> +#undef ACCESS
> +};
> +
> +/* If request to get default access. */
> +if ( gfn_eq(gfn, INVALID_GFN) )
> +{
> +*access = memaccess[p2m->default_access];
> +return 0;
> +}
> +
> +gfn_lock(p2m, gfn, 0);
> +mfn = p2m->get_entry(p2m, gfn_x(gfn), &t, &a, 0, NULL, NULL);
> +gfn_unlock(p2m, gfn, 0);
> +
> +if ( mfn_eq(mfn, INVALID_MFN) )
> +return -ESRCH;
> +
> +if ( (unsigned) a >= ARRAY_SIZE(memaccess) )
> +return -ERANGE;
> +
> +*access = memaccess[a];
> +return 0;
> +}
> +
> bool p2m_mem_access_emulate_check(struct vcpu *v,
>const vm_event_response_t *rsp)
> {
> xenmem_access_t access;
> bool violation = 1;
> const struct vm_event_mem_access *data = &rsp->u.mem_access;
> +struct domain *d = v->domain;
> +struct p2m_domain *p2m = NULL;
>
> -if ( p2m_get_mem_access(v->domain, _gfn(data->gfn), &access) == 0 )
> +if ( altp2m_active(d) )
> +p2m = p2m_get_altp2m(v);
> +if ( !p2m )
> +p2m = p2m_get_hostp2m(d);
> +
> +if ( _p2m_get_mem_access(p2m, _gfn(data->gfn), &access) == 0 )
> {
> switch ( access )
> {
> @@ -405,51 +459,11 @@ long p2m_set_mem_access_multi(struct domain *d,
> return rc;
> }
>
> -/*
> - * Get access type for a gfn.
> - * If gfn == INVALID_GFN, gets the default access type.
> - */
> int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t *access)
> {
> struct p2m_domain *p2m = p2m_get_hostp2m(d);
> -p2m_type_t t;
> -p2m_access_t a;
> -mfn_t mfn;
> -
> -static const xenmem_access_t memaccess[] = {
> -#define ACCESS(ac) [p2m_access_##ac] = XENMEM_access_##ac
> -ACCESS(n),
> -ACCESS(r),
> -ACCESS(w),
> -ACCESS(rw),
> -ACCESS(x),
> -ACCESS(rx),
> -ACCESS(wx),
> -ACCESS(rwx),
> -ACCESS(rx2rw),
> -ACCESS(n2rwx),
> -#undef ACCESS
> -};
> -
> -/* If request to get default access. */
> -if ( gfn_eq(gfn, INVALID_GFN) )
> -{
> -*access = memaccess[p2m->default_access];
> -return 0;
> -}
>
> -gfn_lock(p2m, gfn, 0);
> -mfn = p2m->get_entry(p2m, gfn_x(gfn), &t, &a, 0, NULL, NULL);
> -gfn_unlock(p2m, gfn, 0);
> -
> -if ( mfn_eq(mfn, INVALID_MFN) )
> -return -ESRCH;
> -
> -if ( (unsigned) a >= ARRAY_SIZE(memaccess) )
> -return -ERANGE;
> -
> -*access = memaccess[a];
> -return 0;
> +return _p2m_get_mem_access(p2m, gfn, access);
> }
>
> /*
> --
> 1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 11/29] drivers, media: convert cx88_core.refcount from atomic_t to refcount_t
Hello.
On 03/06/2017 05:20 PM, Elena Reshetova wrote:
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova
Signed-off-by: Hans Liljestrand
Signed-off-by: Kees Cook
Signed-off-by: David Windsor
[...]
diff --git a/drivers/media/pci/cx88/cx88.h b/drivers/media/pci/cx88/cx88.h
index 115414c..16c1313 100644
--- a/drivers/media/pci/cx88/cx88.h
+++ b/drivers/media/pci/cx88/cx88.h
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
@@ -339,7 +340,7 @@ struct cx8802_dev;
struct cx88_core {
struct list_head devlist;
- atomic_t refcount;
+ refcount_t refcount;
Could you please keep the name aligned with above and below?
/* board name */
intnr;
MBR, Sergei
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [xtf test] 106493: regressions - all pass
flight 106493 xtf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/106493/
Regressions :-(
Tests which did not succeed and are blocking,
including tests which could not be run:
test-xtf-amd64-amd64-5 34 xtf/test-hvm32pae-swint-emulation fail REGR. vs.
106490
test-xtf-amd64-amd64-5 38 xtf/test-hvm32pse-swint-emulation fail REGR. vs.
106490
test-xtf-amd64-amd64-5 46 xtf/test-hvm64-swint-emulation fail REGR. vs. 106490
version targeted for testing:
xtf 1d68cab2189bd035002ecc32cce154f979f9eff5
baseline version:
xtf 4d06734fbb48879be65bab9fefda51dcb253541c
Last test of basis 106490 2017-03-06 11:15:38 Z0 days
Testing same since 106493 2017-03-06 15:14:31 Z0 days1 attempts
People who touched revisions under test:
Andrew Cooper
jobs:
build-amd64-xtf pass
build-amd64 pass
build-amd64-pvopspass
test-xtf-amd64-amd64-1 pass
test-xtf-amd64-amd64-2 pass
test-xtf-amd64-amd64-3 pass
test-xtf-amd64-amd64-4 pass
test-xtf-amd64-amd64-5 pass
sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images
Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs
Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master
Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary
Not pushing.
commit 1d68cab2189bd035002ecc32cce154f979f9eff5
Author: Andrew Cooper
Date: Mon Mar 6 11:58:05 2017 +
Drop test_wants_user_mappings infrastructure
As noted in Errata, the test_wants_user_mappings infrastructure has problems
for pv32pae environments on Xen 4.6 and earlier, if Xen leaks SMEP/SMAP
settings into the guest.
Now that all tests have moved to the new .text.user infrastructure, drop
test_wants_user_mappings.
Signed-off-by: Andrew Cooper
commit 55260c180db50b84690bb13a6946787a7ecd5ccc
Author: Andrew Cooper
Date: Mon Mar 6 11:41:10 2017 +
Switch tests over to using .text.user
... in preference to test_wants_user_mappings. This involves duplicating
the
stubs which need to be executed in user context, and moving them into
.text.user.
As a result, the tests become SMEP/SMAP-safe, even in cases were such
settings
are leaked from Xen.
Signed-off-by: Andrew Cooper
commit 07f29b74020fa3b3f4fc2209e620e71a838062b4
Author: Andrew Cooper
Date: Mon Mar 6 13:31:42 2017 +
Fix x{get,set}bv() build with Clang 3.9
In 32bit builds, Clang objects to using uint64_t's with 32bit asm operands.
In file included from /local/xen-test-framework.git/arch/x86/pv/traps.c:7:
/local/xen-test-framework.git/arch/x86/include/arch/lib.h:404:52: error:
invalid output size for constraint '=d'
asm volatile ("xgetbv" : "=a" (feat_lo), "=d" (feat_hi)
^
/local/xen-test-framework.git/arch/x86/include/arch/lib.h:412:59: error:
invalid input size for constraint 'd'
asm volatile ("xsetbv" :: "a" ((uint32_t)value), "d" (value >> 32),
^
Signed-off-by: Andrew Cooper
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
