All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
From: Faiz Abbas @ 2020-02-20 11:24 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, linux-mmc, devicetree, ulf.hansson, adrian.hunter
In-Reply-To: <24e6ac71-00c7-f140-86d8-fa5ec0dcaff0@ti.com>

Rob,

On 14/02/20 4:28 pm, Faiz Abbas wrote:
> Rob,
> 
> On 07/02/20 3:07 pm, Faiz Abbas wrote:
>> Rob,
>>
>> On 20/01/20 11:00 am, Faiz Abbas wrote:
>>> Hi Rob,
>>>
>>> On 15/01/20 7:20 am, Rob Herring wrote:
>>>> On Wed, Jan 08, 2020 at 08:39:18PM +0530, Faiz Abbas wrote:
>>>>> According to latest AM65x Data Manual[1], a different output tap delay
>>>>> value is recommended for all speed modes. Therefore, replace the
>>>>> ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
>>>>> mode.
>>>>>
>>>>> [1] http://www.ti.com/lit/gpn/am6526
>>>>>
>>>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>>>>> ---
>>>>>  .../devicetree/bindings/mmc/sdhci-am654.txt   | 21 +++++++++++++++++--
>>>>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>>> index 50e87df47971..c6ccecb9ae5a 100644
>>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>>> @@ -18,7 +18,20 @@ Required Properties:
>>>>>  	- clocks: Handles to the clock inputs.
>>>>>  	- clock-names: Tuple including "clk_xin" and "clk_ahb"
>>>>>  	- interrupts: Interrupt specifiers
>>>>> -	- ti,otap-del-sel: Output Tap Delay select
>>>>> +	Output tap delay for each speed mode:
>>>>> +	- ti,otap-del-sel-legacy
>>>>> +	- ti,otap-del-sel-mmc-hs
>>>>> +	- ti,otap-del-sel-sd-hs
>>>>> +	- ti,otap-del-sel-sdr12
>>>>> +	- ti,otap-del-sel-sdr25
>>>>> +	- ti,otap-del-sel-sdr50
>>>>> +	- ti,otap-del-sel-sdr104
>>>>> +	- ti,otap-del-sel-ddr50
>>>>> +	- ti,otap-del-sel-ddr52
>>>>> +	- ti,otap-del-sel-hs200
>>>>> +	- ti,otap-del-sel-hs400
>>>>> +	  These bindings must be provided otherwise the driver will disable the
>>>>> +	  corresponding speed mode (i.e. all nodes must provide at least -legacy)
>>>>
>>>> Why not just extend the existing property to be an array. We already 
>>>> have properties to enable/disable speed modes.
>>>>
>>>
>>> Its hard to keep track of which modes have values and which don't when
>>> you add an array. This scheme is just easier on anyone adding new values
>>> or updating old values.
>>>
>>> We already disable speed modes based on platform specific properties in
>>> other drivers. In sdhci-omap.c, the driver disables the corresponding
>>> speed mode if the corresponding pinmux and iodelay values are not populated.
>>>
>>
>> Do you agree on above?
>>
> 
> Gentle ping.
> 

Ping.

Thanks,
Faiz

^ permalink raw reply

* Re: [PATCH v3 05/17] s390x: protvirt: Support unpack facility
From: Janosch Frank @ 2020-02-20 11:21 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: qemu-s390x, mihajlov, qemu-devel, david
In-Reply-To: <20200220113950.015984bf.cohuck@redhat.com>


