* [PATCH v4 7/8] dt-bindings: iio: document envelope-detector bindings
From: Peter Rosin @ 2016-11-08 11:58 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Daniel Baluta,
Slawomir Stepien, Thomas Gleixner, linux-iio, devicetree
In-Reply-To: <1478606339-31253-1-git-send-email-peda@axentia.se>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
.../bindings/iio/adc/envelope-detector.txt | 54 ++++++++++++++++++++++
MAINTAINERS | 6 +++
2 files changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
new file mode 100644
index 000000000000..27544bdd4478
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
@@ -0,0 +1,54 @@
+Bindings for ADC envelope detector using a DAC and a comparator
+
+The DAC is used to find the peak level of an alternating voltage input
+signal by a binary search using the output of a comparator wired to
+an interrupt pin. Like so:
+ _
+ | \
+ input +------>-------|+ \
+ | \
+ .-------. | }---.
+ | | | / |
+ | dac|-->--|- / |
+ | | |_/ |
+ | | |
+ | | |
+ | irq|------<-------'
+ | |
+ '-------'
+
+Required properties:
+- compatible: Should be "axentia,tse850-envelope-detector"
+- io-channels: Channel node of the dac to be used for comparator input.
+- io-channel-names: Should be "dac".
+- interrupt specification for one client interrupt,
+ see ../../interrupt-controller/interrupts.txt for details.
+- interrupt-names: Should be "comp".
+
+Example:
+
+ &i2c {
+ dpot: mcp4651-104@28 {
+ compatible = "microchip,mcp4651-104";
+ reg = <0x28>;
+ #io-channel-cells = <1>;
+ };
+ };
+
+ dac: dac {
+ compatible = "dpot-dac";
+ vref-supply = <®_3v3>;
+ io-channels = <&dpot 0>;
+ io-channel-names = "dpot";
+ #io-channel-cells = <1>;
+ };
+
+ envelope-detector {
+ compatible = "axentia,tse850-envelope-detector";
+ io-channels = <&dac 0>;
+ io-channel-names = "dac";
+
+ interrupt-parent = <&gpio>;
+ interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-names = "comp";
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 583c6c93b6f3..0e13066ca3a2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6132,6 +6132,12 @@ F: Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
F: Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
F: drivers/iio/dac/dpot-dac.c
+IIO ENVELOPE DETECTOR
+M: Peter Rosin <peda@axentia.se>
+L: linux-iio@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
+
IIO SUBSYSTEM AND DRIVERS
M: Jonathan Cameron <jic23@kernel.org>
R: Hartmut Knaack <knaack.h@gmx.de>
--
2.1.4
^ permalink raw reply related
* [PATCH v4 2/8] iio: inkern: add helpers to query available values from channels
From: Peter Rosin @ 2016-11-08 11:58 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Daniel Baluta,
Slawomir Stepien, Thomas Gleixner, linux-iio, devicetree
In-Reply-To: <1478606339-31253-1-git-send-email-peda@axentia.se>
Specifically a helper for reading the available maximum raw value of a
channel and a helper for forwarding read_avail requests for raw values
from one iio driver to an iio channel that is consumed.
These rather specific helpers are in turn built with generic helpers
making it easy to build more helpers for available values as needed.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/iio/inkern.c | 104 +++++++++++++++++++++++++++++++++++++++++++
include/linux/iio/consumer.h | 28 ++++++++++++
include/linux/iio/iio.h | 17 +++++++
3 files changed, 149 insertions(+)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c4757e6367e7..cfca17ba2535 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -703,6 +703,110 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val, int *val2)
}
EXPORT_SYMBOL_GPL(iio_read_channel_scale);
+static int iio_channel_read_avail(struct iio_channel *chan,
+ const int **vals, int *type, int *length,
+ enum iio_chan_info_enum info)
+{
+ if (!iio_channel_has_available(chan->channel, info))
+ return -EINVAL;
+
+ return chan->indio_dev->info->read_avail(chan->indio_dev, chan->channel,
+ vals, type, length, info);
+}
+
+int iio_read_avail_channel_raw(struct iio_channel *chan,
+ const int **vals, int *length)
+{
+ int ret;
+ int type;
+
+ mutex_lock(&chan->indio_dev->info_exist_lock);
+ if (!chan->indio_dev->info) {
+ ret = -ENODEV;
+ goto err_unlock;
+ }
+
+ ret = iio_channel_read_avail(chan,
+ vals, &type, length, IIO_CHAN_INFO_RAW);
+err_unlock:
+ mutex_unlock(&chan->indio_dev->info_exist_lock);
+
+ if (ret >= 0 && type != IIO_VAL_INT) {
+ /* raw values are assumed to be IIO_VAL_INT */
+ ret = -EINVAL;
+ goto err_unlock;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iio_read_avail_channel_raw);
+
+static int iio_channel_read_max(struct iio_channel *chan,
+ int *val, int *val2, int *type,
+ enum iio_chan_info_enum info)
+{
+ int unused;
+ const int *vals;
+ int length;
+ int ret;
+
+ if (!val2)
+ val2 = &unused;
+
+ ret = iio_channel_read_avail(chan, &vals, type, &length, info);
+ switch (ret) {
+ case IIO_AVAIL_RANGE:
+ switch (*type) {
+ case IIO_VAL_INT:
+ *val = vals[2];
+ break;
+ default:
+ *val = vals[4];
+ *val2 = vals[5];
+ }
+ return 0;
+
+ case IIO_AVAIL_LIST:
+ if (length <= 0)
+ return -EINVAL;
+ switch (*type) {
+ case IIO_VAL_INT:
+ *val = vals[--length];
+ while (length) {
+ if (vals[--length] > *val)
+ *val = vals[length];
+ }
+ break;
+ default:
+ /* FIXME: learn about max for other iio values */
+ return -EINVAL;
+ }
+ return 0;
+
+ default:
+ return ret;
+ }
+}
+
+int iio_read_max_channel_raw(struct iio_channel *chan, int *val)
+{
+ int ret;
+ int type;
+
+ mutex_lock(&chan->indio_dev->info_exist_lock);
+ if (!chan->indio_dev->info) {
+ ret = -ENODEV;
+ goto err_unlock;
+ }
+
+ ret = iio_channel_read_max(chan, val, NULL, &type, IIO_CHAN_INFO_RAW);
+err_unlock:
+ mutex_unlock(&chan->indio_dev->info_exist_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iio_read_max_channel_raw);
+
int iio_get_channel_type(struct iio_channel *chan, enum iio_chan_type *type)
{
int ret = 0;
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index 9edccfba1ffb..9a4f336d8b4a 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -226,6 +226,34 @@ int iio_read_channel_processed(struct iio_channel *chan, int *val);
int iio_write_channel_raw(struct iio_channel *chan, int val);
/**
+ * iio_read_max_channel_raw() - read maximum available raw value from a given
+ * channel, i.e. the maximum possible value.
+ * @chan: The channel being queried.
+ * @val: Value read back.
+ *
+ * Note raw reads from iio channels are in adc counts and hence
+ * scale will need to be applied if standard units are required.
+ */
+int iio_read_max_channel_raw(struct iio_channel *chan, int *val);
+
+/**
+ * iio_read_avail_channel_raw() - read available raw values from a given channel
+ * @chan: The channel being queried.
+ * @vals: Available values read back.
+ * @length: Number of entries in vals.
+ *
+ * Returns an error code, IIO_AVAIL_RANGE or IIO_AVAIL_LIST.
+ *
+ * For ranges, three vals are always returned; min, step and max.
+ * For lists, all the possible values are enumerated.
+ *
+ * Note raw available values from iio channels are in adc counts and
+ * hence scale will need to be applied if standard units are required.
+ */
+int iio_read_avail_channel_raw(struct iio_channel *chan,
+ const int **vals, int *length);
+
+/**
* iio_get_channel_type() - get the type of a channel
* @channel: The channel being queried.
* @type: The type of the channel.
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 45b781084a4b..f9be8836ef23 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -315,6 +315,23 @@ static inline bool iio_channel_has_info(const struct iio_chan_spec *chan,
(chan->info_mask_shared_by_all & BIT(type));
}
+/**
+ * iio_channel_has_available() - Checks if a channel has an available attribute
+ * @chan: The channel to be queried
+ * @type: Type of the available attribute to be checked
+ *
+ * Returns true if the channel supports reporting available values for the
+ * given attribute type, false otherwise.
+ */
+static inline bool iio_channel_has_available(const struct iio_chan_spec *chan,
+ enum iio_chan_info_enum type)
+{
+ return (chan->info_mask_separate_available & BIT(type)) |
+ (chan->info_mask_shared_by_type_available & BIT(type)) |
+ (chan->info_mask_shared_by_dir_available & BIT(type)) |
+ (chan->info_mask_shared_by_all_available & BIT(type));
+}
+
#define IIO_CHAN_SOFT_TIMESTAMP(_si) { \
.type = IIO_TIMESTAMP, \
.channel = -1, \
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] nfsd: constify reply_cache_stats_operations structure
From: J. Bruce Fields @ 2016-11-08 20:36 UTC (permalink / raw)
To: Jeff Layton; +Cc: Julia Lawall, kernel-janitors, linux-nfs, linux-kernel
In-Reply-To: <1472468267.2597.0.camel@poochiereds.net>
On Mon, Aug 29, 2016 at 06:57:47AM -0400, Jeff Layton wrote:
> On Sun, 2016-08-28 at 22:36 +0200, Julia Lawall wrote:
> > reply_cache_stats_operations, of type struct file_operations, is
> > never
> > modified, so declare it as const.
> >
> > Done with the help of Coccinelle.
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > ---
> > fs/nfsd/nfsctl.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > index 65ad016..3bc9693 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -217,7 +217,7 @@ static const struct file_operations
> > pool_stats_operations = {
> > .release = nfsd_pool_stats_release,
> > };
> >
> > -static struct file_operations reply_cache_stats_operations = {
> > +static const struct file_operations reply_cache_stats_operations = {
> > .open = nfsd_reply_cache_stats_open,
> > .read = seq_read,
> > .llseek = seq_lseek,
> >
>
> I'm pretty sure Bruce will pick this up for v4.9:
Except, for some reason it's been languishing in my inbox. Apologies,
applying....
--b.
>
> Reviewed-by: Jeff Layton <jlayton@poochiereds.net>
^ permalink raw reply
* Re: [PATCH 1/3] powerpc: add NMI IPI infrastructure
From: kbuild test robot @ 2016-11-08 20:35 UTC (permalink / raw)
To: Nicholas Piggin
Cc: kbuild-all, linuxppc-dev, Alistair Popple, Nicholas Piggin
In-Reply-To: <20161108140125.21455-2-npiggin@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1751 bytes --]
Hi Nicholas,
[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.9-rc4]
[cannot apply to next-20161108]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-NMI-IPIs/20161109-005049
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
Note: the linux-review/Nicholas-Piggin/powerpc-NMI-IPIs/20161109-005049 HEAD ebfcf2d2f068a2e00d385864b5ba9371e15adcef builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
arch/powerpc/kernel/smp.c: In function 'nmi_ipi_safe_action':
>> arch/powerpc/kernel/smp.c:160:2: error: implicit declaration of function 'smp_handle_nmi_ipi' [-Werror=implicit-function-declaration]
smp_handle_nmi_ipi(get_irq_regs());
^~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/smp_handle_nmi_ipi +160 arch/powerpc/kernel/smp.c
154 tick_broadcast_ipi_handler();
155 return IRQ_HANDLED;
156 }
157
158 static irqreturn_t nmi_ipi_safe_action(int irq, void *data)
159 {
> 160 smp_handle_nmi_ipi(get_irq_regs());
161 return IRQ_HANDLED;
162 }
163
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22506 bytes --]
^ permalink raw reply
* Re: what is infernalis' EOL date?
From: Sage Weil @ 2016-11-08 20:36 UTC (permalink / raw)
To: Samuel Just; +Cc: Ken Dreyer, ceph-devel
In-Reply-To: <CAN=+7FVhwt9AfRgjLKZ1w_Z2WtNVa4kMzjndm8AfvuDjZ1GhrA@mail.gmail.com>
On Tue, 8 Nov 2016, Samuel Just wrote:
> I think the EOL would have been the date of the Jewel release, right?
Yeah
> -Sam
>
> On Tue, Nov 8, 2016 at 8:50 AM, Ken Dreyer <kdreyer@redhat.com> wrote:
> > Hi folks,
> >
> > http://docs.ceph.com/docs/master/releases/ does not list an EOL date
> > for Infernalis.
> >
> > Can we say "Feb 2016" there? That was the date of the last point
> > release, v9.2.1.
> >
> > - Ken
> > --
> > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply
* linux-next: bad commit in the omap tree
From: Stephen Rothwell @ 2016-11-08 20:37 UTC (permalink / raw)
To: Tony Lindgren
Cc: linux-next, linux-kernel, Santosh Shilimkar, Nishanth Menon,
Keerthy
Hi Tony,
Commit 63fdf6527272 ("ARM: OMAP5: Add basic cpuidle MPU CSWR support")
in the omap tree has no Signed-off-by from you as the committer.
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 3/3] Plumb the HAXM-based hardware acceleration support
From: Paolo Bonzini @ 2016-11-08 20:37 UTC (permalink / raw)
To: Vincent Palatin, qemu-devel
In-Reply-To: <3d8be595c30d0009464ce16e90ee8fb5451e5605.1478619442.git.vpalatin@chromium.org>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 4188fed..4bd238b 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
All this should not be needed anymore with unrestricted guest support.
> diff --git a/cpus.c b/cpus.c
> index fc78502..6e0f572 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -35,6 +35,7 @@
> #include "sysemu/dma.h"
> #include "sysemu/hw_accel.h"
> #include "sysemu/kvm.h"
> +#include "sysemu/hax.h"
> #include "qmp-commands.h"
> #include "exec/exec-all.h"
>
> @@ -1221,6 +1222,52 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
> return NULL;
> }
>
> +static void *qemu_hax_cpu_thread_fn(void *arg)
> +{
> + CPUState *cpu = arg;
> + int r;
> + qemu_thread_get_self(cpu->thread);
> + qemu_mutex_lock(&qemu_global_mutex);
> +
> + cpu->thread_id = qemu_get_thread_id();
> + cpu->created = true;
> + cpu->halted = 0;
> + current_cpu = cpu;
> +
> + hax_init_vcpu(cpu);
> + qemu_cond_signal(&qemu_cpu_cond);
> +
> + while (1) {
> + if (cpu_can_run(cpu)) {
> + r = hax_smp_cpu_exec(cpu);
> + if (r == EXCP_DEBUG) {
> + cpu_handle_guest_debug(cpu);
> + }
> + }
> +
> + while (cpu_thread_is_idle(cpu)) {
> + qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex);
> + }
> +
> + qemu_wait_io_event_common(cpu);
> + }
> + return NULL;
> +}
> +
> +
> +static void qemu_cpu_kick_no_halt(void)
> +{
> + CPUState *cpu;
> + /* Ensure whatever caused the exit has reached the CPU threads before
> + * writing exit_request.
> + */
> + atomic_mb_set(&exit_request, 1);
> + cpu = atomic_mb_read(&tcg_current_cpu);
> + if (cpu) {
> + cpu_exit(cpu);
> + }
> +}
> +
> static void qemu_cpu_kick_thread(CPUState *cpu)
> {
> #ifndef _WIN32
> @@ -1235,28 +1282,52 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
> fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
> exit(1);
> }
> -#else /* _WIN32 */
> - abort();
> -#endif
> -}
>
> -static void qemu_cpu_kick_no_halt(void)
> -{
> - CPUState *cpu;
> - /* Ensure whatever caused the exit has reached the CPU threads before
> - * writing exit_request.
> +#ifdef CONFIG_DARWIN
> + /* The cpu thread cannot catch it reliably when shutdown the guest on Mac.
> + * We can double check it and resend it
> */
> - atomic_mb_set(&exit_request, 1);
> - cpu = atomic_mb_read(&tcg_current_cpu);
> - if (cpu) {
> - cpu_exit(cpu);
> + if (!exit_request)
> + qemu_cpu_kick_no_halt();
This must be a conflict resolved wrong. exit_request is never read by
the HAX code.
> + if (hax_enabled() && hax_ug_platform())
> + cpu->exit_request = 1;
> +#endif
> +#else /* _WIN32 */
> + if (!qemu_cpu_is_self(cpu)) {
> + CONTEXT tcgContext;
> +
> + if (SuspendThread(cpu->hThread) == (DWORD)-1) {
> + fprintf(stderr, "qemu:%s: GetLastError:%lu\n", __func__,
> + GetLastError());
> + exit(1);
> + }
> +
> + /* On multi-core systems, we are not sure that the thread is actually
> + * suspended until we can get the context.
> + */
> + tcgContext.ContextFlags = CONTEXT_CONTROL;
> + while (GetThreadContext(cpu->hThread, &tcgContext) != 0) {
> + continue;
> + }
> +
> + qemu_cpu_kick_no_halt();
> + if (hax_enabled() && hax_ug_platform())
> + cpu->exit_request = 1;
> +
> + if (ResumeThread(cpu->hThread) == (DWORD)-1) {
> + fprintf(stderr, "qemu:%s: GetLastError:%lu\n", __func__,
> + GetLastError());
> + exit(1);
> + }
This is weird too. The SuspendThread/ResumeThread dance comes from an
old version of QEMU. It is not needed anymore and, again,
qemu_cpu_kick_no_halt would only be useful if hax_ug_platform() is false.
Here, Linux/KVM uses a signal and pthread_kill. It's probably good for
HAX on Darwin too, but not on Windows. It's possible that
SuspendThread/ResumeThread just does the right thing (sort of by
chance), in which case you can just keep it (removing
qemu_cpu_kick_no_halt). However, there is a hax_raise_event in patch 2
that is unused. If you can figure out how to use it it would be better.
> @@ -1617,6 +1618,21 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
> } else {
> new_block->host = phys_mem_alloc(new_block->max_length,
> &new_block->mr->align);
> + /*
> + * In Hax, the qemu allocate the virtual address, and HAX kernel
> + * populate the memory with physical memory. Currently we have no
> + * paging, so user should make sure enough free memory in advance
> + */
> + if (hax_enabled()) {
> + int ret;
> + ret = hax_populate_ram((uint64_t)(uintptr_t)new_block->host,
> + new_block->max_length);
> + if (ret < 0) {
> + error_setg(errp, "Hax failed to populate ram");
> + return;
> + }
> + }
Please try removing this block and instead starting QEMU with
-mem-prealloc. If it works, remove hax_populate_ram and just set
mem_prealloc to 1 in hax_accel_init.
> if (!new_block->host) {
> error_setg_errno(errp, errno,
> "cannot set up guest memory '%s'",
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index d78c885..dd4cdc8 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -316,9 +316,10 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
>
> /* Note: We need at least 1M to map the VAPIC option ROM */
> if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
> - ram_size >= 1024 * 1024) {
> + kvm_enabled() && ram_size >= 1024 * 1024) {
This should rather be !hax_enabled(), despite the historical name
mentions KVM.
> vapic = sysbus_create_simple("kvmvapic", -1, NULL);
> }
> +
> s->vapic = vapic;
> if (apic_report_tpr_access && info->enable_tpr_reporting) {
> info->enable_tpr_reporting(s, true);
> diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
> index fb79f31..25b6003 100644
> --- a/target-i386/seg_helper.c
> +++ b/target-i386/seg_helper.c
> @@ -25,6 +25,7 @@
> #include "exec/exec-all.h"
> #include "exec/cpu_ldst.h"
> #include "exec/log.h"
> +#include "sysemu/hax.h"
>
> //#define DEBUG_PCALL
>
> @@ -1336,6 +1337,10 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> !(env->hflags & HF_SMM_MASK)) {
> cpu_svm_check_intercept_param(env, SVM_EXIT_SMI, 0);
> cs->interrupt_request &= ~CPU_INTERRUPT_SMI;
> +#ifdef CONFIG_HAX
> + if (hax_enabled())
> + cs->hax_vcpu->resync = 1;
> +#endif
Not needed for UG mode.
> do_smm_enter(cpu);
> ret = true;
> } else if ((interrupt_request & CPU_INTERRUPT_NMI) &&
> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index 324103c..e027896 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
Same.
> @@ -4060,6 +4066,7 @@ int main(int argc, char **argv, char **envp)
> machine_class = select_machine();
>
> set_memory_options(&ram_slots, &maxram_size, machine_class);
> + hax_pre_init(ram_size);
It should be possible to merge this with hax_accel_init.
> os_daemonize();
>
> @@ -4418,8 +4425,8 @@ int main(int argc, char **argv, char **envp)
>
> cpu_ticks_init();
> if (icount_opts) {
> - if (kvm_enabled() || xen_enabled()) {
> - error_report("-icount is not allowed with kvm or xen");
> + if (kvm_enabled() || xen_enabled() || hax_enabled()) {
> + error_report("-icount is not allowed with kvm or xen or hax");
Let's say it's only allowed with TCG. :) Again, thanks to UG mode if
hax_enabled() you'll have !tcg_enabled().
Paolo
> exit(1);
> }
> configure_icount(icount_opts, &error_abort);
> @@ -4555,6 +4562,10 @@ int main(int argc, char **argv, char **envp)
>
> numa_post_machine_init();
>
> + if (hax_enabled()) {
> + hax_sync_vcpus();
> + }
> +
> if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
> parse_fw_cfg, fw_cfg_find(), NULL) != 0) {
> exit(1);
>
^ permalink raw reply
* Re: [RFC][PATCH] usb: dwc2: Make sure we disconnect the gadget state on reset
From: John Stultz @ 2016-11-08 20:37 UTC (permalink / raw)
To: Felipe Balbi
Cc: lkml, Wei Xu, Guodong Xu, Chen Yu, Amit Pundir, Rob Herring,
Mark Rutland, John Youn, Douglas Anderson, Greg Kroah-Hartman,
Linux USB List
In-Reply-To: <87mvhkrckz.fsf@linux.intel.com>
On Mon, Oct 31, 2016 at 4:18 AM, Felipe Balbi
<felipe.balbi@linux.intel.com> wrote:
> John Stultz <john.stultz@linaro.org> writes:
>> I had seen some odd behavior with HiKey's usb-gadget interface
>> that I finally seemed to have chased down. Basically every other
>> time I pluged in the OTG port, the gadget interface would
>> properly initialize. The other times, I'd get a big WARN_ON
>> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>>
>> Ends up If we don't disconnect the gadget state on reset, the
>> fifo-map doesn't get cleared properly, which causes WARN_ON
>> messages and also results in the device not properly being
>> setup as a gadget every other time the OTG port is connected.
>>
>> So this patch adds a call to dwc2_hsotg_disconnect() in the
>> reset path so the state is properly cleared.
>>
>> With it, the gadget interface initializes properly on every
>> plug in.
>>
>> I don't know if this is actually the right fix, but it seems
>> to work well. Feedback would be greatly appreciated!
>>
[snip]
>> ---
>> drivers/usb/dwc2/gadget.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 24fbebc..5505001 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -2519,6 +2519,8 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg,
>>
>> /* Kill any ep0 requests as controller will be reinitialized */
>> kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
>> + /* Make sure everything is disconnected */
>> + dwc2_hsotg_disconnect(hsotg);
>
> Dunno, seems like you're actually working around a different
> bug. Looking at USB Reset handler we have:
>
> if (gintsts & (GINTSTS_USBRST | GINTSTS_RESETDET)) {
>
> u32 usb_status = dwc2_readl(hsotg->regs + GOTGCTL);
> u32 connected = hsotg->connected;
>
> dev_dbg(hsotg->dev, "%s: USBRst\n", __func__);
> dev_dbg(hsotg->dev, "GNPTXSTS=%08x\n",
> dwc2_readl(hsotg->regs + GNPTXSTS));
>
> dwc2_writel(GINTSTS_USBRST, hsotg->regs + GINTSTS);
>
> /* Report disconnection if it is not already done. */
> dwc2_hsotg_disconnect(hsotg);
>
> if (usb_status & GOTGCTL_BSESVLD && connected)
> dwc2_hsotg_core_init_disconnected(hsotg, true);
> }
>
> so, dwc2_hsotg_disconnect() is already called from Reset path. What
> you're doing here is that you could, potentially, call
> driver->disconnect() twice.
>
> The real problem could be your implementation for ->pullup() which
> duplicates part of what ->udc_start() does:
>
> static int dwc2_hsotg_pullup(struct usb_gadget *gadget, int is_on)
> {
> struct dwc2_hsotg *hsotg = to_hsotg(gadget);
> unsigned long flags = 0;
>
> dev_dbg(hsotg->dev, "%s: is_on: %d op_state: %d\n", __func__, is_on,
> hsotg->op_state);
>
> /* Don't modify pullup state while in host mode */
> if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
> hsotg->enabled = is_on;
> return 0;
> }
>
> spin_lock_irqsave(&hsotg->lock, flags);
> if (is_on) {
> hsotg->enabled = 1;
> dwc2_hsotg_core_init_disconnected(hsotg, false);
> dwc2_hsotg_core_connect(hsotg);
> } else {
> dwc2_hsotg_core_disconnect(hsotg);
> dwc2_hsotg_disconnect(hsotg);
> hsotg->enabled = 0;
> }
>
> hsotg->gadget.speed = USB_SPEED_UNKNOWN;
> spin_unlock_irqrestore(&hsotg->lock, flags);
>
> return 0;
> }
>
> Here's what I think dwc2_hsotg_pullup() and dwc2_hsotg_udc_start()
> should contain:
>
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 9dc6c482b89e..dbe28947d3a9 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3388,6 +3388,7 @@ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
> dwc2_writel(0, hsotg->regs + DAINTMSK);
>
> /* Be in disconnected state until gadget is registered */
> + /* REVISIT!!!! This should be done in probe()!!!! */
> __orr32(hsotg->regs + DCTL, DCTL_SFTDISCON);
>
> /* setup fifos */
> @@ -3428,26 +3429,6 @@ static int dwc2_hsotg_udc_start(struct usb_gadget *gadget,
> unsigned long flags;
> int ret;
>
> - if (!hsotg) {
> - pr_err("%s: called with no device\n", __func__);
> - return -ENODEV;
> - }
> -
> - if (!driver) {
> - dev_err(hsotg->dev, "%s: no driver\n", __func__);
> - return -EINVAL;
> - }
> -
> - if (driver->max_speed < USB_SPEED_FULL)
> - dev_err(hsotg->dev, "%s: bad speed\n", __func__);
> -
> - if (!driver->setup) {
> - dev_err(hsotg->dev, "%s: missing entry points\n", __func__);
> - return -EINVAL;
> - }
> -
> - WARN_ON(hsotg->driver);
> -
> driver->driver.bus = NULL;
> hsotg->driver = driver;
> hsotg->gadget.dev.of_node = hsotg->dev->of_node;
> @@ -3550,17 +3531,15 @@ static int dwc2_hsotg_pullup(struct usb_gadget *gadget, int is_on)
> /* Don't modify pullup state while in host mode */
> if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
> hsotg->enabled = is_on;
> - return 0;
> + return -EINVAL;
> }
>
> spin_lock_irqsave(&hsotg->lock, flags);
> if (is_on) {
> hsotg->enabled = 1;
> - dwc2_hsotg_core_init_disconnected(hsotg, false);
> dwc2_hsotg_core_connect(hsotg);
> } else {
> dwc2_hsotg_core_disconnect(hsotg);
> - dwc2_hsotg_disconnect(hsotg);
> hsotg->enabled = 0;
> }
So I reverted my proposed change and gave the above patch a try on my
HiKey device, but this didn't change the issue I'm seeing. I still
get WARN_ONs in dwc2_hsotg_init_fifo() about the fifo_map not being
clear. Adding my proposed change still helps this issue for me.
Though I do see the potential for calling dwc2_hsotg_disconnect()
twice as its already called in the dwc2_hsotg_irq() path before
calling dwc2_hsotg_core_init_disconnected.
But if its of more help, the error I'm seeing seems to be triggered
from the dwc2_conn_id_status_change() code path calling
dwc2_hsotg_core_init_disconnected().
Would the proper fix to be calling dwc2_hsotg_disconnect() from
dwc2_conn_id_status_change() instead?
thanks
-john
^ permalink raw reply
* Re: [PATCH v1] ufs: add a variety of definitions decribed in UFS spec
From: Subhash Jadavani @ 2016-11-08 20:37 UTC (permalink / raw)
To: Kiwoong Kim
Cc: James E.J. Bottomley, linux-scsi, Martin K. Petersen,
vinholikatti, 추헌광, linux-scsi-owner
In-Reply-To: <002501d23996$327bfdb0$9773f910$@samsung.com>
On 2016-11-08 00:00, Kiwoong Kim wrote:
> These things are defined to be used by some UFS Host controllers.
>
> Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> ---
> drivers/scsi/ufs/mphy.h | 38 ++++++++++++++++++++++++++++++++++++++
> drivers/scsi/ufs/ufshci.h | 14 +++++++++++---
> drivers/scsi/ufs/unipro.h | 24 ++++++++++++++++++++++++
> 3 files changed, 73 insertions(+), 3 deletions(-)
> create mode 100644 drivers/scsi/ufs/mphy.h
>
> diff --git a/drivers/scsi/ufs/mphy.h b/drivers/scsi/ufs/mphy.h
> new file mode 100644
> index 0000000..c431f49
> --- /dev/null
> +++ b/drivers/scsi/ufs/mphy.h
> @@ -0,0 +1,38 @@
> +/*
> + * drivers/scsi/ufs/mphy.h
> + *
> + * Copyright (C) 2014 Samsung Electronics Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or
> modify
> + * it under the terms of the GNU General Public License as published
> by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef _MPHY_H_
> +#define _MPHY_H_
Do we really need separate file for MPHY? May be we can have these
accomodated in unipro.h itself?
> +
> +#define TX_HIBERN8TIME_CAP 0x0f
> +#define TX_MIN_ACTIVATE_TIME 0x33
> +
> +#define RX_HS_G1_SYNC_LEN_CAP 0x8b
> +#define RX_HS_G1_PREP_LEN_CAP 0x8c
> +#define RX_HS_G2_SYNC_LEN_CAP 0x94
> +#define RX_HS_G3_SYNC_LEN_CAP 0x95
> +#define RX_HS_G2_PREP_LEN_CAP 0x96
> +#define RX_HS_G3_PREP_LEN_CAP 0x97
> + #define SYNC_RANGE_FINE (0 << 6)
> + #define SYNC_RANGE_COARSE (1 << 6)
> + #define SYNC_LEN(x) ((x) & 0x3f)
> + #define PREP_LEN(x) ((x) & 0xf)
> +#define RX_ADV_GRANULARITY_CAP 0x98
> + #define RX_ADV_GRAN_STEP(x) ((((x) & 0x3) << 1) | 0x1)
> +#define TX_ADV_GRANULARITY_CAP 0x10
> + #define TX_ADV_GRAN_STEP(x) ((((x) & 0x3) << 1) | 0x1)
> +#define RX_MIN_ACTIVATETIME_CAP 0x8f
> +#define RX_HIBERN8TIME_CAP 0x92
> +#define RX_ADV_HIBERN8TIME_CAP 0x99
> +#define RX_ADV_MIN_ACTIVATETIME_CAP 0x9a
> +
> +#endif /* _MPHY_H_ */
> +
> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> index 9599741..26dc340 100644
> --- a/drivers/scsi/ufs/ufshci.h
> +++ b/drivers/scsi/ufs/ufshci.h
> @@ -170,17 +170,25 @@ enum {
> /* UECDL - Host UIC Error Code Data Link Layer 3Ch */
> #define UIC_DATA_LINK_LAYER_ERROR UFS_BIT(31)
> #define UIC_DATA_LINK_LAYER_ERROR_CODE_MASK 0x7FFF
> -#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT 0x2000
> -#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED 0x0001
> -#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT 0x0002
> +#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED UFS_BIT(0)
> +#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT UFS_BIT(1)
> +#define UIC_DATA_LINK_LAYER_ERROR_RX_BUF_OF UFS_BIT(5)
why don't we just add macros for all the bits in UECDL ? This makes it
easy in future.
> +#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT UFS_BIT(13)
>
> /* UECN - Host UIC Error Code Network Layer 40h */
> #define UIC_NETWORK_LAYER_ERROR UFS_BIT(31)
> #define UIC_NETWORK_LAYER_ERROR_CODE_MASK 0x7
> +#define UIC_NETWORK_UNSUPPORTED_HEADER_TYPE BIT(0)
> +#define UIC_NETWORK_BAD_DEVICEID_ENC BIT(1)
> +#define UIC_NETWORK_LHDR_TRAP_PACKET_DROPPING BIT(2)
>
> /* UECT - Host UIC Error Code Transport Layer 44h */
> #define UIC_TRANSPORT_LAYER_ERROR UFS_BIT(31)
> #define UIC_TRANSPORT_LAYER_ERROR_CODE_MASK 0x7F
> +#define UIC_TRANSPORT_UNSUPPORTED_HEADER_TYPE BIT(0)
> +#define UIC_TRANSPORT_UNKNOWN_CPORTID BIT(1)
> +#define UIC_TRANSPORT_NO_CONNECTION_RX BIT(2)
> +#define UIC_TRANSPORT_BAD_TC BIT(4)
May be add definition for bit-5 and 6 as well.
>
> /* UECDME - Host UIC Error Code DME 48h */
> #define UIC_DME_ERROR UFS_BIT(31)
> diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
> index eff8b56..e47e2c2 100644
> --- a/drivers/scsi/ufs/unipro.h
> +++ b/drivers/scsi/ufs/unipro.h
> @@ -127,6 +127,7 @@
> #define PA_PACPREQEOBTIMEOUT 0x1591
> #define PA_HIBERN8TIME 0x15A7
> #define PA_LOCALVERINFO 0x15A9
> +#define PA_GRANULARITY 0x15AA
> #define PA_TACTIVATE 0x15A8
> #define PA_PACPFRAMECOUNT 0x15C0
> #define PA_PACPERRORCOUNT 0x15C1
> @@ -170,6 +171,9 @@ enum {
> UNCHANGED = 7,
> };
>
> +#define IS_PWR_MODE_HS(m) (((m) == FAST_MODE) || ((m) ==
> FASTAUTO_MODE))
s/IS_PWR_MODE_HS/IS_HS_PWR_MODE ?
> +#define IS_PWR_MODE_PWM(m) (((m) == SLOW_MODE) || ((m) ==
> SLOWAUTO_MODE))
s/IS_PWR_MODE_PWM/IS_PWM_PWR_MODE ?
> +
> /* PA TX/RX Frequency Series */
> enum {
> PA_HS_MODE_A = 1,
> @@ -231,6 +235,11 @@ enum ufs_unipro_ver {
> #define DL_PEERTC1PRESENT 0x2066
> #define DL_PEERTC1RXINITCREVAL 0x2067
>
> +/* Default value of L2 Timer */
> +#define FC0PROTTIMEOUTVAL 8191
> +#define TC0REPLAYTIMEOUTVAL 65535
> +#define AFC0REQTIMEOUTVAL 32767
> +
> /*
> * Network Layer Attributes
> */
> @@ -259,6 +268,21 @@ enum ufs_unipro_ver {
> #define T_TC0TXMAXSDUSIZE 0x4060
> #define T_TC1TXMAXSDUSIZE 0x4061
>
> +/* CPort setting */
> +#define E2EFC_ON (1 << 0)
> +#define E2EFC_OFF (0 << 0)
> +#define CSD_N_ON (0 << 1)
> +#define CSD_N_OFF (1 << 1)
> +#define CSV_N_ON (0 << 2)
> +#define CSV_N_OFF (1 << 2)
Can you please specify where these are coming from? Spec number and
section/line number?
> +#define CPORT_DEF_FLAGS (CSV_N_OFF | CSD_N_OFF | E2EFC_OFF)
> +
> +/* CPort connection state */
> +enum {
> + CPORT_IDLE = 0,
> + CPORT_CONNECTED,
> +};
> +
> #ifdef FALSE
> #undef FALSE
> #endif
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Projec
^ permalink raw reply
* Re: BUG: 'list_empty(&vgdev->free_vbufs)' is true!
From: Michael S. Tsirkin @ 2016-11-08 20:37 UTC (permalink / raw)
To: Jiri Slaby
Cc: David Airlie, Linux kernel mailing list, dri-devel,
virtualization
In-Reply-To: <bfe29853-694b-6cb5-02e7-6986a9927438@suse.cz>
On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote:
> Hi,
>
> I can relatively easily reproduce this bug:
> BUG: 'list_empty(&vgdev->free_vbufs)' is true!
> ------------[ cut here ]------------
> kernel BUG at /home/latest/linux/drivers/gpu/drm/virtio/virtgpu_vq.c:130!
> invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> Modules linked in:
> CPU: 1 PID: 355 Comm: kworker/1:2 Not tainted 4.9.0-rc2-next-20161028+ #32
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
> Workqueue: events drm_fb_helper_dirty_work
> task: ffff88007b124980 task.stack: ffff88007b8a0000
> RIP: 0010:virtio_gpu_get_vbuf+0x32e/0x630
> RSP: 0018:ffff88007b8a78c0 EFLAGS: 00010286
> RAX: 000000000000002e RBX: 1ffff1000f714f1d RCX: 0000000000000000
> RDX: 000000000000002e RSI: 0000000000000001 RDI: ffffed000f714f0e
> RBP: ffff88007b8a7970 R08: 0000000000000001 R09: 0000000000000000
> R10: 0000000000000002 R11: 0000000000000001 R12: 0000000000000030
> R13: ffff88007caeaba8 R14: 0000000000000018 R15: ffff88007cae0000
> FS: 0000000000000000(0000) GS:ffff88007dc80000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000601028 CR3: 000000007740d000 CR4: 00000000000006e0
> Call Trace:
> Code: df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 bb 01 00 00 4c 89 69 e8
> eb 9e 48 c7 c6 e0 d2 d1 83 48 c7 c7 20 d3 d1 83 e8 6c fb 04 ff <0f> 0b
> 48 c7 c7 a0 fb b0 85 e8 09 95 86 ff 48 c7 c6 c0 d3 d1 83
> RIP: virtio_gpu_get_vbuf+0x32e/0x630 RSP: ffff88007b8a78c0
>
>
> There is no stacktrace, as the kernel starts panicing all over the place
> during its generation. Any ideas?
>
> thanks,
CC maintainers.
The following might be helpful for debugging - if kernel still will
not stop panicing, we are looking at some kind
of memory corruption.
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5a0f8a7..d5e1e72 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -127,7 +127,11 @@ virtio_gpu_get_vbuf(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf;
spin_lock(&vgdev->free_vbufs_lock);
- BUG_ON(list_empty(&vgdev->free_vbufs));
+ WARN_ON(list_empty(&vgdev->free_vbufs));
+ if (list_empty(&vgdev->free_vbufs)) {
+ spin_unlock(&vgdev->free_vbufs_lock);
+ return ERR_PTR(-EINVAL);
+ }
vbuf = list_first_entry(&vgdev->free_vbufs,
struct virtio_gpu_vbuffer, list);
list_del(&vbuf->list);
^ permalink raw reply related
* Re: BUG: 'list_empty(&vgdev->free_vbufs)' is true!
From: Michael S. Tsirkin @ 2016-11-08 20:37 UTC (permalink / raw)
To: Jiri Slaby
Cc: Gerd Hoffmann, Linux kernel mailing list, dri-devel,
virtualization
In-Reply-To: <bfe29853-694b-6cb5-02e7-6986a9927438@suse.cz>
On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote:
> Hi,
>
> I can relatively easily reproduce this bug:
> BUG: 'list_empty(&vgdev->free_vbufs)' is true!
> ------------[ cut here ]------------
> kernel BUG at /home/latest/linux/drivers/gpu/drm/virtio/virtgpu_vq.c:130!
> invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> Modules linked in:
> CPU: 1 PID: 355 Comm: kworker/1:2 Not tainted 4.9.0-rc2-next-20161028+ #32
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
> Workqueue: events drm_fb_helper_dirty_work
> task: ffff88007b124980 task.stack: ffff88007b8a0000
> RIP: 0010:virtio_gpu_get_vbuf+0x32e/0x630
> RSP: 0018:ffff88007b8a78c0 EFLAGS: 00010286
> RAX: 000000000000002e RBX: 1ffff1000f714f1d RCX: 0000000000000000
> RDX: 000000000000002e RSI: 0000000000000001 RDI: ffffed000f714f0e
> RBP: ffff88007b8a7970 R08: 0000000000000001 R09: 0000000000000000
> R10: 0000000000000002 R11: 0000000000000001 R12: 0000000000000030
> R13: ffff88007caeaba8 R14: 0000000000000018 R15: ffff88007cae0000
> FS: 0000000000000000(0000) GS:ffff88007dc80000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000601028 CR3: 000000007740d000 CR4: 00000000000006e0
> Call Trace:
> Code: df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 bb 01 00 00 4c 89 69 e8
> eb 9e 48 c7 c6 e0 d2 d1 83 48 c7 c7 20 d3 d1 83 e8 6c fb 04 ff <0f> 0b
> 48 c7 c7 a0 fb b0 85 e8 09 95 86 ff 48 c7 c6 c0 d3 d1 83
> RIP: virtio_gpu_get_vbuf+0x32e/0x630 RSP: ffff88007b8a78c0
>
>
> There is no stacktrace, as the kernel starts panicing all over the place
> during its generation. Any ideas?
>
> thanks,
CC maintainers.
The following might be helpful for debugging - if kernel still will
not stop panicing, we are looking at some kind
of memory corruption.
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5a0f8a7..d5e1e72 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -127,7 +127,11 @@ virtio_gpu_get_vbuf(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf;
spin_lock(&vgdev->free_vbufs_lock);
- BUG_ON(list_empty(&vgdev->free_vbufs));
+ WARN_ON(list_empty(&vgdev->free_vbufs));
+ if (list_empty(&vgdev->free_vbufs)) {
+ spin_unlock(&vgdev->free_vbufs_lock);
+ return ERR_PTR(-EINVAL);
+ }
vbuf = list_first_entry(&vgdev->free_vbufs,
struct virtio_gpu_vbuffer, list);
list_del(&vbuf->list);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* Re: BUG: 'list_empty(&vgdev->free_vbufs)' is true!
From: Michael S. Tsirkin @ 2016-11-08 20:37 UTC (permalink / raw)
To: Jiri Slaby
Cc: virtualization, Linux kernel mailing list, David Airlie,
Gerd Hoffmann, dri-devel
In-Reply-To: <bfe29853-694b-6cb5-02e7-6986a9927438@suse.cz>
On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote:
> Hi,
>
> I can relatively easily reproduce this bug:
> BUG: 'list_empty(&vgdev->free_vbufs)' is true!
> ------------[ cut here ]------------
> kernel BUG at /home/latest/linux/drivers/gpu/drm/virtio/virtgpu_vq.c:130!
> invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> Modules linked in:
> CPU: 1 PID: 355 Comm: kworker/1:2 Not tainted 4.9.0-rc2-next-20161028+ #32
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
> Workqueue: events drm_fb_helper_dirty_work
> task: ffff88007b124980 task.stack: ffff88007b8a0000
> RIP: 0010:virtio_gpu_get_vbuf+0x32e/0x630
> RSP: 0018:ffff88007b8a78c0 EFLAGS: 00010286
> RAX: 000000000000002e RBX: 1ffff1000f714f1d RCX: 0000000000000000
> RDX: 000000000000002e RSI: 0000000000000001 RDI: ffffed000f714f0e
> RBP: ffff88007b8a7970 R08: 0000000000000001 R09: 0000000000000000
> R10: 0000000000000002 R11: 0000000000000001 R12: 0000000000000030
> R13: ffff88007caeaba8 R14: 0000000000000018 R15: ffff88007cae0000
> FS: 0000000000000000(0000) GS:ffff88007dc80000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000601028 CR3: 000000007740d000 CR4: 00000000000006e0
> Call Trace:
> Code: df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 bb 01 00 00 4c 89 69 e8
> eb 9e 48 c7 c6 e0 d2 d1 83 48 c7 c7 20 d3 d1 83 e8 6c fb 04 ff <0f> 0b
> 48 c7 c7 a0 fb b0 85 e8 09 95 86 ff 48 c7 c6 c0 d3 d1 83
> RIP: virtio_gpu_get_vbuf+0x32e/0x630 RSP: ffff88007b8a78c0
>
>
> There is no stacktrace, as the kernel starts panicing all over the place
> during its generation. Any ideas?
>
> thanks,
CC maintainers.
The following might be helpful for debugging - if kernel still will
not stop panicing, we are looking at some kind
of memory corruption.
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5a0f8a7..d5e1e72 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -127,7 +127,11 @@ virtio_gpu_get_vbuf(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf;
spin_lock(&vgdev->free_vbufs_lock);
- BUG_ON(list_empty(&vgdev->free_vbufs));
+ WARN_ON(list_empty(&vgdev->free_vbufs));
+ if (list_empty(&vgdev->free_vbufs)) {
+ spin_unlock(&vgdev->free_vbufs_lock);
+ return ERR_PTR(-EINVAL);
+ }
vbuf = list_first_entry(&vgdev->free_vbufs,
struct virtio_gpu_vbuffer, list);
list_del(&vbuf->list);
^ permalink raw reply related
* Re: [PATCH] drm/sun4i: Fix error handling
From: Maxime Ripard @ 2016-11-08 20:38 UTC (permalink / raw)
To: Christophe JAILLET
Cc: kernel-janitors, linux-kernel, dri-devel, wens, linux-arm-kernel
In-Reply-To: <1d4a8fc9-6b62-4af7-19bc-565b15cdc413@wanadoo.fr>
[-- Attachment #1: Type: text/plain, Size: 5922 bytes --]
Salut,
On Sat, Nov 05, 2016 at 07:15:45AM +0100, Christophe JAILLET wrote:
> Le 02/11/2016 à 19:14, Maxime Ripard a écrit :
> > Hi,
> >
> > On Sun, Oct 30, 2016 at 12:53:02PM +0100, Christophe JAILLET wrote:
> > > BTW, memory allocation in 'sun4i_layers_init()' looks spurious, especially
> > > the use of 'layer' in the for loop.
> > > Just my 2 cents.
> > What do you mean by it's spurious?
> Hi Maxime,
>
> what I mean is:
>
> > struct sun4i_layer **sun4i_layers_init(struct drm_device *drm)
> > {
> > struct sun4i_drv *drv = drm->dev_private;
> > struct sun4i_layer **layers;
> > int i;
> >
> > layers = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes),
> > sizeof(**layers), GFP_KERNEL);
> Here, we allocate some memory for ARRAY_SIZE(sun4i_backend_planes) (i.e. 2)
> 'struct sun4i_layer'.
> We do not allocate some space for some pointers but for some structure.
>
> Also, these structures are zeroed and seem to never be initialized by
> anything else.
>
> > if (!layers)
> > return ERR_PTR(-ENOMEM);
> >
> > /*
> > * The hardware is a bit unusual here.
> > *
> > * Even though it supports 4 layers, it does the composition
> > * in two separate steps.
> > *
> > * The first one is assigning a layer to one of its two
> > * pipes. If more that 1 layer is assigned to the same pipe,
> > * and if pixels overlaps, the pipe will take the pixel from
> > * the layer with the highest priority.
> > *
> > * The second step is the actual alpha blending, that takes
> > * the two pipes as input, and uses the eventual alpha
> > * component to do the transparency between the two.
> > *
> > * This two steps scenario makes us unable to guarantee a
> > * robust alpha blending between the 4 layers in all
> > * situations. So we just expose two layers, one per pipe. On
> > * SoCs that support it, sprites could fill the need for more
> > * layers.
> > */
> The comment make me think that this driver (and this function) only handles
> 2 layers ("So we just expose two layers"), which is consistent with
> ARRAY_SIZE(sun4i_backend_planes) (i.e. 2)
> So I would expect that only 2 'struct sun4i_layer' to be allcoated
>
> > for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
> > const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
> > struct sun4i_layer *layer = layers[i];
> Here, we take the address of one of the 2 structure allocated above.
> This is overridden, just the line after.
>
> >
> > layer = sun4i_layer_init_one(drm, plane);
> 'sun4i_layer_init_one()' looks() like:
>
> struct sun4i_layer *layer;
> layer = devm_kzalloc(drm->dev, sizeof(*layer), GFP_KERNEL);
> ...
> return layer;
>
> So we once more allocate some 'struct sun4i_layer'
>
> BUT, the corresponding address is stored into the 'layer' variable, and
> finally seems to get lost and no reference is kept of this. (i.e. 'layers'
> (with an s) is left unchanged)
>
> > if (IS_ERR(layer)) {
> > dev_err(drm->dev, "Couldn't initialize %s plane\n",
> > i ? "overlay" : "primary");
> > return ERR_CAST(layer);
> > };
> >
> > DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
> > i ? "overlay" : "primary", plane->pipe);
> > regmap_update_bits(drv->backend->regs,
> SUN4I_BACKEND_ATTCTL_REG0(i),
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL_MASK,
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(plane->pipe));
> >
> > layer->id = i;
> > };
> >
> > return layers;
> > }
>
>
> So, 4 'struct sun4i_layer' have been allocated. 2 in 'sun4i_layers_init()'
> and 2 in 'sun4i_layer_init_one()' (once at a time, but called twice)
>
> What looks spurious to me is either:
> - 'struct sun4i_layer *layer = layers[i];' which should just be 'struct
> sun4i_layer *layer;'
> or
> - 'layers' (with an s) should be an array of pointers and the addresses
> returned by 'sun4i_layer_init_one()' should be saved there.
>
>
> I don't know at all this driver, so I'm maybe completely wrong.
> What I would have expected would be something like: (un-tested, just to give
> an idea)
>
>
> ==============8<================================================
>
> @@ -133,9 +133,9 @@ struct sun4i_layer **sun4i_layers_init(struct drm_device
> *drm)
> struct sun4i_layer **layers;
> int i;
>
> layers = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes),
> - sizeof(**layers), GFP_KERNEL);
> + sizeof(*layers), GFP_KERNEL);
> if (!layers)
> return ERR_PTR(-ENOMEM);
>
> /*
> @@ -160,16 +160,17 @@ struct sun4i_layer **sun4i_layers_init(struct
> drm_device *drm)
> * layers.
> */
> for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
> const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
> - struct sun4i_layer *layer = layers[i];
> + struct sun4i_layer *layer;
>
> layer = sun4i_layer_init_one(drm, plane);
> if (IS_ERR(layer)) {
> dev_err(drm->dev, "Couldn't initialize %s plane\n",
> i ? "overlay" : "primary");
> return ERR_CAST(layer);
> };
> + layers[i] = layer;
>
> DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
> i ? "overlay" : "primary", plane->pipe);
> regmap_update_bits(drv->backend->regs,
> SUN4I_BACKEND_ATTCTL_REG0(i),
You're totally right. Can you send this as a formal patch?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* [PATCH] drm/sun4i: Fix error handling
From: Maxime Ripard @ 2016-11-08 20:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1d4a8fc9-6b62-4af7-19bc-565b15cdc413@wanadoo.fr>
Salut,
On Sat, Nov 05, 2016 at 07:15:45AM +0100, Christophe JAILLET wrote:
> Le 02/11/2016 ? 19:14, Maxime Ripard a ?crit :
> > Hi,
> >
> > On Sun, Oct 30, 2016 at 12:53:02PM +0100, Christophe JAILLET wrote:
> > > BTW, memory allocation in 'sun4i_layers_init()' looks spurious, especially
> > > the use of 'layer' in the for loop.
> > > Just my 2 cents.
> > What do you mean by it's spurious?
> Hi Maxime,
>
> what I mean is:
>
> > struct sun4i_layer **sun4i_layers_init(struct drm_device *drm)
> > {
> > struct sun4i_drv *drv = drm->dev_private;
> > struct sun4i_layer **layers;
> > int i;
> >
> > layers = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes),
> > sizeof(**layers), GFP_KERNEL);
> Here, we allocate some memory for ARRAY_SIZE(sun4i_backend_planes) (i.e. 2)
> 'struct sun4i_layer'.
> We do not allocate some space for some pointers but for some structure.
>
> Also, these structures are zeroed and seem to never be initialized by
> anything else.
>
> > if (!layers)
> > return ERR_PTR(-ENOMEM);
> >
> > /*
> > * The hardware is a bit unusual here.
> > *
> > * Even though it supports 4 layers, it does the composition
> > * in two separate steps.
> > *
> > * The first one is assigning a layer to one of its two
> > * pipes. If more that 1 layer is assigned to the same pipe,
> > * and if pixels overlaps, the pipe will take the pixel from
> > * the layer with the highest priority.
> > *
> > * The second step is the actual alpha blending, that takes
> > * the two pipes as input, and uses the eventual alpha
> > * component to do the transparency between the two.
> > *
> > * This two steps scenario makes us unable to guarantee a
> > * robust alpha blending between the 4 layers in all
> > * situations. So we just expose two layers, one per pipe. On
> > * SoCs that support it, sprites could fill the need for more
> > * layers.
> > */
> The comment make me think that this driver (and this function) only handles
> 2 layers ("So we just expose two layers"), which is consistent with
> ARRAY_SIZE(sun4i_backend_planes) (i.e. 2)
> So I would expect that only 2 'struct sun4i_layer' to be allcoated
>
> > for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
> > const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
> > struct sun4i_layer *layer = layers[i];
> Here, we take the address of one of the 2 structure allocated above.
> This is overridden, just the line after.
>
> >
> > layer = sun4i_layer_init_one(drm, plane);
> 'sun4i_layer_init_one()' looks() like:
>
> struct sun4i_layer *layer;
> layer = devm_kzalloc(drm->dev, sizeof(*layer), GFP_KERNEL);
> ...
> return layer;
>
> So we once more allocate some 'struct sun4i_layer'
>
> BUT, the corresponding address is stored into the 'layer' variable, and
> finally seems to get lost and no reference is kept of this. (i.e. 'layers'
> (with an s) is left unchanged)
>
> > if (IS_ERR(layer)) {
> > dev_err(drm->dev, "Couldn't initialize %s plane\n",
> > i ? "overlay" : "primary");
> > return ERR_CAST(layer);
> > };
> >
> > DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
> > i ? "overlay" : "primary", plane->pipe);
> > regmap_update_bits(drv->backend->regs,
> SUN4I_BACKEND_ATTCTL_REG0(i),
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL_MASK,
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(plane->pipe));
> >
> > layer->id = i;
> > };
> >
> > return layers;
> > }
>
>
> So, 4 'struct sun4i_layer' have been allocated. 2 in 'sun4i_layers_init()'
> and 2 in 'sun4i_layer_init_one()' (once at a time, but called twice)
>
> What looks spurious to me is either:
> - 'struct sun4i_layer *layer = layers[i];' which should just be 'struct
> sun4i_layer *layer;'
> or
> - 'layers' (with an s) should be an array of pointers and the addresses
> returned by 'sun4i_layer_init_one()' should be saved there.
>
>
> I don't know at all this driver, so I'm maybe completely wrong.
> What I would have expected would be something like: (un-tested, just to give
> an idea)
>
>
> ==============8<================================================
>
> @@ -133,9 +133,9 @@ struct sun4i_layer **sun4i_layers_init(struct drm_device
> *drm)
> struct sun4i_layer **layers;
> int i;
>
> layers = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes),
> - sizeof(**layers), GFP_KERNEL);
> + sizeof(*layers), GFP_KERNEL);
> if (!layers)
> return ERR_PTR(-ENOMEM);
>
> /*
> @@ -160,16 +160,17 @@ struct sun4i_layer **sun4i_layers_init(struct
> drm_device *drm)
> * layers.
> */
> for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
> const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
> - struct sun4i_layer *layer = layers[i];
> + struct sun4i_layer *layer;
>
> layer = sun4i_layer_init_one(drm, plane);
> if (IS_ERR(layer)) {
> dev_err(drm->dev, "Couldn't initialize %s plane\n",
> i ? "overlay" : "primary");
> return ERR_CAST(layer);
> };
> + layers[i] = layer;
>
> DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
> i ? "overlay" : "primary", plane->pipe);
> regmap_update_bits(drv->backend->regs,
> SUN4I_BACKEND_ATTCTL_REG0(i),
You're totally right. Can you send this as a formal patch?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/2ed78f33/attachment.sig>
^ permalink raw reply
* Re: [PATCH] drm/sun4i: Fix error handling
From: Maxime Ripard @ 2016-11-08 20:38 UTC (permalink / raw)
To: Christophe JAILLET
Cc: kernel-janitors, linux-kernel, dri-devel, wens, linux-arm-kernel
In-Reply-To: <1d4a8fc9-6b62-4af7-19bc-565b15cdc413@wanadoo.fr>
[-- Attachment #1.1: Type: text/plain, Size: 5922 bytes --]
Salut,
On Sat, Nov 05, 2016 at 07:15:45AM +0100, Christophe JAILLET wrote:
> Le 02/11/2016 à 19:14, Maxime Ripard a écrit :
> > Hi,
> >
> > On Sun, Oct 30, 2016 at 12:53:02PM +0100, Christophe JAILLET wrote:
> > > BTW, memory allocation in 'sun4i_layers_init()' looks spurious, especially
> > > the use of 'layer' in the for loop.
> > > Just my 2 cents.
> > What do you mean by it's spurious?
> Hi Maxime,
>
> what I mean is:
>
> > struct sun4i_layer **sun4i_layers_init(struct drm_device *drm)
> > {
> > struct sun4i_drv *drv = drm->dev_private;
> > struct sun4i_layer **layers;
> > int i;
> >
> > layers = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes),
> > sizeof(**layers), GFP_KERNEL);
> Here, we allocate some memory for ARRAY_SIZE(sun4i_backend_planes) (i.e. 2)
> 'struct sun4i_layer'.
> We do not allocate some space for some pointers but for some structure.
>
> Also, these structures are zeroed and seem to never be initialized by
> anything else.
>
> > if (!layers)
> > return ERR_PTR(-ENOMEM);
> >
> > /*
> > * The hardware is a bit unusual here.
> > *
> > * Even though it supports 4 layers, it does the composition
> > * in two separate steps.
> > *
> > * The first one is assigning a layer to one of its two
> > * pipes. If more that 1 layer is assigned to the same pipe,
> > * and if pixels overlaps, the pipe will take the pixel from
> > * the layer with the highest priority.
> > *
> > * The second step is the actual alpha blending, that takes
> > * the two pipes as input, and uses the eventual alpha
> > * component to do the transparency between the two.
> > *
> > * This two steps scenario makes us unable to guarantee a
> > * robust alpha blending between the 4 layers in all
> > * situations. So we just expose two layers, one per pipe. On
> > * SoCs that support it, sprites could fill the need for more
> > * layers.
> > */
> The comment make me think that this driver (and this function) only handles
> 2 layers ("So we just expose two layers"), which is consistent with
> ARRAY_SIZE(sun4i_backend_planes) (i.e. 2)
> So I would expect that only 2 'struct sun4i_layer' to be allcoated
>
> > for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
> > const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
> > struct sun4i_layer *layer = layers[i];
> Here, we take the address of one of the 2 structure allocated above.
> This is overridden, just the line after.
>
> >
> > layer = sun4i_layer_init_one(drm, plane);
> 'sun4i_layer_init_one()' looks() like:
>
> struct sun4i_layer *layer;
> layer = devm_kzalloc(drm->dev, sizeof(*layer), GFP_KERNEL);
> ...
> return layer;
>
> So we once more allocate some 'struct sun4i_layer'
>
> BUT, the corresponding address is stored into the 'layer' variable, and
> finally seems to get lost and no reference is kept of this. (i.e. 'layers'
> (with an s) is left unchanged)
>
> > if (IS_ERR(layer)) {
> > dev_err(drm->dev, "Couldn't initialize %s plane\n",
> > i ? "overlay" : "primary");
> > return ERR_CAST(layer);
> > };
> >
> > DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
> > i ? "overlay" : "primary", plane->pipe);
> > regmap_update_bits(drv->backend->regs,
> SUN4I_BACKEND_ATTCTL_REG0(i),
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL_MASK,
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(plane->pipe));
> >
> > layer->id = i;
> > };
> >
> > return layers;
> > }
>
>
> So, 4 'struct sun4i_layer' have been allocated. 2 in 'sun4i_layers_init()'
> and 2 in 'sun4i_layer_init_one()' (once at a time, but called twice)
>
> What looks spurious to me is either:
> - 'struct sun4i_layer *layer = layers[i];' which should just be 'struct
> sun4i_layer *layer;'
> or
> - 'layers' (with an s) should be an array of pointers and the addresses
> returned by 'sun4i_layer_init_one()' should be saved there.
>
>
> I don't know at all this driver, so I'm maybe completely wrong.
> What I would have expected would be something like: (un-tested, just to give
> an idea)
>
>
> ==============8<================================================
>
> @@ -133,9 +133,9 @@ struct sun4i_layer **sun4i_layers_init(struct drm_device
> *drm)
> struct sun4i_layer **layers;
> int i;
>
> layers = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes),
> - sizeof(**layers), GFP_KERNEL);
> + sizeof(*layers), GFP_KERNEL);
> if (!layers)
> return ERR_PTR(-ENOMEM);
>
> /*
> @@ -160,16 +160,17 @@ struct sun4i_layer **sun4i_layers_init(struct
> drm_device *drm)
> * layers.
> */
> for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
> const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
> - struct sun4i_layer *layer = layers[i];
> + struct sun4i_layer *layer;
>
> layer = sun4i_layer_init_one(drm, plane);
> if (IS_ERR(layer)) {
> dev_err(drm->dev, "Couldn't initialize %s plane\n",
> i ? "overlay" : "primary");
> return ERR_CAST(layer);
> };
> + layers[i] = layer;
>
> DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
> i ? "overlay" : "primary", plane->pipe);
> regmap_update_bits(drv->backend->regs,
> SUN4I_BACKEND_ATTCTL_REG0(i),
You're totally right. Can you send this as a formal patch?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH] drm/sun4i: Fix error handling
From: Maxime Ripard @ 2016-11-08 20:38 UTC (permalink / raw)
To: Christophe JAILLET
Cc: airlied, wens, dri-devel, linux-arm-kernel, linux-kernel,
kernel-janitors
In-Reply-To: <1d4a8fc9-6b62-4af7-19bc-565b15cdc413@wanadoo.fr>
[-- Attachment #1: Type: text/plain, Size: 5922 bytes --]
Salut,
On Sat, Nov 05, 2016 at 07:15:45AM +0100, Christophe JAILLET wrote:
> Le 02/11/2016 à 19:14, Maxime Ripard a écrit :
> > Hi,
> >
> > On Sun, Oct 30, 2016 at 12:53:02PM +0100, Christophe JAILLET wrote:
> > > BTW, memory allocation in 'sun4i_layers_init()' looks spurious, especially
> > > the use of 'layer' in the for loop.
> > > Just my 2 cents.
> > What do you mean by it's spurious?
> Hi Maxime,
>
> what I mean is:
>
> > struct sun4i_layer **sun4i_layers_init(struct drm_device *drm)
> > {
> > struct sun4i_drv *drv = drm->dev_private;
> > struct sun4i_layer **layers;
> > int i;
> >
> > layers = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes),
> > sizeof(**layers), GFP_KERNEL);
> Here, we allocate some memory for ARRAY_SIZE(sun4i_backend_planes) (i.e. 2)
> 'struct sun4i_layer'.
> We do not allocate some space for some pointers but for some structure.
>
> Also, these structures are zeroed and seem to never be initialized by
> anything else.
>
> > if (!layers)
> > return ERR_PTR(-ENOMEM);
> >
> > /*
> > * The hardware is a bit unusual here.
> > *
> > * Even though it supports 4 layers, it does the composition
> > * in two separate steps.
> > *
> > * The first one is assigning a layer to one of its two
> > * pipes. If more that 1 layer is assigned to the same pipe,
> > * and if pixels overlaps, the pipe will take the pixel from
> > * the layer with the highest priority.
> > *
> > * The second step is the actual alpha blending, that takes
> > * the two pipes as input, and uses the eventual alpha
> > * component to do the transparency between the two.
> > *
> > * This two steps scenario makes us unable to guarantee a
> > * robust alpha blending between the 4 layers in all
> > * situations. So we just expose two layers, one per pipe. On
> > * SoCs that support it, sprites could fill the need for more
> > * layers.
> > */
> The comment make me think that this driver (and this function) only handles
> 2 layers ("So we just expose two layers"), which is consistent with
> ARRAY_SIZE(sun4i_backend_planes) (i.e. 2)
> So I would expect that only 2 'struct sun4i_layer' to be allcoated
>
> > for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
> > const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
> > struct sun4i_layer *layer = layers[i];
> Here, we take the address of one of the 2 structure allocated above.
> This is overridden, just the line after.
>
> >
> > layer = sun4i_layer_init_one(drm, plane);
> 'sun4i_layer_init_one()' looks() like:
>
> struct sun4i_layer *layer;
> layer = devm_kzalloc(drm->dev, sizeof(*layer), GFP_KERNEL);
> ...
> return layer;
>
> So we once more allocate some 'struct sun4i_layer'
>
> BUT, the corresponding address is stored into the 'layer' variable, and
> finally seems to get lost and no reference is kept of this. (i.e. 'layers'
> (with an s) is left unchanged)
>
> > if (IS_ERR(layer)) {
> > dev_err(drm->dev, "Couldn't initialize %s plane\n",
> > i ? "overlay" : "primary");
> > return ERR_CAST(layer);
> > };
> >
> > DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
> > i ? "overlay" : "primary", plane->pipe);
> > regmap_update_bits(drv->backend->regs,
> SUN4I_BACKEND_ATTCTL_REG0(i),
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL_MASK,
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(plane->pipe));
> >
> > layer->id = i;
> > };
> >
> > return layers;
> > }
>
>
> So, 4 'struct sun4i_layer' have been allocated. 2 in 'sun4i_layers_init()'
> and 2 in 'sun4i_layer_init_one()' (once at a time, but called twice)
>
> What looks spurious to me is either:
> - 'struct sun4i_layer *layer = layers[i];' which should just be 'struct
> sun4i_layer *layer;'
> or
> - 'layers' (with an s) should be an array of pointers and the addresses
> returned by 'sun4i_layer_init_one()' should be saved there.
>
>
> I don't know at all this driver, so I'm maybe completely wrong.
> What I would have expected would be something like: (un-tested, just to give
> an idea)
>
>
> ==============8<================================================
>
> @@ -133,9 +133,9 @@ struct sun4i_layer **sun4i_layers_init(struct drm_device
> *drm)
> struct sun4i_layer **layers;
> int i;
>
> layers = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes),
> - sizeof(**layers), GFP_KERNEL);
> + sizeof(*layers), GFP_KERNEL);
> if (!layers)
> return ERR_PTR(-ENOMEM);
>
> /*
> @@ -160,16 +160,17 @@ struct sun4i_layer **sun4i_layers_init(struct
> drm_device *drm)
> * layers.
> */
> for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
> const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
> - struct sun4i_layer *layer = layers[i];
> + struct sun4i_layer *layer;
>
> layer = sun4i_layer_init_one(drm, plane);
> if (IS_ERR(layer)) {
> dev_err(drm->dev, "Couldn't initialize %s plane\n",
> i ? "overlay" : "primary");
> return ERR_CAST(layer);
> };
> + layers[i] = layer;
>
> DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
> i ? "overlay" : "primary", plane->pipe);
> regmap_update_bits(drv->backend->regs,
> SUN4I_BACKEND_ATTCTL_REG0(i),
You're totally right. Can you send this as a formal patch?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: WARNING: mismatch_cnt is not 0 on <array device>
From: Phil Turmel @ 2016-11-08 20:38 UTC (permalink / raw)
To: Benjammin2068, Linux-RAID
In-Reply-To: <287df6d6-3850-1142-5c69-c7b54a8a22d4@gmail.com>
On 11/08/2016 02:53 PM, Benjammin2068 wrote:
> On 11/08/2016 12:47 PM, Benjammin2068 wrote:
> Now that I think about it -- and have been talking out loud to myself (I don't think I'm crazy)...
>
> A parallel to all this is:
>
> I don't think the mismatch_cnt started showing up until I moved from RAID5 to RAID6.
>
> :O
>
> How painful is it to switch back to RAID5 to test that theory?
Don't. Sounds like raid6's stricter calculations are catching a real
problem. Do you have ECC RAM? If so, are you getting any machine check
exceptions? If not, have you done a thorough memtest any time in the
recent past?
If it's not memory, can you exercise the controller channels heavily to
see if they drop from errors?
Have you added up the peak current draws of your drives to make sure
your power supply keeps up when all drives are writing simultaneously
(common with parity raid)?
One more: do you have swap on top of md raid?
Phil
^ permalink raw reply
* Re: [PATCH v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation
From: NeilBrown @ 2016-11-08 20:38 UTC (permalink / raw)
To: Peter Chen
Cc: Baolin Wang, Felipe Balbi, Greg KH, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, robh, Jun Li,
Marek Szyprowski, Ruslan Bilovol, Peter Chen, Alan Stern,
grygorii.strashko, Yoshihiro Shimoda, Lee Jones, Mark Brown,
John Stultz, Charles Keepax, patches, Linux PM list
In-Reply-To: <20161108084133.GA12276@b29397-desktop>
[-- Attachment #1: Type: text/plain, Size: 1414 bytes --]
On Tue, Nov 08 2016, Peter Chen wrote:
> On Thu, Nov 03, 2016 at 12:25:42PM +1100, NeilBrown wrote:
>>
>>
>>
>> 2/ Change all usb phys to register an extcon and to send appropriate
>> notifications. Many already do, but I don't think it is universal.
>> It is probable that the extcon should be registered using common code
>> instead of each phy driver having its own
>> extcon_get_edev_by_phandle()
>> or whatever.
>> If the usb phy driver needs to look at battery charger registers to
>> know what sort of cable was connected (which I believe is the case
>> for the chips you are interested in), then it should do that.
>
> Not only USB PHY to register an extcon, but also for the drivers which
> can detect USB charger type, it may be USB controller driver, USB type-c
> driver, pmic driver, and these drivers may not have an extcon device
> since the internal part can finish the vbus detect.
Can you point to an example of the sort of hardware/driver you are
referring to, preferably in the mainline kernel. Concrete examples make
this sort of thing much easier to understand.
I think I would argue that if some piece of hardware can detect the USB
charger type, then that piece of hardware is part of the "USB PHY", even
if the hardware manufacturer has chosen to partition the register set in
a way which doesn't make that obvious.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]
^ permalink raw reply
* Re: [PATCH v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation
From: NeilBrown @ 2016-11-08 20:38 UTC (permalink / raw)
To: Peter Chen
Cc: Baolin Wang, Felipe Balbi, Greg KH, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, robh, Jun Li,
Marek Szyprowski, Ruslan Bilovol, Peter Chen, Alan Stern,
grygorii.strashko, Yoshihiro Shimoda, Lee Jones, Mark Brown,
John Stultz, Charles Keepax, patches, Linux PM list, USB,
device-mainlining, LKML
In-Reply-To: <20161108084133.GA12276@b29397-desktop>
[-- Attachment #1: Type: text/plain, Size: 1414 bytes --]
On Tue, Nov 08 2016, Peter Chen wrote:
> On Thu, Nov 03, 2016 at 12:25:42PM +1100, NeilBrown wrote:
>>
>>
>>
>> 2/ Change all usb phys to register an extcon and to send appropriate
>> notifications. Many already do, but I don't think it is universal.
>> It is probable that the extcon should be registered using common code
>> instead of each phy driver having its own
>> extcon_get_edev_by_phandle()
>> or whatever.
>> If the usb phy driver needs to look at battery charger registers to
>> know what sort of cable was connected (which I believe is the case
>> for the chips you are interested in), then it should do that.
>
> Not only USB PHY to register an extcon, but also for the drivers which
> can detect USB charger type, it may be USB controller driver, USB type-c
> driver, pmic driver, and these drivers may not have an extcon device
> since the internal part can finish the vbus detect.
Can you point to an example of the sort of hardware/driver you are
referring to, preferably in the mainline kernel. Concrete examples make
this sort of thing much easier to understand.
I think I would argue that if some piece of hardware can detect the USB
charger type, then that piece of hardware is part of the "USB PHY", even
if the hardware manufacturer has chosen to partition the register set in
a way which doesn't make that obvious.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] ARM: gr8: evb: Add i2s codec
From: Maxime Ripard @ 2016-11-08 20:39 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Liam Girdwood, Mark Brown, linux-arm-kernel, linux-kernel,
Linux-ALSA
In-Reply-To: <CAGb2v6557B5dooERbKc709Hbrzb3vP8eoSj3nh6rKek7UwrgEQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]
On Tue, Nov 08, 2016 at 03:57:48PM +0800, Chen-Yu Tsai wrote:
> On Tue, Nov 8, 2016 at 3:44 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Mon, Nov 07, 2016 at 10:11:45PM +0800, Chen-Yu Tsai wrote:
> >> On Mon, Nov 7, 2016 at 9:08 PM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > The GR8-EVB comes with a wm8978 codec connected to the i2s bus.
> >> >
> >> > Add a card in order to have it working
> >> >
> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> > ---
> >> > arch/arm/boot/dts/ntc-gr8-evb.dts | 14 ++++++++++++++
> >> > 1 file changed, 14 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/arch/arm/boot/dts/ntc-gr8-evb.dts b/arch/arm/boot/dts/ntc-gr8-evb.dts
> >> > index 12b4317a4383..5291e425caf9 100644
> >> > --- a/arch/arm/boot/dts/ntc-gr8-evb.dts
> >> > +++ b/arch/arm/boot/dts/ntc-gr8-evb.dts
> >> > @@ -76,6 +76,20 @@
> >> > default-brightness-level = <8>;
> >> > };
> >> >
> >> > + i2s {
> >>
> >> "sound" might be a better node name? The I2S controllers are also
> >> named "i2s".
> >
> > I know, but we also had the codec and SPDIF on this board, so sound
> > was too generic to be meaningful I guess. I don't really care about
> > the name though, if you have any suggestion...
>
> Well people seem to use "sound" for the sound card nodes...
>
> What about "sound-analog" for this one, and "sound-spdif" for the
> SPDIF simple card? Or "analog-sound" and "spdif-sound" if that looks
> better.
sound-analog works for me. I'll either fix it in the v2, or while
applying.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* [PATCH 3/3] ARM: gr8: evb: Add i2s codec
From: Maxime Ripard @ 2016-11-08 20:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v6557B5dooERbKc709Hbrzb3vP8eoSj3nh6rKek7UwrgEQ@mail.gmail.com>
On Tue, Nov 08, 2016 at 03:57:48PM +0800, Chen-Yu Tsai wrote:
> On Tue, Nov 8, 2016 at 3:44 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Mon, Nov 07, 2016 at 10:11:45PM +0800, Chen-Yu Tsai wrote:
> >> On Mon, Nov 7, 2016 at 9:08 PM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > The GR8-EVB comes with a wm8978 codec connected to the i2s bus.
> >> >
> >> > Add a card in order to have it working
> >> >
> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> > ---
> >> > arch/arm/boot/dts/ntc-gr8-evb.dts | 14 ++++++++++++++
> >> > 1 file changed, 14 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/arch/arm/boot/dts/ntc-gr8-evb.dts b/arch/arm/boot/dts/ntc-gr8-evb.dts
> >> > index 12b4317a4383..5291e425caf9 100644
> >> > --- a/arch/arm/boot/dts/ntc-gr8-evb.dts
> >> > +++ b/arch/arm/boot/dts/ntc-gr8-evb.dts
> >> > @@ -76,6 +76,20 @@
> >> > default-brightness-level = <8>;
> >> > };
> >> >
> >> > + i2s {
> >>
> >> "sound" might be a better node name? The I2S controllers are also
> >> named "i2s".
> >
> > I know, but we also had the codec and SPDIF on this board, so sound
> > was too generic to be meaningful I guess. I don't really care about
> > the name though, if you have any suggestion...
>
> Well people seem to use "sound" for the sound card nodes...
>
> What about "sound-analog" for this one, and "sound-spdif" for the
> SPDIF simple card? Or "analog-sound" and "spdif-sound" if that looks
> better.
sound-analog works for me. I'll either fix it in the v2, or while
applying.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161108/68ade2b6/attachment.sig>
^ permalink raw reply
* Re: [PATCH v2 2/2] scsi: ufs: Use the resource-managed function to add devfreq device
From: Subhash Jadavani @ 2016-11-08 20:40 UTC (permalink / raw)
To: Chanwoo Choi
Cc: myungjoo.ham, kyungmin.park, rjw, cpgs, linux-pm, linux-kernel,
Vinayak Holikatti, James E.J. Bottomley, Martin K. Petersen,
linux-scsi
In-Reply-To: <1478596619-3064-1-git-send-email-cw00.choi@samsung.com>
On 2016-11-08 01:16, Chanwoo Choi wrote:
> This patch uses the resource-managed to add the devfreq device.
> This function will make it easy to handle the devfreq device.
>
> - struct devfreq *devm_devfreq_add_device(struct device *dev,
> struct devfreq_dev_profile *profile,
> const char *governor_name,
> void *data);
> Cc: Vinayak Holikatti <vinholikatti@gmail.com>
> Cc: James E.J. Bottomley <jejb@linux.vnet.ibm.com>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: linux-scsi@vger.kernel.org
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index e491c4bda32f..e8c5ba274830 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6250,8 +6250,6 @@ void ufshcd_remove(struct ufs_hba *hba)
> ufshcd_hba_stop(hba, true);
>
> ufshcd_exit_clk_gating(hba);
> - if (ufshcd_is_clkscaling_enabled(hba))
> - devfreq_remove_device(hba->devfreq);
> ufshcd_hba_exit(hba);
> }
> EXPORT_SYMBOL_GPL(ufshcd_remove);
> @@ -6579,7 +6577,7 @@ int ufshcd_init(struct ufs_hba *hba, void
> __iomem *mmio_base, unsigned int irq)
> }
>
> if (ufshcd_is_clkscaling_enabled(hba)) {
> - hba->devfreq = devfreq_add_device(dev, &ufs_devfreq_profile,
> + hba->devfreq = devm_devfreq_add_device(dev, &ufs_devfreq_profile,
> "simple_ondemand", NULL);
> if (IS_ERR(hba->devfreq)) {
> dev_err(hba->dev, "Unable to register with devfreq %ld\n",
LGTM.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v11 17/22] vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare()
From: Kirti Wankhede @ 2016-11-08 20:41 UTC (permalink / raw)
To: Alexey Kardashevskiy, alex.williamson, pbonzini, kraxel, cjia
Cc: qemu-devel, kvm, kevin.tian, jike.song, bjsdjshi, linux-kernel
In-Reply-To: <e8d532f1-5724-9772-6a86-c48b50b7efdc@ozlabs.ru>
On 11/8/2016 2:22 PM, Alexey Kardashevskiy wrote:
> On 05/11/16 08:10, Kirti Wankhede wrote:
>> Updated vfio_platform_common.c file to use
>> vfio_set_irqs_validate_and_prepare()
>>
>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>> Signed-off-by: Neo Jia <cjia@nvidia.com>
>> Change-Id: Id87cd6b78ae901610b39bf957974baa6f40cd7b0
>> ---
>> drivers/vfio/platform/vfio_platform_common.c | 31 +++++++---------------------
>> 1 file changed, 8 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
>> index d78142830754..4c27f4be3c3d 100644
>> --- a/drivers/vfio/platform/vfio_platform_common.c
>> +++ b/drivers/vfio/platform/vfio_platform_common.c
>> @@ -364,36 +364,21 @@ static long vfio_platform_ioctl(void *device_data,
>> struct vfio_irq_set hdr;
>> u8 *data = NULL;
>> int ret = 0;
>> + size_t data_size = 0;
>>
>> minsz = offsetofend(struct vfio_irq_set, count);
>>
>> if (copy_from_user(&hdr, (void __user *)arg, minsz))
>> return -EFAULT;
>>
>> - if (hdr.argsz < minsz)
>> - return -EINVAL;
>> -
>> - if (hdr.index >= vdev->num_irqs)
>> - return -EINVAL;
>> -
>> - if (hdr.flags & ~(VFIO_IRQ_SET_DATA_TYPE_MASK |
>> - VFIO_IRQ_SET_ACTION_TYPE_MASK))
>> - return -EINVAL;
>> -
>> - if (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE)) {
>> - size_t size;
>> -
>> - if (hdr.flags & VFIO_IRQ_SET_DATA_BOOL)
>> - size = sizeof(uint8_t);
>> - else if (hdr.flags & VFIO_IRQ_SET_DATA_EVENTFD)
>> - size = sizeof(int32_t);
>> - else
>> - return -EINVAL;
>> -
>> - if (hdr.argsz - minsz < size)
>> - return -EINVAL;
>> + ret = vfio_set_irqs_validate_and_prepare(&hdr, vdev->num_irqs,
>> + vdev->num_irqs, &data_size);
>
> The patch does not change this but I am still curious:
>
> is not the second vdev->num_irqs supposed to be one of
> VFIO_PCI_INTX_IRQ_INDEX..VFIO_PCI_NUM_IRQS, not the actual number of
> interrupt vectors (as in vfio-pci)?
>
>
Those are PCI specific. I don't think those counts are applicable here.
If you see the prototype, second argument and third argument have
different meaning.
int vfio_set_irqs_validate_and_prepare(struct vfio_irq_set *hdr, int
num_irqs, int max_irq_type, size_t *data_size)
- num_irqs are number of irqs caller want to setup and
- max_irq_type is the one which is return to user in
VFIO_DEVICE_GET_INFO ioctl's info.num_irqs.
For platform these two are same.
Thanks,
Kirti
>
>
>> + if (ret)
>> + return ret;
>>
>> - data = memdup_user((void __user *)(arg + minsz), size);
>> + if (data_size) {
>> + data = memdup_user((void __user *)(arg + minsz),
>> + data_size);
>> if (IS_ERR(data))
>> return PTR_ERR(data);
>> }
>>
>
>
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 0/3] [RFC] Add HAX support
From: Paolo Bonzini @ 2016-11-08 20:41 UTC (permalink / raw)
To: Vincent Palatin; +Cc: qemu-devel
In-Reply-To: <CAP_ceTwvtCLW8U-jSPpZ13zYf+cPVx67DDnrtTmuvV7Tq7=zLg@mail.gmail.com>
On 08/11/2016 20:41, Vincent Palatin wrote:
> > If so, I think we should only support those
> > processors and slash all the part related to HAX_EMULATE_STATE_INITIAL
> > and HAX_EMULATE_STATE_REAL. This would probably let us make patch 3
> > much less intrusive.
>
> Sure the whole patchset would be lighter, not sure which proportion of
> user have VT machines without UG support though.
All Intel machines sold after ~2010 should have unrestricted guest
support. (HAX doesn't support AMD, but anyway AMD's virt extensions
have always had the equivalent feature).
I'm not sure we want !UG support at all but, if we do, QEMU 2.9 will
have multithreaded TCG so it would be possible to make it less
intrusive. So it's worth starting with the minimum patchset and see
what happens.
Thanks for working on this!
Paolo
^ permalink raw reply
* Re: [PATCH] x86/MCE: Remove MCP_TIMESTAMP
From: Thomas Gleixner @ 2016-11-08 20:39 UTC (permalink / raw)
To: Borislav Petkov; +Cc: Luck, Tony, linux-edac, X86 ML, LKML
In-Reply-To: <20161108180932.wvvcbbzrzrai26eg@pd.tnic>
On Tue, 8 Nov 2016, Borislav Petkov wrote:
>
> Also, this fixes another bug where machine_check_poll() would clear
> mce.tsc unconditionally even if we requested precise MCP_TIMESTAMP
> logging.
> @@ -713,7 +713,6 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
> m.misc = 0;
> m.addr = 0;
> m.bank = i;
> - m.tsc = 0;
That does not make any sense. Where is m.tsc initialized? I couldn't find
any place which does, except this and the conditional clear farther down in
that function.
Thanks,
tglx
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
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.