[-- Attachment #1.1: Type: text/plain, Size: 8830 bytes --]

On 2/20/20 11:39 AM, Cornelia Huck wrote:
> On Fri, 14 Feb 2020 10:16:24 -0500
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> When a guest has saved a ipib of type 5 and call diagnose308 with
> 
> s/call/calls/
> 
>> subcode 10, we have to setup the protected processing environment via
>> Ultravisor calls. The calls are done by KVM and are exposed via an API.
>>
>> The following steps are necessary:
>> 1. Create a VM (register it with the Ultravisor)
>> 2. Create secure CPUs for all of our current cpus
>> 3. Forward the secure header to the Ultravisor (has all information on
>> how to decrypt the image and VM information)
>> 4. Protect image pages from the host and decrypt them
>> 5. Verify the image integrity
>>
>> Only after step 5 a protected VM is allowed to run.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> [Changes
>> to machine]
>> ---
>>  hw/s390x/Makefile.objs              |   1 +
>>  hw/s390x/ipl.c                      |  32 ++++++
>>  hw/s390x/ipl.h                      |   2 +
>>  hw/s390x/pv.c                       | 154 ++++++++++++++++++++++++++++
>>  hw/s390x/pv.h                       |  38 +++++++
>>  hw/s390x/s390-virtio-ccw.c          |  79 ++++++++++++++
>>  include/hw/s390x/s390-virtio-ccw.h  |   1 +
>>  target/s390x/cpu.c                  |   4 +
>>  target/s390x/cpu.h                  |   1 +
>>  target/s390x/cpu_features_def.inc.h |   1 +
>>  10 files changed, 313 insertions(+)
>>  create mode 100644 hw/s390x/pv.c
>>  create mode 100644 hw/s390x/pv.h
> 
> (...)
> 
>> diff --git a/hw/s390x/pv.c b/hw/s390x/pv.c
>> new file mode 100644
>> index 0000000000..5b6a26cba9
>> --- /dev/null
>> +++ b/hw/s390x/pv.c
>> @@ -0,0 +1,154 @@
>> +/*
>> + * Secure execution functions
>> + *
>> + * Copyright IBM Corp. 2019
> 
> Update the year?

ack.

> 
>> + * Author(s):
>> + *  Janosch Frank <frankja@linux.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
>> + * your option) any later version. See the COPYING file in the top-level
>> + * directory.
>> + */
> 
> (...)
> 
>> +void s390_pv_vm_destroy(void)
>> +{
>> +     s390_pv_cmd_exit(KVM_PV_VM_DESTROY, NULL);
> 
> Why does this exit()? Should Never Happen?

Yes, and we can't recover from this.

> 
>> +}
>> +
>> +int s390_pv_vcpu_create(CPUState *cs)
>> +{
>> +    int rc;
>> +
>> +    rc = s390_pv_cmd_vcpu(cs, KVM_PV_VCPU_CREATE, NULL);
>> +    if (!rc) {
>> +        S390_CPU(cs)->env.pv = true;
>> +    }
>> +
>> +    return rc;
>> +}
>> +
>> +void s390_pv_vcpu_destroy(CPUState *cs)
>> +{
>> +    s390_pv_cmd_vcpu_exit(cs, KVM_PV_VCPU_DESTROY, NULL);
> 
> dito
> 
>> +    S390_CPU(cs)->env.pv = false;
>> +}
> 
> (...)
> 
>> +void s390_pv_perf_clear_reset(void)
>> +{
>> +    s390_pv_cmd_exit(KVM_PV_VM_PREP_RESET, NULL);
> 
> And here. Or is that because the machine should not be left around in
> an undefined state?

If it failed, we could only try again, there's no fixing the problem.
So I chose to rather exit instead of looping around something which most
likely will never recover after the first error.

> 
>> +}
>> +
>> +int s390_pv_verify(void)
>> +{
>> +    return s390_pv_cmd(KVM_PV_VM_VERIFY, NULL);
>> +}
>> +
>> +void s390_pv_unshare(void)
>> +{
>> +    s390_pv_cmd_exit(KVM_PV_VM_UNSHARE_ALL, NULL);
>> +}
>> diff --git a/hw/s390x/pv.h b/hw/s390x/pv.h
>> new file mode 100644
>> index 0000000000..7d20bdd12e
>> --- /dev/null
>> +++ b/hw/s390x/pv.h
>> @@ -0,0 +1,38 @@
>> +/*
>> + * Protected Virtualization header
>> + *
>> + * Copyright IBM Corp. 2019
> 
> Year++
> 
>> + * Author(s):
>> + *  Janosch Frank <frankja@linux.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
>> + * your option) any later version. See the COPYING file in the top-level
>> + * directory.
>> + */
>> +
>> +#ifndef HW_S390_PV_H
>> +#define HW_S390_PV_H
>> +
>> +#ifdef CONFIG_KVM
>> +int s390_pv_vm_create(void);
>> +void s390_pv_vm_destroy(void);
>> +void s390_pv_vcpu_destroy(CPUState *cs);
>> +int s390_pv_vcpu_create(CPUState *cs);
>> +int s390_pv_set_sec_parms(uint64_t origin, uint64_t length);
>> +int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak);
>> +void s390_pv_perf_clear_reset(void);
>> +int s390_pv_verify(void);
>> +void s390_pv_unshare(void);
>> +#else
>> +int s390_pv_vm_create(void) { return 0; }
> 
> I'm wondering why you return 0 here (and below). These function should
> not be called for !KVM, but just to help catch logic error, use -EINVAL
> or so?
> 
>> +void s390_pv_vm_destroy(void) {}
>> +void s390_pv_vcpu_destroy(CPUState *cs) {}
>> +int s390_pv_vcpu_create(CPUState *cs) { return 0; }
>> +int s390_pv_set_sec_parms(uint64_t origin, uint64_t length) { return 0; }
>> +int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak) { return 0: }
>> +void s390_pv_perf_clear_reset(void) {}
>> +int s390_pv_verify(void) { return 0; }
>> +void s390_pv_unshare(void) {}
>> +#endif
>> +
>> +#endif /* HW_S390_PV_H */
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index e759eb5f83..5fa4372083 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -41,6 +41,7 @@
>>  #include "hw/qdev-properties.h"
>>  #include "hw/s390x/tod.h"
>>  #include "sysemu/sysemu.h"
>> +#include "hw/s390x/pv.h"
>>  
>>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>>  {
>> @@ -240,9 +241,11 @@ static void s390_create_sclpconsole(const char *type, Chardev *chardev)
>>  static void ccw_init(MachineState *machine)
>>  {
>>      int ret;
>> +    S390CcwMachineState *ms = S390_CCW_MACHINE(machine);
>>      VirtualCssBus *css_bus;
>>      DeviceState *dev;
>>  
>> +    ms->pv = false;
> 
> I'm wondering why you need to init this to false - isn't it already
> zeroed out?
> 
>>      s390_sclp_init();
>>      /* init memory + setup max page size. Required for the CPU model */
>>      s390_memory_init(machine->ram_size);
>> @@ -318,10 +321,58 @@ static inline void s390_do_cpu_ipl(CPUState *cs, run_on_cpu_data arg)
>>      s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
>>  }
>>  
>> +static int s390_machine_pv_secure(S390CcwMachineState *ms)
>> +{
>> +    CPUState *t;
>> +    int rc;
>> +
>> +    /* Create SE VM */
>> +    rc = s390_pv_vm_create();
>> +    if (rc) {
>> +        return rc;
>> +    }
>> +
>> +    CPU_FOREACH(t) {
>> +        rc = s390_pv_vcpu_create(t);
>> +        if (rc) {
>> +            return rc;
> 
> No need to undo something on error?

There have been changes in this area anyway, since Christian switched to
one create/destroy instead of separate for vm and vcpu.

I'll update the error handling in the new state and send out the patches
ssonish.


> 
>> +        }
>> +    }
>> +
>> +    ms->pv = true;
>> +
>> +    /* Set SE header and unpack */
>> +    rc = s390_ipl_prepare_pv_header();
>> +    if (rc) {
>> +        return rc;
> 
> Also here.
> 
>> +    }
>> +
>> +    /* Decrypt image */
>> +    rc = s390_ipl_pv_unpack();
>> +    if (rc) {
>> +        return rc;
> 
> And here.
> 
>> +    }
>> +
>> +    /* Verify integrity */
>> +    rc = s390_pv_verify();
>> +    return rc;
> 
> And here.
> 
>> +}
> 
> (...)
> 
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index 8da1905485..1dbd84b9d7 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -37,6 +37,8 @@
>>  #include "sysemu/hw_accel.h"
>>  #include "hw/qdev-properties.h"
>>  #ifndef CONFIG_USER_ONLY
>> +#include "hw/s390x/s390-virtio-ccw.h"
>> +#include "hw/s390x/pv.h"
>>  #include "hw/boards.h"
>>  #include "sysemu/arch_init.h"
>>  #include "sysemu/sysemu.h"
>> @@ -191,6 +193,7 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
>>  
>>  #if !defined(CONFIG_USER_ONLY)
>>      MachineState *ms = MACHINE(qdev_get_machine());
>> +    S390CcwMachineState *ccw = S390_CCW_MACHINE(ms);
> 
> I find the variable name a bit confusing... maybe ccw_ms?
> 
>>      unsigned int max_cpus = ms->smp.max_cpus;
>>      if (cpu->env.core_id >= max_cpus) {
>>          error_setg(&err, "Unable to add CPU with core-id: %" PRIu32
>> @@ -205,6 +208,7 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
>>          goto out;
>>      }
>>  
>> +    cpu->env.pv = ccw->pv;
> 
> So, if you add a cpu, it will inherit the pv state of the machine...
> doesn't it need any setup?
> 
>>      /* sync cs->cpu_index and env->core_id. The latter is needed for TCG. */
>>      cs->cpu_index = cpu->env.core_id;
>>  #endif
> 
> (...)
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] KVM: Suppress warning in __kvm_gfn_to_hva_cache_init
From: Paolo Bonzini @ 2020-02-20 11:22 UTC (permalink / raw)
  To: Sean Christopherson, Oliver Upton; +Cc: kvm
In-Reply-To: <20200218190729.GD28156@linux.intel.com>

On 18/02/20 20:07, Sean Christopherson wrote:
> On Tue, Feb 18, 2020 at 10:47:56AM -0800, Oliver Upton wrote:
>> Particularly draconian compilers warn of a possible uninitialized use of
>> the nr_pages_avail variable. Silence this warning by initializing it to
>> zero.
> Can you check if the warning still exists with commit 6ad1e29fe0ab ("KVM:
> Clean up __kvm_gfn_to_hva_cache_init() and its callers")?  I'm guessing
> (hoping?) the suppression is no longer necessary.

What if __gfn_to_hva_many and gfn_to_hva_many are marked __always_inline?

Thanks,

Paolo


^ permalink raw reply

* Re: [PATCH net-next v3 4/9] sysfs: add sysfs_change_owner()
From: Greg Kroah-Hartman @ 2020-02-20 11:23 UTC (permalink / raw)
  To: Christian Brauner
  Cc: David S. Miller, linux-kernel, netdev, Rafael J. Wysocki,
	Pavel Machek, Jakub Kicinski, Eric Dumazet, Stephen Hemminger,
	linux-pm
In-Reply-To: <20200218162943.2488012-5-christian.brauner@ubuntu.com>

On Tue, Feb 18, 2020 at 05:29:38PM +0100, Christian Brauner wrote:
> Add a helper to change the owner of sysfs objects.
> This function will be used to correctly account for kobject ownership
> changes, e.g. when moving network devices between network namespaces.
> 
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> /* v2 */
> -  Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
>    - Add comment how ownership of sysfs object is changed.
> 
> /* v3 */
> -  Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
>    - Add explicit uid/gid parameters.
> ---
>  fs/sysfs/file.c       | 39 +++++++++++++++++++++++++++++++++++++++
>  include/linux/sysfs.h |  6 ++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> index df5107d7b3fd..02f7e852aad4 100644
> --- a/fs/sysfs/file.c
> +++ b/fs/sysfs/file.c
> @@ -665,3 +665,42 @@ int sysfs_file_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid)
>  	return error;
>  }
>  EXPORT_SYMBOL_GPL(sysfs_file_change_owner);
> +
> +/**
> + *	sysfs_change_owner - change owner of the given object.

"and all of the files associated with this kobject", right?

> + *	@kobj:	object.
> + *	@kuid:	new owner's kuid
> + *	@kgid:	new owner's kgid
> + */
> +int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid)
> +{
> +	int error;
> +	const struct kobj_type *ktype;
> +
> +	if (!kobj->state_in_sysfs)
> +		return -EINVAL;
> +
> +	error = sysfs_file_change_owner(kobj, kuid, kgid);

Ok, this changes the attributes of the sysfs directory for the kobject
itself.

> +	if (error)
> +		return error;
> +
> +	ktype = get_ktype(kobj);
> +	if (ktype) {
> +		struct attribute **kattr;
> +
> +		for (kattr = ktype->default_attrs; kattr && *kattr; kattr++) {
> +			error = sysfs_file_change_owner_by_name(
> +				kobj, (*kattr)->name, kuid, kgid);
> +			if (error)
> +				return error;
> +		}

And here you change all of the files of the kobject.

But what about files that have a subdir?  Does that also happen here?

> +
> +		error = sysfs_groups_change_owner(kobj, ktype->default_groups,
> +						  kuid, kgid);

Then what are you changing here?

I think the kerneldoc needs a lot more explaination as to what is going
on in this function and why you would call it, and not some of the other
functions you are adding.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 2/2] MAINTAINERS: Set MIPS status to Odd Fixes
From: Thomas Bogendoerfer @ 2020-02-20 11:23 UTC (permalink / raw)
  To: Paul Burton; +Cc: linux-mips, linux-kernel
In-Reply-To: <20200219191730.1277800-3-paulburton@kernel.org>

On Wed, Feb 19, 2020 at 11:17:30AM -0800, Paul Burton wrote:
> My time with MIPS the company has reached its end, and so at best I'll
> have little time spend on maintaining arch/mips/. Reflect that in
> MAINTAINERS by changing status to Odd Fixes. Hopefully this might spur
> the involvement of someone with more time, but even if not it should
> help serve to avoid unrealistic expectations.

I'd like to jump in as MIPS maintainer. I'm doing Linux MIPS kernel
development since ages (started with an Olivetti M700 and kernel version
2.x) and right now time for doing the jobs isn't issue:-)

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

^ permalink raw reply

* Re: [PATCHv5 33/34] drm/rockchip: Use helper for common task
From: Boris Brezillon @ 2020-02-20 11:24 UTC (permalink / raw)
  To: Andrzej Pietrasiewicz
  Cc: kernel, Mihail Atanassov, David Airlie, Liviu Dudau, Sandy Huang,
	dri-devel, James Wang, Ayan Halder, Sean Paul
In-Reply-To: <20191217145020.14645-34-andrzej.p@collabora.com>

On Tue, 17 Dec 2019 15:50:19 +0100
Andrzej Pietrasiewicz <andrzej.p@collabora.com> wrote:

> Use generic helper code.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index 221e72e71432..5806f908aa53 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -18,6 +18,7 @@
>  #include "rockchip_drm_fb.h"
>  #include "rockchip_drm_gem.h"
>  
> +

You can drop this blank line. Looks good otherwise, but I'll wait for v6
(with the new ->check_size() maybe) before giving my R-b.

>  static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
>  	.destroy       = drm_gem_fb_destroy,
>  	.create_handle = drm_gem_fb_create_handle,
> @@ -30,22 +31,13 @@ rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cm
>  {
>  	struct drm_framebuffer *fb;
>  	int ret;
> -	int i;
>  
>  	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
>  	if (!fb)
>  		return ERR_PTR(-ENOMEM);
>  
> -	drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
> -
> -	for (i = 0; i < num_planes; i++)
> -		fb->obj[i] = obj[i];
> -
> -	ret = drm_framebuffer_init(dev, fb, &rockchip_drm_fb_funcs);
> +	ret = drm_gem_fb_init_with_funcs(fb, dev, mode_cmd, obj, num_planes, &rockchip_drm_fb_funcs);
>  	if (ret) {
> -		DRM_DEV_ERROR(dev->dev,
> -			      "Failed to initialize framebuffer: %d\n",
> -			      ret);
>  		kfree(fb);
>  		return ERR_PTR(ret);
>  	}

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH 05/12] drm/msm/dpu: Stop copying around mode->private_flags
From: Emil Velikov @ 2020-02-20 11:24 UTC (permalink / raw)
  To: Ville Syrjala
  Cc: Sean Paul, linux-arm-msm, Intel Graphics Development, freedreno,
	ML dri-devel
In-Reply-To: <20200219203544.31013-6-ville.syrjala@linux.intel.com>

On Wed, 19 Feb 2020 at 20:36, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The driver never sets mode->private_flags so copying
> it back and forth is entirely pointless. Stop doing it.
>
> Also drop private_flags from the tracepoint.
>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Perhaps the msm team has a WIP which makes use of it ?

Otherwise:
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH 05/12] drm/msm/dpu: Stop copying around mode->private_flags
From: Emil Velikov @ 2020-02-20 11:24 UTC (permalink / raw)
  To: Ville Syrjala
  Cc: ML dri-devel, linux-arm-msm, Intel Graphics Development,
	freedreno, Sean Paul
In-Reply-To: <20200219203544.31013-6-ville.syrjala@linux.intel.com>

On Wed, 19 Feb 2020 at 20:36, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The driver never sets mode->private_flags so copying
> it back and forth is entirely pointless. Stop doing it.
>
> Also drop private_flags from the tracepoint.
>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Perhaps the msm team has a WIP which makes use of it ?

Otherwise:
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

-Emil

^ permalink raw reply

* Re: [Intel-gfx] [PATCH 05/12] drm/msm/dpu: Stop copying around mode->private_flags
From: Emil Velikov @ 2020-02-20 11:24 UTC (permalink / raw)
  To: Ville Syrjala
  Cc: linux-arm-msm, Intel Graphics Development, freedreno,
	ML dri-devel
In-Reply-To: <20200219203544.31013-6-ville.syrjala@linux.intel.com>

On Wed, 19 Feb 2020 at 20:36, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The driver never sets mode->private_flags so copying
> it back and forth is entirely pointless. Stop doing it.
>
> Also drop private_flags from the tracepoint.
>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Perhaps the msm team has a WIP which makes use of it ?

Otherwise:
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

-Emil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH net-next v3 5/9] device: add device_change_owner()
From: Greg Kroah-Hartman @ 2020-02-20 11:25 UTC (permalink / raw)
  To: Christian Brauner
  Cc: David S. Miller, linux-kernel, netdev, Rafael J. Wysocki,
	Pavel Machek, Jakub Kicinski, Eric Dumazet, Stephen Hemminger,
	linux-pm
In-Reply-To: <20200218162943.2488012-6-christian.brauner@ubuntu.com>

On Tue, Feb 18, 2020 at 05:29:39PM +0100, Christian Brauner wrote:
> Add a helper to change the owner of a device's sysfs entries. This
> needs to happen when the ownership of a device is changed, e.g. when
> moving network devices between network namespaces.
> This function will be used to correctly account for ownership changes,
> e.g. when moving network devices between network namespaces.
> 
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> /* v2 */
> unchanged
> 
> /* v3 */
> -  Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
>    - Add explicit uid/gid parameters.
> ---
>  drivers/base/core.c    | 80 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/device.h |  1 +
>  2 files changed, 81 insertions(+)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 42a672456432..ec0d5e8cfd0f 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -3458,6 +3458,86 @@ int device_move(struct device *dev, struct device *new_parent,
>  }
>  EXPORT_SYMBOL_GPL(device_move);
>  
> +static int device_attrs_change_owner(struct device *dev, kuid_t kuid,
> +				     kgid_t kgid)
> +{
> +	struct kobject *kobj = &dev->kobj;
> +	struct class *class = dev->class;
> +	const struct device_type *type = dev->type;
> +	int error;
> +
> +	if (class) {
> +		error = sysfs_groups_change_owner(kobj, class->dev_groups, kuid,
> +						  kgid);
> +		if (error)
> +			return error;
> +	}
> +
> +	if (type) {
> +		error = sysfs_groups_change_owner(kobj, type->groups, kuid,
> +						  kgid);
> +		if (error)
> +			return error;
> +	}
> +
> +	error = sysfs_groups_change_owner(kobj, dev->groups, kuid, kgid);
> +	if (error)
> +		return error;
> +
> +	if (device_supports_offline(dev) && !dev->offline_disabled) {
> +		error = sysfs_file_change_owner_by_name(
> +			kobj, dev_attr_online.attr.name, kuid, kgid);
> +		if (error)
> +			return error;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * device_change_owner - change the owner of an existing device.

The "owner" and what else gets changed here?  Please document this
better.


> + * @dev: device.
> + * @kuid: new owner's kuid
> + * @kgid: new owner's kgid
> + */
> +int device_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid)
> +{
> +	int error;
> +	struct kobject *kobj = &dev->kobj;
> +
> +	dev = get_device(dev);
> +	if (!dev)
> +		return -EINVAL;
> +
> +	error = sysfs_change_owner(kobj, kuid, kgid);

the kobject of the device is changed, good.

> +	if (error)
> +		goto out;
> +
> +	error = sysfs_file_change_owner_by_name(kobj, dev_attr_uevent.attr.name,
> +						kuid, kgid);

Why call out the uevent file explicitly here?

> +	if (error)
> +		goto out;
> +
> +	error = device_attrs_change_owner(dev, kuid, kgid);
> +	if (error)
> +		goto out;

Doesn't this also change the uevent file?

> +
> +#ifdef CONFIG_BLOCK
> +	if (sysfs_deprecated && dev->class == &block_class)
> +		goto out;
> +#endif

Ugh, we still need this?

> +
> +	error = sysfs_link_change_owner(&dev->class->p->subsys.kobj, &dev->kobj,
> +					dev_name(dev), kuid, kgid);

Now what is this changing?

Again, more documentation please as to exactly what is being changed in
this function is needed.

thanks,

greg k-h

^ permalink raw reply

* Re: [alsa-devel] Implicit feedback solution for Boss GT-1 and maybe other devices
From: Keith A. Milner @ 2020-02-20 11:24 UTC (permalink / raw)
  To: alsa-devel; +Cc: Mike Oliphant
In-Reply-To: <CAHXb3bcrSnAnwS+-HY8jh4eoBdt+tBoZTHSr-5jsGomN156fFw@mail.gmail.com>

On Thursday, 23 January 2020 10:06:58 GMT Mike Oliphant wrote:
> I received a very helpful email response to my previous thread about trying
> to get the Boss GT-1 pedalboard working reliably, and as a result I now
> have a working solution.
> 
> The issue seems to be that the GT-1 does not send any capture data until it
> gets some playback data first.

Hi Mike,

In my original investigations (which you referenced) this was the conclusion I 
was starting to reach, but real-world events meant I didn't have the time to 
follow up on it, so I'm grateful that you have.

In the kernel.org bugzilla report you also mention that you have a potential 
fix. I would be willing to test this, subject to time constraints (in other 
words it may or may not take me some time to get around to testing this).

Feel free to contact me directly to send me a patch (best email address is 
user kamilner on the same domain used to send this email).

For reference, I have a GT-1, a GT-001, and a 1st gen Katana I can test with.

Regards,

Keith



^ permalink raw reply

* [dpdk-dev] DPDK Release Status Meeting 20/02/2020
From: Ferruh Yigit @ 2020-02-20 11:25 UTC (permalink / raw)
  To: dpdk-dev; +Cc: Thomas Monjalon

Minutes 20 February 2020
------------------------

Agenda:
* Release Dates
* -rc3 status
* Subtrees
* OvS
* Opens

Participants:
* Arm
* Debian/Microsoft
* Intel
* Marvell
* Mellanox
* NXP
* Red Hat


Release Dates
-------------

* v20.02 dates:
  * -rc3 is released on 16 February
    * https://mails.dpdk.org/archives/dev/2020-February/157922.html
  * -rc4			Friday 21 February 2020
  * Release pushed to		*Friday 25 February 2020*
    * Release date may postponed if any critical issue found in -rc4 testing

* v20.05 *updated* proposal, please comment:
  * Proposal/V1:		Wednesday 18 March 2020
  * Integration/Merge/RC1:	Friday 17 April 2020
  * Release:			Wednesday 20 May 2020

  * The update request is based on delays in 20.02
  * Release date postponed more taking into PRC holiday on 1-5 May into account


-rc3 Status
-----------

* Intel and Red Hat sent test results, thank you.
  * https://mails.dpdk.org/archives/dev/2020-February/158184.html
  * https://mails.dpdk.org/archives/dev/2020-February/158285.html


Subtrees
--------

* main
  * For pure experimental libraries, there is an library versioning problem
    This is critical to be solved for the -rc4, Intel will try to find resource
    to work on it.
    * https://mails.dpdk.org/archives/dev/2020-February/158012.html
  * Devtools will be merged
  * Doc/deprecation notices are in the queue
  * Requested an early merge from next-net, since some fixes accumulated
  * John will start on working release notes review

* next-net
  * Bug fixes merged for -rc4, pulled sub-trees

* next-net-crypto
  * There are some fixes, Akhil will merge in next a few hours
  * examples/fips_validation patch will be merged to main repo by David

* next-net-eventdev
  * No update

* next-net-virtio
  * Bug fixes merged for -rc4

* next-net-intel
  * Bug fixes merged for -rc4

* LTS

  * 17.11.10-rc1 released, please test and report results
    * https://mails.dpdk.org/archives/dev/2020-January/154915.html
    * Waiting for QAT issue
      * Release will be postponed one more week


OvS
---

* 2.13 is ready, waiting for announcement


Opens
-----

* Lots of coverity issues fixed, thanks.

* There is a coverity sticker in patchwork now
  * https://patches.dpdk.org/project/dpdk/list/
    Which reports 14 new defects as of now
  * Plan is to trigger coverity scan from the travis



DPDK Release Status Meetings
============================

The DPDK Release Status Meeting is intended for DPDK Committers to discuss
the status of the master tree and sub-trees, and for project managers to
track progress or milestone dates.

The meeting occurs on Thursdays at 8:30 UTC. If you wish to attend just
send an email to "John McNamara <john.mcnamara@intel.com>" for the invite.

^ permalink raw reply

* Re: [PATCH v3 06/17] s390x: protvirt: Add migration blocker
From: Janosch Frank @ 2020-02-20 11:24 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: qemu-s390x, mihajlov, qemu-devel, david
In-Reply-To: <20200220114815.01634a4c.cohuck@redhat.com>


[-- Attachment #1.1: Type: text/plain, Size: 2672 bytes --]

On 2/20/20 11:48 AM, Cornelia Huck wrote:
> On Fri, 14 Feb 2020 10:16:25 -0500
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Migration is not yet supported.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  hw/s390x/s390-virtio-ccw.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 5fa4372083..d64724af91 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -42,6 +42,9 @@
>>  #include "hw/s390x/tod.h"
>>  #include "sysemu/sysemu.h"
>>  #include "hw/s390x/pv.h"
>> +#include "migration/blocker.h"
>> +
>> +static Error *pv_mig_blocker;
>>  
>>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>>  {
>> @@ -373,6 +376,7 @@ static void s390_machine_reset(MachineState *machine)
>>      CPUState *cs, *t;
>>      S390CPU *cpu;
>>      S390CcwMachineState *ms = S390_CCW_MACHINE(machine);
>> +    static Error *local_err;
>>  
>>      /* get the reset parameters, reset them once done */
>>      s390_ipl_get_reset_request(&cs, &reset_type);
>> @@ -422,6 +426,17 @@ static void s390_machine_reset(MachineState *machine)
>>          }
>>          run_on_cpu(cs, s390_do_cpu_reset, RUN_ON_CPU_NULL);
>>  
>> +        if (!pv_mig_blocker) {
>> +            error_setg(&pv_mig_blocker,
>> +                       "protected VMs are currently not migrateable.");
>> +        }
>> +        migrate_add_blocker(pv_mig_blocker, &local_err);
> 
> If I'm not lost in the context, that's during PV_RESET. I'm a bit
> confused why you'd add the blocker here?

Where would you want me to add it?
It's here where we switch into secure mode and I need to block before
switching and unblock if it fails.

When having the blocker in diag.c, I'd have a hard time unblocking on a
PV switch fail.

> 
>> +        if (local_err) {
>> +            error_report_err(local_err);
>> +            error_free(pv_mig_blocker);
>> +            exit(1);
> 
> Why the exit()? Can't you fail the call?

Well, if that fails and we go protected, I wouldn't be protected agains
migrations, right?

> 
>> +        }
>> +
>>          if (s390_machine_pv_secure(ms)) {
>>              CPU_FOREACH(t) {
>>                  s390_pv_vcpu_destroy(t);
>> @@ -430,6 +445,7 @@ static void s390_machine_reset(MachineState *machine)
>>              ms->pv = false;
>>  
>>              s390_machine_inject_pv_error(cs);
>> +            migrate_del_blocker(pv_mig_blocker);
>>              s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
>>              return;
>>          }
> 
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] describe: output tag's ref instead of embedded name
From: Jeff King @ 2020-02-20 11:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matheus Tavares, git, rhi
In-Reply-To: <xmqqftf6hlrt.fsf@gitster-ct.c.googlers.com>

On Wed, Feb 19, 2020 at 03:14:14AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I think a left a few things unsaid in my "v1.0-bob" example. I was
> > imagining that there are _two_ v1.0 tags floating around. One that you
> > consider "wrong", tagged by Bob, and one you like. You keep the latter
> > in refs/tags/v1.0.
> 
> Ahh, OK.  
> 
> To continue playing devil's advocate and to step back a bit,
> 
>  - The "git describe" command finds that the given commit is
>    "closest" to that tag Bob called "v1.0".
> 
>  - But if it outputs "v1.0" like the current code does, it cannot be
>    fed back to get_oid() to name the right object, if the given commit
>    is "at" the tag (i.e. there is no "-$number-g$objectname" suffix),
>    which is a problem.  We want "git describe" to give an output
>    usable with get_oid() and the name must refer to the correct
>    object (i.e. the one given to "git describe" as an input).
> 
> There are multiple approaches to make it possible to feed the output
> back to get_oid() to obtain correct result:
> [...]

Thanks for clearly laying out your thinking. All of what you wrote makes
sense to me and I'd be OK with any of the options you described.

The "-g$objectname" one is kind of clever, and definitely not something
I had thought of. We already have "--long", and of course we'd show the
long version for any name that isn't an exact match anyway. So as an
added bonus, it seems unlikely to surprise anybody who is expecting the
current "show the tag, not the refname" output (though again, this is
rare enough that I think people simply expect them to be the same ;) ).

-Peff

^ permalink raw reply

* Re: [dpdk-dev] [PATCH] net/mlx5: fix incorrect set VLAN ID action offset
From: Raslan Darawsheh @ 2020-02-20 11:25 UTC (permalink / raw)
  To: Suanming Mou, Slava Ovsiienko, Matan Azrad, Dekel Peled
  Cc: dev@dpdk.org, stable@dpdk.org
In-Reply-To: <1582121717-53976-1-git-send-email-suanmingm@mellanox.com>

Hi,

> -----Original Message-----
> From: Suanming Mou <suanmingm@mellanox.com>
> Sent: Wednesday, February 19, 2020 4:15 PM
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Matan Azrad
> <matan@mellanox.com>; Dekel Peled <dekelp@mellanox.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@mellanox.com>;
> stable@dpdk.org
> Subject: [PATCH] net/mlx5: fix incorrect set VLAN ID action offset
> 
> Currently, the set VLAN ID header modify action has already got the
> action pointer with offset from the header modify action array, but
> the configuration saves the detail to the memory of action with the
> offset again. It causes double offset to set the VLAN ID action to
> the wrong place in the header modify array.
> 
> Remove the offset when get the action pointer to fix that issue.
> 
> Fixes: 5f163d520cff ("net/mlx5: support modify VLAN ID on existing VLAN
> header")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
> Acked-by: Dekel Peled <dekelp@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow_dv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 359a037..5950274 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -637,7 +637,7 @@ struct field_modify_info modify_tcp[] = {
>  	const struct rte_flow_action_of_set_vlan_vid *conf =
>  		(const struct rte_flow_action_of_set_vlan_vid *)(action-
> >conf);
>  	int i = resource->actions_num;
> -	struct mlx5_modification_cmd *actions = &resource->actions[i];
> +	struct mlx5_modification_cmd *actions = resource->actions;
>  	struct field_modify_info *field = modify_vlan_out_first_vid;
> 
>  	if (i >= MLX5_MAX_MODIFY_NUM)
> --
> 1.8.3.1


Patch applied to next-net-mlx,
Kindest regards,
Raslan Darawsheh

^ permalink raw reply

* Re: [PATCH v1 13/13] migrate/ram: Tolerate partially changed mappings in postcopy code
From: David Hildenbrand @ 2020-02-20 11:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Andrea Arcangeli, Eduardo Habkost, Juan Quintela,
	Dr . David Alan Gilbert, Peter Xu, Paolo Bonzini,
	Richard Henderson
In-Reply-To: <20200219161725.115218-14-david@redhat.com>

On 19.02.20 17:17, David Hildenbrand wrote:
> When we partially change mappings (e.g., mmap over parts of an existing
> mmap) where we have a userfaultfd handler registered, the handler will
> implicitly be unregistered from the parts that changed. This is e.g., the
> case when doing a qemu_ram_remap(), but is also a preparation for RAM
> blocks with resizable allocations and we're shrinking RAM blocks.
> 
> When the mapping is changed and the handler is removed, any waiters are
> woken up. Trying to place pages will fail. We can simply ignore erors
> due to that when placing pages - as the mapping changed on the migration
> destination, also the content is stale. E.g., after shrinking a RAM
> block, nobody should be using that memory. After doing a
> qemu_ram_remap(), the old memory is expected to have vanished.
> 
> Let's tolerate such errors (but still warn for now) when placing pages.
> Also, add a comment why unregistering will continue to work even though
> the mapping might have changed.
> 
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  migration/postcopy-ram.c | 43 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index c68caf4e42..df9d27c004 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -506,6 +506,13 @@ static int cleanup_range(RAMBlock *rb, void *opaque)
>      range_struct.start = (uintptr_t)host_addr;
>      range_struct.len = length;
>  
> +    /*
> +     * In case the mapping was partially changed since we enabled userfault
> +     * (esp. when whrinking RAM blocks and we have resizable allocations, or
> +     * via qemu_ram_remap()), the userfaultfd handler was already removed for
> +     * the mappings that changed. Unregistering will, however, still work and
> +     * ignore mappings without a registered handler.
> +     */
>      if (ioctl(mis->userfault_fd, UFFDIO_UNREGISTER, &range_struct)) {
>          error_report("%s: userfault unregister %s", __func__, strerror(errno));
>  
> @@ -1239,10 +1246,28 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
>       */
>      if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize, rb)) {
>          int e = errno;
> -        error_report("%s: %s copy host: %p from: %p (size: %zd)",
> -                     __func__, strerror(e), host, from, pagesize);
>  
> -        return -e;
> +        /*
> +         * When the mapping gets partially changed before we try to place a page
> +         * (esp. when whrinking RAM blocks and we have resizable allocations, or
> +         * via qemu_ram_remap()), the userfaultfd handler will be removed and
> +         * placing pages will fail. In that case, any waiter was already woken
> +         * up when the mapping was changed. We can safely ignore this, as
> +         * mappings that change once we're running on the destination imply
> +         * that memory of these mappings vanishes. Let's still print a warning
> +         * for now.
> +         *

After talking to Andrea, on mapping changes, no waiter will be woken up
automatically. We have to do an UFFDIO_WAKE, which even works when there
is no longer a handler registered for that reason. Interesting stuff :)

-- 
Thanks,

David / dhildenb



^ permalink raw reply

* Re: [PATCHv5 29/34] drm/arm/malidp: Make verify funcitons invocations independent
From: Boris Brezillon @ 2020-02-20 11:26 UTC (permalink / raw)
  To: Andrzej Pietrasiewicz
  Cc: kernel, Mihail Atanassov, David Airlie, Liviu Dudau, Sandy Huang,
	dri-devel, James Wang, Ayan Halder, Sean Paul
In-Reply-To: <20191217145020.14645-30-andrzej.p@collabora.com>

In the subject: s/funcitons/functions/

On Tue, 17 Dec 2019 15:50:15 +0100
Andrzej Pietrasiewicz <andrzej.p@collabora.com> wrote:

> This will make it easier to transition to generic afbc-aware helpers.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/gpu/drm/arm/malidp_drv.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 37d92a06318e..961e5a3f5b08 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -362,10 +362,10 @@ static bool
>  malidp_verify_afbc_framebuffer(struct drm_device *dev, struct drm_file *file,
>  			       const struct drm_mode_fb_cmd2 *mode_cmd)
>  {
> -	if (malidp_verify_afbc_framebuffer_caps(dev, mode_cmd))
> -		return malidp_verify_afbc_framebuffer_size(dev, file, mode_cmd);
> +	if (!malidp_verify_afbc_framebuffer_caps(dev, mode_cmd))
> +		return false;
>  
> -	return false;
> +	return malidp_verify_afbc_framebuffer_size(dev, file, mode_cmd);
>  }
>  
>  static struct drm_framebuffer *

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH net-next v3 0/9] net: fix sysfs permssions when device changes network
From: Greg KH @ 2020-02-20 11:26 UTC (permalink / raw)
  To: David Miller
  Cc: christian.brauner, linux-kernel, netdev, rafael, pavel, kuba,
	edumazet, stephen, linux-pm
In-Reply-To: <20200219.162416.1910523123736311797.davem@davemloft.net>

On Wed, Feb 19, 2020 at 04:24:16PM -0800, David Miller wrote:
> From: Christian Brauner <christian.brauner@ubuntu.com>
> Date: Tue, 18 Feb 2020 17:29:34 +0100
> 
> > This is v3 with explicit uid and gid parameters added to functions that
> > change sysfs object ownership as Greg requested.
> 
> Greg, please review.

Give me a chance :)

It's looking better, still needs a little bit of work before I'm happy
with the driver core and sysfs bits, see my review comments so far.

thanks,

greg k-h

^ permalink raw reply

* [leo:next 3/3] drivers/soc/fsl/dpio/qbman-portal.c:869:14: warning: cast from pointer to integer of different size
From: kbuild test robot @ 2020-02-20 11:26 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3782 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/leo/linux.git next
head:   3b2abda7d28c69f564c1157b9b9c21ef40092ee9
commit: 3b2abda7d28c69f564c1157b9b9c21ef40092ee9 [3/3] soc: fsl: dpio: Replace QMAN array mode with ring mode enqueue
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.5.0-5) 7.5.0
reproduce:
        git checkout 3b2abda7d28c69f564c1157b9b9c21ef40092ee9
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/soc/fsl/dpio/qbman-portal.c: In function 'qbman_swp_enqueue_multiple_desc_direct':
>> drivers/soc/fsl/dpio/qbman-portal.c:869:14: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     addr_cena = (uint64_t)s->addr_cena;
                 ^

vim +869 drivers/soc/fsl/dpio/qbman-portal.c

   804	
   805	/**
   806	 * qbman_swp_enqueue_multiple_desc_direct() - Issue a multi enqueue command
   807	 * using multiple enqueue descriptor
   808	 * @s:  the software portal used for enqueue
   809	 * @d:  table of minimal enqueue descriptor
   810	 * @fd: table pointer of frame descriptor table to be enqueued
   811	 * @num_frames: number of fd to be enqueued
   812	 *
   813	 * Return the number of fd enqueued, or a negative error number.
   814	 */
   815	static
   816	int qbman_swp_enqueue_multiple_desc_direct(struct qbman_swp *s,
   817						   const struct qbman_eq_desc *d,
   818						   const struct dpaa2_fd *fd,
   819						   int num_frames)
   820	{
   821		uint32_t *p;
   822		const uint32_t *cl;
   823		uint32_t eqcr_ci, eqcr_pi, half_mask, full_mask;
   824		int i, num_enqueued = 0;
   825		uint64_t addr_cena;
   826	
   827		half_mask = (s->eqcr.pi_ci_mask>>1);
   828		full_mask = s->eqcr.pi_ci_mask;
   829		if (!s->eqcr.available) {
   830			eqcr_ci = s->eqcr.ci;
   831			p = s->addr_cena + QBMAN_CENA_SWP_EQCR_CI;
   832			s->eqcr.ci = qbman_read_register(s, QBMAN_CINH_SWP_EQCR_CI);
   833			s->eqcr.available = qm_cyc_diff(s->eqcr.pi_ring_size,
   834						eqcr_ci, s->eqcr.ci);
   835			if (!s->eqcr.available)
   836				return 0;
   837		}
   838	
   839		eqcr_pi = s->eqcr.pi;
   840		num_enqueued = (s->eqcr.available < num_frames) ?
   841				s->eqcr.available : num_frames;
   842		s->eqcr.available -= num_enqueued;
   843		/* Fill in the EQCR ring */
   844		for (i = 0; i < num_enqueued; i++) {
   845			p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi & half_mask));
   846			cl = (uint32_t *)(&d[i]);
   847			/* Skip copying the verb */
   848			memcpy(&p[1], &cl[1], EQ_DESC_SIZE_WITHOUT_FD - 1);
   849			memcpy(&p[EQ_DESC_SIZE_FD_START/sizeof(uint32_t)],
   850			       &fd[i], sizeof(*fd));
   851			eqcr_pi++;
   852		}
   853	
   854		dma_wmb();
   855	
   856		/* Set the verb byte, have to substitute in the valid-bit */
   857		eqcr_pi = s->eqcr.pi;
   858		for (i = 0; i < num_enqueued; i++) {
   859			p = (s->addr_cena + QBMAN_CENA_SWP_EQCR(eqcr_pi & half_mask));
   860			cl = (uint32_t *)(&d[i]);
   861			p[0] = cl[0] | s->eqcr.pi_vb;
   862			eqcr_pi++;
   863			if (!(eqcr_pi & half_mask))
   864				s->eqcr.pi_vb ^= QB_VALID_BIT;
   865		}
   866	
   867		/* Flush all the cacheline without load/store in between */
   868		eqcr_pi = s->eqcr.pi;
 > 869		addr_cena = (uint64_t)s->addr_cena;
   870		for (i = 0; i < num_enqueued; i++)
   871			eqcr_pi++;
   872		s->eqcr.pi = eqcr_pi & full_mask;
   873	
   874		return num_enqueued;
   875	}
   876	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 72054 bytes --]

^ permalink raw reply

* Re: [dpdk-dev] [PATCH 0/2] net/mlx5: fix incorrect layer choose with decap
From: Raslan Darawsheh @ 2020-02-20 11:26 UTC (permalink / raw)
  To: Suanming Mou, Slava Ovsiienko, Matan Azrad; +Cc: dev@dpdk.org
In-Reply-To: <1582122380-54467-1-git-send-email-suanmingm@mellanox.com>

Hi,

> -----Original Message-----
> From: Suanming Mou <suanmingm@mellanox.com>
> Sent: Wednesday, February 19, 2020 4:26 PM
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Matan Azrad
> <matan@mellanox.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@mellanox.com>
> Subject: [PATCH 0/2] net/mlx5: fix incorrect layer choose with decap
> 
> For header modify actions after decapsulation action, the header modify
> actions should be applied to the inner layers.
> 
> Currently, it always treats the outermost layers as the corresponding
> layer even though with decapsulation action.
> 
> Add the layer validation with decapsulation for the header modify action
> and fix the incorrect layers chosen in header modify action with
> decapsulation action.
> 
> Suanming Mou (2):
>   net/mlx5: fix wrong layer validation with decapsulation
>   net/mlx5: fix header modifiy choose wrong layer type
> 
>  drivers/net/mlx5/mlx5_flow_dv.c | 104
> +++++++++++++++++++++++++++++++++-------
>  1 file changed, 87 insertions(+), 17 deletions(-)
> 
> --
> 1.8.3.1


Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

^ permalink raw reply

* Re: [RESEND RFC PATCH v3] clk: Use new helper in managed functions
From: Greg Kroah-Hartman @ 2020-02-20 11:27 UTC (permalink / raw)
  To: Marc Gonzalez
  Cc: Stephen Boyd, Michael Turquette, Kuninori Morimoto, Russell King,
	Sudip Mukherjee, Dmitry Torokhov, Guenter Roeck, Bjorn Andersson,
	Robin Murphy, Geert Uytterhoeven, Arnd Bergmann, Ard Biesheuvel,
	Rafael Wysocki, Suzuki Poulose, Mark Rutland, linux-clk,
	Linux ARM, LKML
In-Reply-To: <f48d1df3-fc1f-ac5c-b11e-330f18aad539@free.fr>

On Thu, Feb 20, 2020 at 11:04:58AM +0100, Marc Gonzalez wrote:
> Introduce devm_add() to wrap devres_alloc() / devres_add() calls.
> 
> Using that helper produces simpler code, and smaller object size.
> E.g. with gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-gnu:
> 
>     text	   data	    bss	    dec	    hex	filename
> -   1708	     80	      0	   1788	    6fc	drivers/clk/clk-devres.o
> +   1524	     80	      0	   1604	    644	drivers/clk/clk-devres.o
> 
> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> ---
> Differences from v2 to v3
> x Make devm_add() return an error-code rather than the raw data pointer
>   (in case devres_alloc ever returns an ERR_PTR) as suggested by Geert
> x Provide a variadic version devm_vadd() to work with structs as suggested
>   by Geert
> x Don't use nested ifs in clk_devm* implementations (hopefully simpler
>   code logic to follow) as suggested by Geert
> 
> Questions:
> x This patch might need to be split in two? (Introduce the new API, then use it)
> x Convert other subsystems to show the value of this proposal?
> x Maybe comment the API usage somewhere
> ---
>  drivers/base/devres.c    | 15 ++++++
>  drivers/clk/clk-devres.c | 99 ++++++++++++++--------------------------
>  include/linux/device.h   |  3 ++
>  3 files changed, 53 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index 0bbb328bd17f..b2603789755b 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -685,6 +685,21 @@ int devres_release_group(struct device *dev, void *id)
>  }
>  EXPORT_SYMBOL_GPL(devres_release_group);
>  
> +int devm_add(struct device *dev, dr_release_t func, void *arg, size_t size)

Please add a bunch of kerneldoc here, as I have no idea what this
function does just by looking at the name of it :(

thanks,

greg k-h

^ permalink raw reply

* Re: [RESEND RFC PATCH v3] clk: Use new helper in managed functions
From: Greg Kroah-Hartman @ 2020-02-20 11:27 UTC (permalink / raw)
  To: Marc Gonzalez
  Cc: Mark Rutland, linux-clk, LKML, Arnd Bergmann, Kuninori Morimoto,
	Ard Biesheuvel, Stephen Boyd, Suzuki Poulose, Michael Turquette,
	Dmitry Torokhov, Rafael Wysocki, Russell King, Bjorn Andersson,
	Geert Uytterhoeven, Linux ARM, Robin Murphy, Sudip Mukherjee,
	Guenter Roeck
In-Reply-To: <f48d1df3-fc1f-ac5c-b11e-330f18aad539@free.fr>

On Thu, Feb 20, 2020 at 11:04:58AM +0100, Marc Gonzalez wrote:
> Introduce devm_add() to wrap devres_alloc() / devres_add() calls.
> 
> Using that helper produces simpler code, and smaller object size.
> E.g. with gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-gnu:
> 
>     text	   data	    bss	    dec	    hex	filename
> -   1708	     80	      0	   1788	    6fc	drivers/clk/clk-devres.o
> +   1524	     80	      0	   1604	    644	drivers/clk/clk-devres.o
> 
> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> ---
> Differences from v2 to v3
> x Make devm_add() return an error-code rather than the raw data pointer
>   (in case devres_alloc ever returns an ERR_PTR) as suggested by Geert
> x Provide a variadic version devm_vadd() to work with structs as suggested
>   by Geert
> x Don't use nested ifs in clk_devm* implementations (hopefully simpler
>   code logic to follow) as suggested by Geert
> 
> Questions:
> x This patch might need to be split in two? (Introduce the new API, then use it)
> x Convert other subsystems to show the value of this proposal?
> x Maybe comment the API usage somewhere
> ---
>  drivers/base/devres.c    | 15 ++++++
>  drivers/clk/clk-devres.c | 99 ++++++++++++++--------------------------
>  include/linux/device.h   |  3 ++
>  3 files changed, 53 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index 0bbb328bd17f..b2603789755b 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -685,6 +685,21 @@ int devres_release_group(struct device *dev, void *id)
>  }
>  EXPORT_SYMBOL_GPL(devres_release_group);
>  
> +int devm_add(struct device *dev, dr_release_t func, void *arg, size_t size)

Please add a bunch of kerneldoc here, as I have no idea what this
function does just by looking at the name of it :(

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Bitbake returning non-zero due to sstate errors
From: Paul Barker @ 2020-02-20 11:26 UTC (permalink / raw)
  To: Yocto discussion list

In my new CI setup I'm using an sstate mirror which seems to have some
occasional download issues. This results in the setscene task failing.
For example:

ERROR: qt3d-5.13.2+gitAUTOINC+93361f1a59-r0
do_package_write_ipk_setscene: Fetcher failure: Unable to find file
file://fd/sstate:qt3d:armv7at2hf-neon-linux-gnueabi:5.13.2+gitAUTOINC+93361f1a59:r0:armv7at2hf-neon:3:fda6c3edff0205b07ff176cf16771247117fa310bc65a6a1df6befc4230e0a74_package_write_ipk.tgz;downloadfilename=fd/sstate:qt3d:armv7at2hf-neon-linux-gnueabi:5.13.2+gitAUTOINC+93361f1a59:r0:armv7at2hf-neon:3:fda6c3edff0205b07ff176cf16771247117fa310bc65a6a1df6befc4230e0a74_package_write_ipk.tgz
anywhere. The paths that were searched were:
    /builds/SanCloudLtd/sancloud-arago/build/sstate-cache
    /builds/SanCloudLtd/sancloud-arago/build/sstate-cache
ERROR: qt3d-5.13.2+gitAUTOINC+93361f1a59-r0
do_package_write_ipk_setscene: No suitable staging package found
ERROR: Logfile of failure stored in:
/builds/SanCloudLtd/sancloud-arago/build/tmp/work/armv7at2hf-neon-linux-gnueabi/qt3d/5.13.2+gitAUTOINC+93361f1a59-r0/temp/log.do_package_write_ipk_setscene.10524
NOTE: recipe qt3d-5.13.2+gitAUTOINC+93361f1a59-r0: task
do_package_write_ipk_setscene: Failed
WARNING: Setscene task
(/builds/SanCloudLtd/sancloud-arago/sources/meta-qt5/recipes-qt/qt5/qt3d_git.bb:do_package_write_ipk_setscene)
failed with exit code '1' - real task will be run instead

As indicated in the final warning message there the real tasks run
since no sstate artifact is available. These tasks succeed:

NOTE: recipe qt3d-5.13.2+gitAUTOINC+93361f1a59-r0: task
do_package_write_ipk: Succeeded

The result is a successful build of the desired images. However, the
build is marked as a failure due to those sstate errors:

Summary: There were 11 ERROR messages shown, returning a non-zero exit code.

Is this the expected behaviour? The final images are built correctly.
I can't see any simple way to mask those setscene errors but I might
be missing something.

The full log can be seen at
https://gitlab.com/SanCloudLtd/sancloud-arago/-/jobs/443901140/raw.
I'm on the zeus branch here, I'll try to re-test on master later if I
can.

Thanks,
Paul

^ permalink raw reply

* Re: [PATCH v3 09/17] s390: protvirt: Move STSI data over SIDAD
From: Janosch Frank @ 2020-02-20 11:25 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: qemu-s390x, mihajlov, qemu-devel, david
In-Reply-To: <20200220115401.50658d2c.cohuck@redhat.com>


[-- Attachment #1.1: Type: text/plain, Size: 1944 bytes --]

On 2/20/20 11:54 AM, Cornelia Huck wrote:
> On Fri, 14 Feb 2020 10:16:28 -0500
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> For protected guests, we need to put the STSI emulation results into
>> the SIDA, so SIE will write them into the guest at the next entry.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  target/s390x/kvm.c | 15 ++++++++++++---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index eec0b92479..fe669ed24c 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -1772,11 +1772,16 @@ static int handle_tsch(S390CPU *cpu)
>>  
>>  static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
>>  {
>> +    CPUS390XState *env = &cpu->env;
>>      SysIB_322 sysib;
>>      int del;
>>  
>> -    if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib, sizeof(sysib))) {
>> -        return;
>> +    if (env->pv) {
>> +        s390_cpu_pv_mem_read(cpu, 0, &sysib, sizeof(sysib));
> 
> This is only introduced by the next patch, right?

Ups, time to reorder.

> 
>> +    } else {
>> +        if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib, sizeof(sysib))) {
>> +            return;
>> +        }
>>      }
>>      /* Shift the stack of Extended Names to prepare for our own data */
>>      memmove(&sysib.ext_names[1], &sysib.ext_names[0],
>> @@ -1815,7 +1820,11 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
>>      /* Insert UUID */
>>      memcpy(sysib.vm[0].uuid, &qemu_uuid, sizeof(sysib.vm[0].uuid));
>>  
>> -    s390_cpu_virt_mem_write(cpu, addr, ar, &sysib, sizeof(sysib));
>> +    if (env->pv) {
>> +        s390_cpu_pv_mem_write(cpu, 0, &sysib, sizeof(sysib));
>> +    } else {
>> +        s390_cpu_virt_mem_write(cpu, addr, ar, &sysib, sizeof(sysib));
>> +    }
>>  }
>>  
>>  static int handle_stsi(S390CPU *cpu)
> 
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: Race condition in overlayed qcow2?
From: Vladimir Sementsov-Ogievskiy @ 2020-02-20 11:26 UTC (permalink / raw)
  To: Pavel Dovgalyuk; +Cc: kwolf, qemu-devel, mreitz
In-Reply-To: <003701d5e7d4$90bcc130$b2364390$@ru>

20.02.2020 13:00, Pavel Dovgalyuk wrote:
>> From: Vladimir Sementsov-Ogievskiy [mailto:vsementsov@virtuozzo.com]
>> 20.02.2020 11:31, dovgaluk wrote:
>>> Vladimir Sementsov-Ogievskiy писал 2020-02-19 19:07:
>>>> 19.02.2020 17:32, dovgaluk wrote:
>>>>> I encountered a problem with record/replay of QEMU execution and figured out the
>> following, when
>>>>> QEMU is started with one virtual disk connected to the qcow2 image with applied 'snapshot'
>> option.
>>>>>
>>>>> The patch d710cf575ad5fb3ab329204620de45bfe50caa53 "block/qcow2: introduce parallel
>> subrequest handling in read and write"
>>>>> introduces some kind of race condition, which causes difference in the data read from the
>> disk.
>>>>>
>>>>> I detected this by adding the following code, which logs IO operation checksum. And this
>> checksum may be different in different runs of the same recorded execution.
>>>>>
>>>>> logging in blk_aio_complete function:
>>>>>           qemu_log("%"PRId64": blk_aio_complete\n", replay_get_current_icount());
>>>>>           QEMUIOVector *qiov = acb->rwco.iobuf;
>>>>>           if (qiov && qiov->iov) {
>>>>>               size_t i, j;
>>>>>               uint64_t sum = 0;
>>>>>               int count = 0;
>>>>>               for (i = 0 ; i < qiov->niov ; ++i) {
>>>>>                   for (j = 0 ; j < qiov->iov[i].iov_len ; ++j) {
>>>>>                       sum += ((uint8_t*)qiov->iov[i].iov_base)[j];
>>>>>                       ++count;
>>>>>                   }
>>>>>               }
>>>>>               qemu_log("--- iobuf offset %"PRIx64" len %x sum: %"PRIx64"\n", acb-
>>> rwco.offset, count, sum);
>>>>>           }
>>>>>
>>>>> I tried to get rid of aio task by patching qcow2_co_preadv_part:
>>>>> ret = qcow2_co_preadv_task(bs, ret, cluster_offset, offset, cur_bytes, qiov, qiov_offset);
>>>>>
>>>>> That change fixed a bug, but I have no idea what to debug next to figure out the exact
>> reason of the failure.
>>>>>
>>>>> Do you have any ideas or hints?
>>>>>
>>>>
>>>> Hi!
>>>>
>>>> Hmm, do mean that read from the disk may return wrong data? It would
>>>> be very bad of course :(
>>>> Could you provide a reproducer, so that I can look at it and debug?
>>>
>>> It is just a winxp-32 image. I record the execution and replay it with the following command
>> lines:
>>>
>>> qemu-system-i386 -icount shift=7,rr=record,rrfile=replay.bin -m 512M -drive
>> file=xp.qcow2,if=none,id=device-34-file,snapshot -drive driver=blkreplay,if=none,image=device-
>> 34-file,id=device-34-driver -device ide-hd,drive=device-34-driver,bus=ide.0,id=device-34 -net
>> none
>>>
>>> qemu-system-i386 -icount shift=7,rr=replay,rrfile=replay.bin -m 512M -drive
>> file=xp.qcow2,if=none,id=device-34-file,snapshot -drive driver=blkreplay,if=none,image=device-
>> 34-file,id=device-34-driver -device ide-hd,drive=device-34-driver,bus=ide.0,id=device-34 -net
>> none
>>>
>>> Replay stalls at some moment due to the non-determinism of the execution (probably caused by
>> the wrong data read).
>>
>> Hmm.. I tried it  (with x86_64 qemu and centos image). I waited for some time for a first
>> command, than Ctrl+C it. After it replay.bin was 4M. Than started the second command. It
>> works, not failing, not finishing. Is it bad? What is expected behavior and what is wrong?
> 
> The second command should finish. There is no replay introspection yet (in master), but you can
> stop qemu with gdb and inspect replay_state.current_icount field. It should increase with every
> virtual CPU instruction execution. If that counter has stopped, it means that replay hangs.

It hangs for me even with QCOW2_MAX_WORKERS = 1..

> 
>>>> What is exactly the case? May be you have other parallel aio
>>>> operations to the same region?
>>>
>>> As far as I understand, all aio operations, initiated by IDE controller, are performed one-
>> by-one.
>>> I don't see anything else in the logs.
>>>
>>>> Ideas to experiment:
>>>>
>>>> 1. change QCOW2_MAX_WORKERS to 1 or to 2, will it help?
>>>
>>> 1 or 2 are ok, and 4 or 8 lead to the failures.
>>>
>>>> 2. understand what is the case in code: is it read from one or several
>>>> clusters, is it aligned,
>>>> what is the type of clusters, is encryption in use, compression?
>>>
>>> There is no encryption and I thinks compression is not enabled too.
>>> Clusters are read from the temporary overlay:
>>>
>>> blk_aio_prwv
>>> blk_aio_read_entry
>>> bdrv_co_preadv_part complete offset: 26300000 qiov_offset: 1c200 len: 1e00
>>> bdrv_co_preadv_part complete offset: 24723e00 qiov_offset: 0 len: 1c200
>>> bdrv_co_preadv_part complete offset: c0393e00 qiov_offset: 0 len: 1e000
>>> bdrv_co_preadv_part complete offset: c0393e00 qiov_offset: 0 len: 1e000
>>> bdrv_co_preadv_part complete offset: c0393e00 qiov_offset: 0 len: 1e000
>>>
>>>
>>>> 3. understand what kind of data corruption. What we read instead of
>>>> correct data? Just garbage, or may be zeroes, or what..
>>>
>>> Most bytes are the same, but some are different:
>>>
>>> < 00 00 00 00 00 00 00 00 00 00 00 00 00 00 02 00
>>> < 46 49 4c 45 30 00 03 00 18 d1 33 02 00 00 00 00
>>> < 01 00 01 00 38 00 01 00 68 01 00 00 00 04 00 00
>>> < 00 00 00 00 00 00 00 00 04 00 00 00 9d 0e 00 00
>>> < 02 00 00 00 00 00 00 00 10 00 00 00 60 00 00 00
>>> ---
>>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 04 00
>>>> 46 49 4c 45 30 00 03 00 86 78 35 03 00 00 00 00
>>>> 01 00 01 00 38 00 01 00 60 01 00 00 00 04 00 00
>>>> 00 00 00 00 00 00 00 00 04 00 00 00 a1 0e 00 00
>>>> 04 00 00 00 00 00 00 00 10 00 00 00 60 00 00 00
>>>
>>> That is strange. I could think, that it was caused by the bugs in
>>> deterministic CPU execution, but the first difference in logs
>>> occur in READ operation (I dump read/write buffers in blk_aio_complete).
>>>
>>
>> Aha, yes, looks strange.
>>
>> Then next steps:
>>
>> 1. Does problem hit into the same offset every time?
> 
> Yes, almost the same offset, almost the same phase of the execution.
> 
>> 2. Do we write to this region before this strange read?
> 
> No.
> 
>> 2.1. If yes, we need to check that we read what we write.. You say you dump buffers
>> in blk_aio_complete... I think it would be more reliable to dump at start of
>> bdrv_co_pwritev and at end of bdrv_co_preadv. Also, guest may modify its buffers
>> during operation which would be strange but possible.
> 
> I dumped every write in file-posix.c handle_aiocb_rw_linear and qemu_pwritev
> and found no difference in executions.
> 
>> 2.2 If not, hmm...
> 
> Exactly.
> 
> Pavel Dovgalyuk
> 


-- 
Best regards,
Vladimir


^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.