]> Git Repo - linux.git/log
linux.git
4 years agomedia: staging: media: atomisp: fix stack overflow in init_pipe_defaults()
Arnd Bergmann [Fri, 29 May 2020 20:00:27 +0000 (22:00 +0200)]
media: staging: media: atomisp: fix stack overflow in init_pipe_defaults()

When building with clang, multiple copies of the structures to be
initialized are passed around on the stack and copied locally, using an
insane amount of stack space:

drivers/staging/media/atomisp/pci/sh_css.c:2371:1: error: stack frame size of 26864 bytes in function 'create_pipe' [-Werror,-Wframe-larger-than=]

Use constantly-allocated variables plus an explicit memcpy()
to avoid that.

Co-authored-by: Mauro Carvalho Chehab <[email protected]>
Fixes: 6dc9a2568f84 ("media: atomisp: convert default struct values to use compound-literals with designated initializers")
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: re-enable warnings again
Mauro Carvalho Chehab [Sat, 30 May 2020 16:10:07 +0000 (18:10 +0200)]
media: atomisp: re-enable warnings again

For most warnings, the current code is OK. There are still
some issues with implicit-fallthough warnings.

Solve those and re-enable all warnings for this driver.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: remove format duplication at mbus->fourcc table
Mauro Carvalho Chehab [Sat, 30 May 2020 15:24:15 +0000 (17:24 +0200)]
media: atomisp: remove format duplication at mbus->fourcc table

This table used to be used also to translate between ia_css
abstraction and V4L2 fourcc codes.

This was removed on a past patch, but the table now contains
two fields with identical values.

Get rid of one of them.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: staging: media: atomisp: add PMIC_OPREGION dependency
Arnd Bergmann [Fri, 29 May 2020 20:00:31 +0000 (22:00 +0200)]
media: staging: media: atomisp: add PMIC_OPREGION dependency

Without that driver, there is a link failure in

ERROR: modpost: "intel_soc_pmic_exec_mipi_pmic_seq_element"
[drivers/staging/media/atomisp/pci/atomisp_gmin_platform.ko] undefined!

Add an explicit Kconfig dependency.

Signed-off-by: Arnd Bergmann <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: staging: media: atomisp: disable all custom formats
Arnd Bergmann [Fri, 29 May 2020 20:00:30 +0000 (22:00 +0200)]
media: staging: media: atomisp: disable all custom formats

clang points out the usage of an incorrect enum type in the
list of supported image formats:

drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:65: error: implicit conversion from enumeration type 'enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
        { V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:39: error: implicit conversion from enumeration type 'enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
        { V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
        { V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0, CSS_FRAME_FORMAT_NV12 },
        { MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 },

Checking the git history, I found a commit that disabled one such case
because it did not work. It seems likely that the incorrect enum was
part of the original problem and that the others do not work either,
or have never been tested.

Disable all the ones that cause a warning.

Fixes: cb02ae3d71ea ("media: staging: atomisp: Disable custom format for now")
Signed-off-by: Arnd Bergmann <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: staging: media: atomisp: fix enum type mixups
Arnd Bergmann [Fri, 29 May 2020 20:00:29 +0000 (22:00 +0200)]
media: staging: media: atomisp: fix enum type mixups

Some function calls pass an incorrect enum type:

drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:858:16: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
        gp_device_rst(INPUT_SYSTEM0_ID);
        ~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:860:19: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
        input_switch_rst(INPUT_SYSTEM0_ID);
        ~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:876:27: error: implicit conversion from enumeration type 'input_system_cfg_flag_t' to different enumeration type 'input_system_connection_t' [-Werror,-Wenum-conversion]
                config.multicast[i]              = INPUT_SYSTEM_CFG_FLAG_RESET;
                                                 ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1326:32: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
        input_selector_cfg_for_sensor(INPUT_SYSTEM0_ID);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1329:19: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
        input_switch_cfg(INPUT_SYSTEM0_ID, &config.input_switch_cfg);
        ~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~

INPUT_SYSTEM0_ID is zero, so use the corresponding zero-value
of the expected types instead.

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: staging: media: atomisp: declare 'struct device' before using it
Arnd Bergmann [Fri, 29 May 2020 20:00:24 +0000 (22:00 +0200)]
media: staging: media: atomisp: declare 'struct device' before using it

In some configurations, including this header leads to a warning:

drivers/staging/media/atomisp//pci/sh_css_firmware.h:41:38: error: declaration of 'struct device' will not be visible outside of this function [-Werror,-Wvisibility]

Make sure the struct tag is known before declaring a function
that uses it as an argument.

Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy")
Signed-off-by: Arnd Bergmann <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: add SPDX headers
Mauro Carvalho Chehab [Sat, 30 May 2020 05:38:24 +0000 (07:38 +0200)]
media: atomisp: add SPDX headers

This driver is licensed under GPL 2.0, as stated inside their
headers.

Add the proper tag there. We should probably latter cleanup
the reduntant licensing text, but this could be done later,
after we get rid of other abstraction layers.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: staging: atomisp: Check return value from compat_alloc_user_space
Sakari Ailus [Thu, 4 Jun 2020 16:16:21 +0000 (18:16 +0200)]
media: staging: atomisp: Check return value from compat_alloc_user_space

If something gets wrong, return, instead of trying to
convert from a NULL pointer.

Signed-off-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: staging: atomisp: Fix compat IOCTL handling
Sakari Ailus [Thu, 4 Jun 2020 16:16:20 +0000 (18:16 +0200)]
media: staging: atomisp: Fix compat IOCTL handling

Atomisp compat IOCTL handling suffers from the same security issue than
the V4L2 did. Fix this for atomisp.

See more information in patch a1dfb4c48cc1 ("media: v4l2-compat-ioctl32.c:
refactor compat ioctl32 logic").

Signed-off-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: staging: atomisp: Fix atomisp_overlay32 compat handling
Sakari Ailus [Thu, 4 Jun 2020 16:16:19 +0000 (18:16 +0200)]
media: staging: atomisp: Fix atomisp_overlay32 compat handling

The struct atomisp_overlay contains overlay_start_x and overlay_start_y
fields. Instead of copying the value of the overlay_start_x field between
the two structs, the value of the overlay_start_y field of the compat
struct was copied to the overlay_start_x field of the 64-bit kernel struct
in get operation and back in put. The overlay_start_x field value was not
copied from or to the user space struct.

Fix this so that the value of overlay_start_x is copied to overlay_start_x
and the value of overlay_start_y is copied to overlay_start_y.

Also do copy blend_overlay_perc_u field only once.

Signed-off-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: staging: atomisp: There's no struct atomisp_dvs2_coefficients
Sakari Ailus [Thu, 4 Jun 2020 16:16:18 +0000 (18:16 +0200)]
media: staging: atomisp: There's no struct atomisp_dvs2_coefficients

It's called struct atomisp_dis_coefficients.

Signed-off-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: add some debug messages when binaries are used
Mauro Carvalho Chehab [Fri, 29 May 2020 08:41:02 +0000 (10:41 +0200)]
media: atomisp: add some debug messages when binaries are used

The ISP firmware logic is complex, as several binaries are
contained into a single file.

Print debug messages:
- with a stack dump if binary not found;
- when a firmware is selected.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: add a debug message at hmm free
Mauro Carvalho Chehab [Fri, 29 May 2020 10:18:25 +0000 (12:18 +0200)]
media: atomisp: add a debug message at hmm free

In order to check if aren't there any memory leaks, let's
add a debug print for hmm_free().

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: allow passing firmware name at modprobe time
Mauro Carvalho Chehab [Fri, 29 May 2020 07:56:05 +0000 (09:56 +0200)]
media: atomisp: allow passing firmware name at modprobe time

It can be useful to be able to test different firmware files
at modprobe time, in order to be able to test different
variants without much efforts.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: print firmware data during load
Mauro Carvalho Chehab [Fri, 29 May 2020 07:29:58 +0000 (09:29 +0200)]
media: atomisp: print firmware data during load

While there's a way to list the firmware binaries in runtime,
it is worth to also print it during firmware load.

One advantage is that this code also introduces additional
checks with regards to invalid firmware types, which can be
useful to identify problems.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: provide more details about the firmware binaries
Mauro Carvalho Chehab [Fri, 29 May 2020 06:10:30 +0000 (08:10 +0200)]
media: atomisp: provide more details about the firmware binaries

In order to make easier to identify what a firmware file
contains, add more info at the firmware dump log facility.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agoKVM: selftests: fix sync_with_host() in smm_test
Vitaly Kuznetsov [Wed, 10 Jun 2020 16:41:16 +0000 (18:41 +0200)]
KVM: selftests: fix sync_with_host() in smm_test

It was reported that older GCCs compile smm_test in a way that breaks
it completely:

  kvm_exit:             reason EXIT_CPUID rip 0x4014db info 0 0
  func 7ffffffd idx 830 rax 0 rbx 0 rcx 0 rdx 0, cpuid entry not found
  ...
  kvm_exit:             reason EXIT_MSR rip 0x40abd9 info 0 0
  kvm_msr:              msr_read 487 = 0x0 (#GP)
  ...

Note, '7ffffffd' was supposed to be '80000001' as we're checking for
SVM. Dropping '-O2' from compiler flags help. Turns out, asm block in
sync_with_host() is wrong. We us 'in 0xe, %%al' instruction to sync
with the host and in 'AL' register we actually pass the parameter
(stage) but after sync 'AL' gets written to but GCC thinks the value
is still there and uses it to compute 'EAX' for 'cpuid'.

smm_test can't fully use standard ucall() framework as we need to
write a very simple SMI handler there. Fix the immediate issue by
making RAX input/output operand. While on it, make sync_with_host()
static inline.

Reported-by: Marcelo Bandeira Condotta <[email protected]>
Signed-off-by: Vitaly Kuznetsov <[email protected]>
Message-Id: <20200610164116[email protected]>
Reviewed-by: Jim Mattson <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
4 years agoKVM: async_pf: Inject 'page ready' event only if 'page not present' was previously...
Vitaly Kuznetsov [Wed, 10 Jun 2020 17:55:32 +0000 (19:55 +0200)]
KVM: async_pf: Inject 'page ready' event only if 'page not present' was previously injected

'Page not present' event may or may not get injected depending on
guest's state. If the event wasn't injected, there is no need to
inject the corresponding 'page ready' event as the guest may get
confused. E.g. Linux thinks that the corresponding 'page not present'
event wasn't delivered *yet* and allocates a 'dummy entry' for it.
This entry is never freed.

Note, 'wakeup all' events have no corresponding 'page not present'
event and always get injected.

s390 seems to always be able to inject 'page not present', the
change is effectively a nop.

Suggested-by: Vivek Goyal <[email protected]>
Signed-off-by: Vitaly Kuznetsov <[email protected]>
Message-Id: <20200610175532[email protected]>
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=208081
Signed-off-by: Paolo Bonzini <[email protected]>
4 years agoKVM: async_pf: Cleanup kvm_setup_async_pf()
Vitaly Kuznetsov [Wed, 10 Jun 2020 17:55:31 +0000 (19:55 +0200)]
KVM: async_pf: Cleanup kvm_setup_async_pf()

schedule_work() returns 'false' only when the work is already on the queue
and this can't happen as kvm_setup_async_pf() always allocates a new one.
Also, to avoid potential race, it makes sense to to schedule_work() at the
very end after we've added it to the queue.

While on it, do some minor cleanup. gfn_to_pfn_async() mentioned in a
comment does not currently exist and, moreover, we can check
kvm_is_error_hva() at the very beginning, before we try to allocate work so
'retry_sync' label can go away completely.

Signed-off-by: Vitaly Kuznetsov <[email protected]>
Message-Id: <20200610175532[email protected]>
Reviewed-by: Sean Christopherson <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
4 years agokvm: i8254: remove redundant assignment to pointer s
Colin Ian King [Tue, 9 Jun 2020 23:31:21 +0000 (00:31 +0100)]
kvm: i8254: remove redundant assignment to pointer s

The pointer s is being assigned a value that is never read, the
assignment is redundant and can be removed.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <[email protected]>
Message-Id: <20200609233121.1118683[email protected]>
Fixes: 7837699fa6d7 ("KVM: In kernel PIT model")
Signed-off-by: Paolo Bonzini <[email protected]>
4 years agoKVM: x86: respect singlestep when emulating instruction
Felipe Franciosi [Tue, 19 May 2020 08:11:22 +0000 (08:11 +0000)]
KVM: x86: respect singlestep when emulating instruction

When userspace configures KVM_GUESTDBG_SINGLESTEP, KVM will manage the
presence of X86_EFLAGS_TF via kvm_set/get_rflags on vcpus. The actual
rflag bit is therefore hidden from callers.

That includes init_emulate_ctxt() which uses the value returned from
kvm_get_flags() to set ctxt->tf. As a result, x86_emulate_instruction()
will skip a single step, leaving singlestep_rip stale and not returning
to userspace.

This resolves the issue by observing the vcpu guest_debug configuration
alongside ctxt->tf in x86_emulate_instruction(), performing the single
step if set.

Cc: [email protected]
Signed-off-by: Felipe Franciosi <[email protected]>
Message-Id: <20200519081048[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
4 years agoKVM: selftests: Don't probe KVM_CAP_HYPERV_ENLIGHTENED_VMCS when nested VMX is unsupp...
Vitaly Kuznetsov [Wed, 10 Jun 2020 13:58:47 +0000 (15:58 +0200)]
KVM: selftests: Don't probe KVM_CAP_HYPERV_ENLIGHTENED_VMCS when nested VMX is unsupported

KVM_CAP_HYPERV_ENLIGHTENED_VMCS will be reported as supported even when
nested VMX is not, fix evmcs_test/hyperv_cpuid tests to check for both.

Signed-off-by: Vitaly Kuznetsov <[email protected]>
Message-Id: <20200610135847[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
4 years agoKVM: selftests: do not substitute SVM/VMX check with KVM_CAP_NESTED_STATE check
Vitaly Kuznetsov [Wed, 10 Jun 2020 13:58:46 +0000 (15:58 +0200)]
KVM: selftests: do not substitute SVM/VMX check with KVM_CAP_NESTED_STATE check

state_test/smm_test use KVM_CAP_NESTED_STATE check as an indicator for
nested VMX/SVM presence and this is incorrect. Check for the required
features dirrectly.

Signed-off-by: Vitaly Kuznetsov <[email protected]>
Message-Id: <20200610135847[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
4 years agoMerge branch 'kvm-basic-exit-reason' into HEAD
Paolo Bonzini [Tue, 9 Jun 2020 10:08:48 +0000 (06:08 -0400)]
Merge branch 'kvm-basic-exit-reason' into HEAD

Using a topic branch so that stable branches can simply cherry-pick the
patch.

Reviewed-by: Oliver Upton <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
4 years agomedia: atomisp: get rid of system_types.h
Mauro Carvalho Chehab [Fri, 29 May 2020 06:15:52 +0000 (08:15 +0200)]
media: atomisp: get rid of system_types.h

This is just a wrapper for system_local.h.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: get rid of a bunch of other wrappers
Mauro Carvalho Chehab [Thu, 28 May 2020 10:35:44 +0000 (12:35 +0200)]
media: atomisp: get rid of a bunch of other wrappers

There are too many wrapper functions at atomisp_compat_css20.c.

Get rid of another set of such wrappers.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: don't cause a warn if probe failed
Mauro Carvalho Chehab [Thu, 28 May 2020 10:11:57 +0000 (12:11 +0200)]
media: atomisp: don't cause a warn if probe failed

When probe fails, it is possible that hmm_init() to not be
called. On such case, hmm_cleanup() will cause a WARN_ON().

Avoid it by adding an explicit check at hmm_cleanup() to
ensure that the hmm code was properly initialized.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: get rid of an error abstraction layer
Mauro Carvalho Chehab [Fri, 29 May 2020 06:42:56 +0000 (08:42 +0200)]
media: atomisp: get rid of an error abstraction layer

There is an abstraction layer there meant to convert to
the Linux standard error codes. As the driver now use
such errors everywhere. we can get rid of this.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: get rid of non-Linux error codes
Mauro Carvalho Chehab [Thu, 28 May 2020 08:01:53 +0000 (10:01 +0200)]
media: atomisp: get rid of non-Linux error codes

The atomisp driver has its own error codes under the
ia_css_err.h file. On several places, those got already
replaced by standard error codes, but there are still a
lot more to be fixed.

Let's get rid of all of those, mapping them into
the already-existing set of Linux error codes.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: do another round of coding style cleanup
Mauro Carvalho Chehab [Thu, 28 May 2020 08:29:31 +0000 (10:29 +0200)]
media: atomisp: do another round of coding style cleanup

Run checkpatch --fix-inline again, in order to get rid
of some additional issues that got introduced (or that
checkpatch can now detect).

This should help preventing receiving random cleanups,
while keeping the code on a better shape.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: remove some trivial wrappers from compat css20
Mauro Carvalho Chehab [Thu, 28 May 2020 07:31:23 +0000 (09:31 +0200)]
media: atomisp: remove some trivial wrappers from compat css20

There are tons of code inside atomisp_compat_css20.c, but
several of them are just trivial wrappers to other functions.

Getting rid of all of them will take some time, but let's
start getting rid of some of the trivial ones.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: avoid an extra memset() when alloc memory
Mauro Carvalho Chehab [Thu, 28 May 2020 07:17:28 +0000 (09:17 +0200)]
media: atomisp: avoid an extra memset() when alloc memory

Use the variant which zeroes the memory when allocating,
instead of having an explicit memset.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: Remove binary_supports_input_format
Nathan Chancellor [Wed, 27 May 2020 07:11:50 +0000 (09:11 +0200)]
media: atomisp: Remove binary_supports_input_format

Clang warns:

drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c:1707:64:
warning: implicit conversion from enumeration type 'const enum
ia_css_frame_format' to different enumeration type 'enum
atomisp_input_format' [-Wenum-conversion]
        binary_supports_input_format(xcandidate, req_in_info->format));
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~             ~~~~~~~~~~~~~^~~~~~

As it turns out, binary_supports_input_format only asserts that
xcandidate is not NULL and just returns true so this call is never
actually made.

There are other functions that are called that assert info is not NULL
so this function actually serves no purpose. Remove it. It can be
brought back if needed later.

Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Signed-off-by: Nathan Chancellor <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: Avoid overflow in compute_blending
Nathan Chancellor [Wed, 27 May 2020 07:11:49 +0000 (09:11 +0200)]
media: atomisp: Avoid overflow in compute_blending

Clang warns:

drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c:129:35:
warning: implicit conversion from 'unsigned long' to 'int32_t' (aka
'int') changes value from 18446744073709543424 to -8192
[-Wconstant-conversion]
        return MAX(MIN(isp_strength, 0), -XNR_BLENDING_SCALE_FACTOR);
        ~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~

XNR_BLENDING_SCALE_FACTOR is BIT(13), or 8192, which will easily fit
into a signed 32-bit integer. However, it is an unsigned long, which
means that negating it is the same as subtracting that value from
ULONG_MAX + 1, which causes it to be larger than a signed 32-bit
integer so it gets implicitly converted.

We can avoid this by using the variable isp_scale, which holds the value
of XNR_BLENDING_SCALE_FACTOR already, where the implicit conversion from
unsigned long to s32 already happened. If that were to ever overflow,
clang would warn: https://godbolt.org/z/EeSxLG

Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Signed-off-by: Nathan Chancellor <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: Remove unnecessary NULL check in atomisp_param
Nathan Chancellor [Wed, 27 May 2020 07:11:48 +0000 (09:11 +0200)]
media: atomisp: Remove unnecessary NULL check in atomisp_param

Clang warns:

drivers/staging/media/atomisp/pci/atomisp_cmd.c:4278:17: warning:
address of 'config->info' will always evaluate to 'true'
[-Wpointer-bool-conversion]
                if (!&config->info) {
                    ~ ~~~~~~~~^~~~

config cannot be NULL because it comes from an ioctl, which ensures that
the user is not giving us an invalid pointer through copy_from_user. If
config is not NULL, info cannot be NULL. Remove this check.

Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Signed-off-by: Nathan Chancellor <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: Remove unnecessary NULL checks in ia_css_pipe_load_extension
Nathan Chancellor [Wed, 27 May 2020 07:11:47 +0000 (09:11 +0200)]
media: atomisp: Remove unnecessary NULL checks in ia_css_pipe_load_extension

Clang warns:

../drivers/staging/media/atomisp/pci/sh_css.c:8537:14: warning: address
of 'pipe->output_stage' will always evaluate to 'true'
[-Wpointer-bool-conversion]
                if (&pipe->output_stage)
                ~~   ~~~~~~^~~~~~~~~~~~
../drivers/staging/media/atomisp/pci/sh_css.c:8545:14: warning: address
of 'pipe->vf_stage' will always evaluate to 'true'
[-Wpointer-bool-conversion]
                if (&pipe->vf_stage)
                ~~   ~~~~~~^~~~~~~~

output_stage and vf_stage are pointers in the middle of a struct, their
addresses cannot be NULL if pipe is not NULL and pipe is already checked
for NULL in this function. Simplify this if block.

Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Signed-off-by: Nathan Chancellor <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: Remove second increment of count in atomisp_subdev_probe
Nathan Chancellor [Wed, 27 May 2020 07:11:45 +0000 (09:11 +0200)]
media: atomisp: Remove second increment of count in atomisp_subdev_probe

Clang warns:

../drivers/staging/media/atomisp/pci/atomisp_v4l2.c:1097:3: warning:
variable 'count' is incremented both in the loop header and in the loop
body [-Wfor-loop-analysis]
                count++;
                ^

This was probably unintentional, remove it.

Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Reported-by: kbuild test robot <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: Clean up if block in sh_css_sp_init_stage
Nathan Chancellor [Wed, 27 May 2020 07:11:44 +0000 (09:11 +0200)]
media: atomisp: Clean up if block in sh_css_sp_init_stage

Clang warns:

../drivers/staging/media/atomisp/pci/sh_css_sp.c:1039:23: warning:
address of 'binary->in_frame_info' will always evaluate to 'true'
[-Wpointer-bool-conversion]
                } else if (&binary->in_frame_info) {
                       ~~   ~~~~~~~~^~~~~~~~~~~~~

in_frame_info is not a pointer so if binary is not NULL, in_frame_info's
address cannot be NULL. Change this to an else since it will always be
evaluated as one.

While we are here, clean up this if block. The contents of both if
blocks are the same but a check against "stage == 0" is added when
ISP2401 is defined. USE_INPUT_SYSTEM_VERSION_2401 is only defined when
isp2401_system_global.h is included, which only happens when ISP2401. In
other words, USE_INPUT_SYSTEM_VERSION_2401 always requires ISP2401 to be
defined so the '#ifndef ISP2401' makes no sense. Remove that part of the
block to simplify everything.

Link: https://github.com/ClangBuiltLinux/linux/issues/1036
Reported-by: kbuild test robot <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: avoid OOPS due to non-existing ref_frames
Mauro Carvalho Chehab [Wed, 27 May 2020 13:46:54 +0000 (15:46 +0200)]
media: atomisp: avoid OOPS due to non-existing ref_frames

stage->args->delay_frames array could point to NULL frames.

What's weird is that we didn't notice this behavior with the
Intel Aero Yocto code.

Handle it, while adding a notice at the code, as this could
be due to some broken pipeline setup.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: remove kvmalloc/kvcalloc abstractions
Mauro Carvalho Chehab [Wed, 27 May 2020 12:23:31 +0000 (14:23 +0200)]
media: atomisp: remove kvmalloc/kvcalloc abstractions

The sh_css layer adds an abstraction for kvmalloc/kvcalloc.

Get rid of them. Most of the work here was done by this
small coccinelle script:

<cocci>
@@
expression size;
@@

- sh_css_malloc(size)
+ kvmalloc(size, GFP_KERNEL)

@@
expression n;
expression size;
@@

- sh_css_calloc(n, size)
+ kvcalloc(n, size, GFP_KERNEL)
</cocci>

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: add more comments about frame allocation
Mauro Carvalho Chehab [Wed, 27 May 2020 12:21:26 +0000 (14:21 +0200)]
media: atomisp: add more comments about frame allocation

The frame allocation logic happens differently for userptr
or normal mmap. On a quick look, this sounded to be unbalanced,
but the logic should actually work for both cases.

Add an extra comment to reflect it.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: add debug functions for received events
Mauro Carvalho Chehab [Wed, 27 May 2020 12:19:27 +0000 (14:19 +0200)]
media: atomisp: add debug functions for received events

For debugging purposes, it helps to know what event
was actually received.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: improve warning for IRQ enable function
Mauro Carvalho Chehab [Wed, 27 May 2020 12:18:00 +0000 (14:18 +0200)]
media: atomisp: improve warning for IRQ enable function

If something gets wrong when enabling or disabling an IRQ,
we should know better about what happened.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: add debug for hmm alloc
Mauro Carvalho Chehab [Tue, 26 May 2020 12:35:16 +0000 (14:35 +0200)]
media: atomisp: add debug for hmm alloc

The hmm code is still complex and has bugs. Add a debug print
when memory gets allocated, in order to help identifying what's
happening out there.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: use pin_user_pages() for memory allocation
Mauro Carvalho Chehab [Tue, 26 May 2020 10:07:26 +0000 (12:07 +0200)]
media: atomisp: use pin_user_pages() for memory allocation

Instead of using a hacked version of an old copy of
get_user_pages(), use pin_user_pages().

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: fix driver caps
Mauro Carvalho Chehab [Tue, 26 May 2020 11:05:21 +0000 (13:05 +0200)]
media: atomisp: fix driver caps

This device driver is not MC-centric. So, remove the wrong
caps from it.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: use Yocto Aero default hmm pool sizes
Mauro Carvalho Chehab [Tue, 26 May 2020 08:45:00 +0000 (10:45 +0200)]
media: atomisp: use Yocto Aero default hmm pool sizes

Yocto Aero driver has a different default for hmm pools.

Use the definitions there.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: add debug message to help debugging hmm code
Mauro Carvalho Chehab [Tue, 26 May 2020 08:11:14 +0000 (10:11 +0200)]
media: atomisp: add debug message to help debugging hmm code

The hmm code is partially based on a fork from 3.10 code,
and has bugs.

Add debug there to help tracking what happens there.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: hmm_bo: untag user pointers
Mauro Carvalho Chehab [Tue, 26 May 2020 07:45:16 +0000 (09:45 +0200)]
media: atomisp: hmm_bo: untag user pointers

The kernel ABI was extended to allow pass tagged user pointers.

Untag the pointers in this function.

Fixes: d93445225cd3 ("uaccess: add noop untagged_addr definition")
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: get rid of memory_access.c
Mauro Carvalho Chehab [Tue, 26 May 2020 06:40:16 +0000 (08:40 +0200)]
media: atomisp: get rid of memory_access.c

Now that we have everything in place, we can get rid of the
memory_access abstraction layer.

Now, everything related to heterogeneous memory management
(hmm) is under hmm.c & related pools.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: change the type returned by mmgr alloc
Mauro Carvalho Chehab [Tue, 26 May 2020 06:13:27 +0000 (08:13 +0200)]
media: atomisp: change the type returned by mmgr alloc

The mmgr alloc code returns a different type than hmm, due to
some abstraction layer.

Change the driver to use just one type to represent the
hmm memory.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: get rid of unused memory_realloc code
Mauro Carvalho Chehab [Tue, 26 May 2020 06:10:18 +0000 (08:10 +0200)]
media: atomisp: get rid of unused memory_realloc code

The code for it is commented out, probably because it is
broken or uneeded for the driver to work. So, let's get
rid of it.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: get rid of mmgr_load and mmgr_store
Mauro Carvalho Chehab [Tue, 26 May 2020 06:02:00 +0000 (08:02 +0200)]
media: atomisp: get rid of mmgr_load and mmgr_store

Those functions are just wrappers for hmm_load/hmm_store.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: go one step further to drop ia_css_memory_access.c
Mauro Carvalho Chehab [Mon, 25 May 2020 10:46:23 +0000 (12:46 +0200)]
media: atomisp: go one step further to drop ia_css_memory_access.c

Move the attrs handling into hmm, simplifying even further
what the ia_css_memory_access.c file does.

Yet, the returned type for ia_css_memory_access.c is an
integer, instead of a pointer.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: reduce abstraction at ia_css_memory_access
Mauro Carvalho Chehab [Mon, 25 May 2020 09:33:24 +0000 (11:33 +0200)]
media: atomisp: reduce abstraction at ia_css_memory_access

Yet another memory abstraction layer. Getting rid of this
may be a little trickier, but let's reduce it to a minimal.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: get rid of the hrt/hive_isp_css_mm_hrt abstraction layer
Mauro Carvalho Chehab [Mon, 25 May 2020 08:53:20 +0000 (10:53 +0200)]
media: atomisp: get rid of the hrt/hive_isp_css_mm_hrt abstraction layer

Simplify the code by removing this extra memory management
abstraction layer.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: simplify hive_isp_css_mm_hrt wrapper
Mauro Carvalho Chehab [Mon, 25 May 2020 08:11:49 +0000 (10:11 +0200)]
media: atomisp: simplify hive_isp_css_mm_hrt wrapper

The code there is a wrapper for hmm/ wrapper. Simplify it,
and get rid of ION-specific code.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: fix a handful of spelling mistakes
Colin Ian King [Thu, 21 May 2020 19:44:32 +0000 (21:44 +0200)]
media: atomisp: fix a handful of spelling mistakes

There are several spelling mistakes in various messages and literal
strings. Fix these.

Signed-off-by: Colin Ian King <[email protected]>
Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: fix size of delay_frames array
Mauro Carvalho Chehab [Sun, 24 May 2020 07:11:07 +0000 (09:11 +0200)]
media: atomisp: fix size of delay_frames array

Right now, the variables that define the max number of
delay frames is defined as:

#define VIDEO_FRAME_DELAY 2
#define MAX_NUM_VIDEO_DELAY_FRAMES (VIDEO_FRAME_DELAY + 1)
#define NUM_PREVIEW_DVS_FRAMES          (2)
#define MAX_NUM_DELAY_FRAMES   MAX(MAX_NUM_VIDEO_DELAY_FRAMES, NUM_PREVIEW_DVS_FRAMES)

In other words, we have:
MAX_NUM_VIDEO_DELAY_FRAMES = 3
MAX_NUM_DELAY_FRAMES = 2

The MAX_NUM_DELAY_FRAMES macro is used only only when allocating
memory. On all other parts, including looping over such array,
MAX_NUM_VIDEO_DELAY_FRAMES is used instead, like:

void sh_css_binary_args_reset(struct sh_css_binary_args *args)
{
unsigned int i;
...

for (i = 0; i < MAX_NUM_VIDEO_DELAY_FRAMES; i++)
args->delay_frames[i] = NULL;

Which will cause buffer overflows, with may override the next array
(tnr_frames[]).

In practice, this may not be causing real issues, as the code
checks for num_delay_frames on some parts (but not everywhere).

So, get rid of the smallest value.

Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: drop a cast for a const argument
Mauro Carvalho Chehab [Sun, 24 May 2020 06:56:58 +0000 (08:56 +0200)]
media: atomisp: drop a cast for a const argument

Some arguments for tnf and ref settings are meant to be const, but
they're defined without such annotation. Due to that, there's an
ugly cast at sh_css_sp.c.

Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: partially get rid of one abstraction layer
Mauro Carvalho Chehab [Thu, 21 May 2020 10:07:53 +0000 (12:07 +0200)]
media: atomisp: partially get rid of one abstraction layer

The very same macros are defined as CSS_foo and IA_CSS_foo.

Remove this abstraction, as it just make things confusing,
for no good reason.

Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: make it use dbg_level to control debug level
Mauro Carvalho Chehab [Sat, 23 May 2020 14:59:14 +0000 (16:59 +0200)]
media: atomisp: make it use dbg_level to control debug level

This driver has 3 different types of debug messages:

- dev_dbg()
- dbg_level
- ia_css_debug_trace_level

Which is crazy. Ideally, it shold just use dev_dbg()
everywhere, but for now let's unify the last two machanisms.

Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: get rid of some old broken debug code
Mauro Carvalho Chehab [Sat, 23 May 2020 14:44:42 +0000 (16:44 +0200)]
media: atomisp: get rid of some old broken debug code

It sounds that someone once changed the debug level at compile
time for some testing, but forgot to remove the legacy code after
finishing debuging it.

Get rid of the dead code.

Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: update TODO list
Mauro Carvalho Chehab [Fri, 22 May 2020 12:01:00 +0000 (14:01 +0200)]
media: atomisp: update TODO list

Let's reflect the current status at the TODO list, as other
developers can help addressing issues over there.

Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: don't flood dmesg with -EAGAIN return codes
Mauro Carvalho Chehab [Thu, 21 May 2020 08:04:49 +0000 (10:04 +0200)]
media: atomisp: don't flood dmesg with -EAGAIN return codes

Using DQBUF on non-blocking mode will return -EAGAIN
if nothing arrives. Printing it has no value, even for debug
purposes. So, only display real return codes.

Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: improve debug messages for set format
Mauro Carvalho Chehab [Wed, 20 May 2020 22:10:04 +0000 (00:10 +0200)]
media: atomisp: improve debug messages for set format

There are several error conditions that don't print anything,
making harder to identify bugs at the code there.

Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: avoid a copy of v4l2_mbus_framefmt at stack
Mauro Carvalho Chehab [Thu, 21 May 2020 07:43:01 +0000 (09:43 +0200)]
media: atomisp: avoid a copy of v4l2_mbus_framefmt at stack

There's no reason to copy isp_sink_fmt, as the driver
uses it for read-only purposes.

Linux stack is a precious resource. Let's avoid wasting it.

Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: reduce debug printk rate when IRQs are received
Mauro Carvalho Chehab [Thu, 21 May 2020 07:05:11 +0000 (09:05 +0200)]
media: atomisp: reduce debug printk rate when IRQs are received

Currently, when an EOF IRQ is received, it generates two messages:

[   59.191893] atomisp-isp2 0000:00:03.0: irq:0x200000
[   59.191913] atomisp-isp2 0000:00:03.0: atomisp_isr EOF exp_id 142, asd 0

Flooding the dmesg with lots of messages per second. The same
pattern happens for all other IRQs.

Change the logic for printing just one message per IRQ and
rate-limit those, as, for debugging purposes, it is usually
interesting to know that IRQs are being received, but not
displaying every single one.

Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: get rid of hmm_vm.c
Mauro Carvalho Chehab [Wed, 20 May 2020 21:04:09 +0000 (23:04 +0200)]
media: atomisp: get rid of hmm_vm.c

This is not used anywhere. So, let's trash it.

Acked-by: Sakari Ailus <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agomedia: atomisp: fix pipeline initialization code
Mauro Carvalho Chehab [Wed, 20 May 2020 05:28:50 +0000 (07:28 +0200)]
media: atomisp: fix pipeline initialization code

The code under load_primary_binaries() is complex and
were hard to understand, because it used to have lots
of ifdefs and broken identation.

The patch which cleaned it and removed the version-specific
ifdefs added a regression.

Solve it.

Fixes: 3c0538fbad9f ("media: atomisp: get rid of most checks for ISP2401 version")
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
4 years agoio_uring: fix io_kiocb.flags modification race in IOPOLL mode
Xiaoguang Wang [Thu, 11 Jun 2020 15:39:36 +0000 (23:39 +0800)]
io_uring: fix io_kiocb.flags modification race in IOPOLL mode

While testing io_uring in arm, we found sometimes io_sq_thread() keeps
polling io requests even though there are not inflight io requests in
block layer. After some investigations, found a possible race about
io_kiocb.flags, see below race codes:
  1) in the end of io_write() or io_read()
    req->flags &= ~REQ_F_NEED_CLEANUP;
    kfree(iovec);
    return ret;

  2) in io_complete_rw_iopoll()
    if (res != -EAGAIN)
        req->flags |= REQ_F_IOPOLL_COMPLETED;

In IOPOLL mode, io requests still maybe completed by interrupt, then
above codes are not safe, concurrent modifications to req->flags, which
is not protected by lock or is not atomic modifications. I also had
disassemble io_complete_rw_iopoll() in arm:
   req->flags |= REQ_F_IOPOLL_COMPLETED;
   0xffff000008387b18 <+76>:    ldr     w0, [x19,#104]
   0xffff000008387b1c <+80>:    orr     w0, w0, #0x1000
   0xffff000008387b20 <+84>:    str     w0, [x19,#104]

Seems that the "req->flags |= REQ_F_IOPOLL_COMPLETED;" is  load and
modification, two instructions, which obviously is not atomic.

To fix this issue, add a new iopoll_completed in io_kiocb to indicate
whether io request is completed.

Signed-off-by: Xiaoguang Wang <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
4 years agoKVM: nVMX: Consult only the "basic" exit reason when routing nested exit
Sean Christopherson [Thu, 27 Feb 2020 17:44:30 +0000 (09:44 -0800)]
KVM: nVMX: Consult only the "basic" exit reason when routing nested exit

Consult only the basic exit reason, i.e. bits 15:0 of vmcs.EXIT_REASON,
when determining whether a nested VM-Exit should be reflected into L1 or
handled by KVM in L0.

For better or worse, the switch statement in nested_vmx_exit_reflected()
currently defaults to "true", i.e. reflects any nested VM-Exit without
dedicated logic.  Because the case statements only contain the basic
exit reason, any VM-Exit with modifier bits set will be reflected to L1,
even if KVM intended to handle it in L0.

Practically speaking, this only affects EXIT_REASON_MCE_DURING_VMENTRY,
i.e. a #MC that occurs on nested VM-Enter would be incorrectly routed to
L1, as "failed VM-Entry" is the only modifier that KVM can currently
encounter.  The SMM modifiers will never be generated as KVM doesn't
support/employ a SMI Transfer Monitor.  Ditto for "exit from enclave",
as KVM doesn't yet support virtualizing SGX, i.e. it's impossible to
enter an enclave in a KVM guest (L1 or L2).

Fixes: 644d711aa0e1 ("KVM: nVMX: Deciding if L0 or L1 should handle an L2 exit")
Cc: Jim Mattson <[email protected]>
Cc: Xiaoyao Li <[email protected]>
Cc: [email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Message-Id: <20200227174430[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
4 years agoumem: remove redundant initialization of variable ret
Colin Ian King [Thu, 11 Jun 2020 14:35:27 +0000 (15:35 +0100)]
umem: remove redundant initialization of variable ret

The variable ret is being initialized with a value that is never read
and it is being updated later with a new value.  The initialization is
redundant and can be removed.

Signed-off-by: Colin Ian King <[email protected]>
Addresses-Coverity: ("Unused value")
Signed-off-by: Jens Axboe <[email protected]>
4 years agopktcdvd: remove redundant initialization of variable ret
Colin Ian King [Thu, 11 Jun 2020 14:30:14 +0000 (15:30 +0100)]
pktcdvd: remove redundant initialization of variable ret

The variable ret is being initialized with a value that is never read
and it is being updated later with a new value.  The initialization is
redundant and can be removed.

Signed-off-by: Colin Ian King <[email protected]>
Addresses-Coverity: ("Unused value")
Signed-off-by: Jens Axboe <[email protected]>
4 years agonvmet: fail outstanding host posted AEN req
Chaitanya Kulkarni [Tue, 9 Jun 2020 23:55:14 +0000 (16:55 -0700)]
nvmet: fail outstanding host posted AEN req

In function nvmet_async_event_process() we only process AENs iff
there is an open slot on the ctrl->async_event_cmds[] && aen
event list posted by the target is not empty. This keeps host
posted AEN outstanding if target generated AEN list is empty.
We do cleanup the target generated entries from the aen list in
nvmet_ctrl_free()-> nvmet_async_events_free() but we don't
process AEN posted by the host. This leads to following problem :-

When processing admin sq at the time of nvmet_sq_destroy() holds
an extra percpu reference(atomic value = 1), so in the following code
path after switching to atomic rcu, release function (nvmet_sq_free())
is not getting called which blocks the sq->free_done in
nvmet_sq_destroy() :-

nvmet_sq_destroy()
 percpu_ref_kill_and_confirm()
 - __percpu_ref_switch_mode()
 --  __percpu_ref_switch_to_atomic()
 ---   call_rcu() -> percpu_ref_switch_to_atomic_rcu()
 ----     /* calls switch callback */
 - percpu_ref_put()
 -- percpu_ref_put_many(ref, 1)
 --- else if (unlikely(atomic_long_sub_and_test(nr, &ref->count)))
 ----   ref->release(ref); <---- Not called.

This results in indefinite hang:-

  void nvmet_sq_destroy(struct nvmet_sq *sq)
...
          if (ctrl && ctrl->sqs && ctrl->sqs[0] == sq) {
                  nvmet_async_events_process(ctrl, status);
                  percpu_ref_put(&sq->ref);
          }
          percpu_ref_kill_and_confirm(&sq->ref, nvmet_confirm_sq);
          wait_for_completion(&sq->confirm_done);
          wait_for_completion(&sq->free_done); <-- Hang here

Which breaks the further disconnect sequence. This problem seems to be
introduced after commit 64f5e9cdd711b ("nvmet: fix memory leak when
removing namespaces and controllers concurrently").

This patch processes ctrl->async_event_cmds[] in the admin sq destroy()
context irrespetive of aen_list. Also we get rid of the controller's
aen_list processing in the nvmet_sq_destroy() context and just ignore
ctrl->aen_list.

This results in nvmet_async_events_process() being called from workqueue
context so we adjust the code accordingly.

Fixes: 64f5e9cdd711 ("nvmet: fix memory leak when removing namespaces and controllers concurrently ")
Signed-off-by: Chaitanya Kulkarni <[email protected]>
Reviewed-by: Sagi Grimberg <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
4 years agonvme-pci: use simple suspend when a HMB is enabled
Christoph Hellwig [Wed, 3 Jun 2020 06:24:17 +0000 (08:24 +0200)]
nvme-pci: use simple suspend when a HMB is enabled

While the NVMe specification allows the device to access the host memory
buffer in host DRAM from all power states, hosts will fail access to
DRAM during S3 and similar power states.

Fixes: d916b1be94b6 ("nvme-pci: use host managed power state for suspend")
Signed-off-by: Christoph Hellwig <[email protected]>
Reviewed-by: Keith Busch <[email protected]>
Reviewed-by: Sagi Grimberg <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
4 years agonvme-fc: don't call nvme_cleanup_cmd() for AENs
Daniel Wagner [Fri, 29 May 2020 11:37:40 +0000 (13:37 +0200)]
nvme-fc: don't call nvme_cleanup_cmd() for AENs

Asynchronous event notifications do not have an associated request.
When fcp_io() fails we unconditionally call nvme_cleanup_cmd() which
leads to a crash.

Fixes: 16686f3a6c3c ("nvme: move common call to nvme_cleanup_cmd to core layer")
Signed-off-by: Daniel Wagner <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Reviewed-by: James Smart <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
4 years agonvmet-tcp: constify nvmet_tcp_ops
Max Gurtovoy [Mon, 1 Jun 2020 17:05:20 +0000 (20:05 +0300)]
nvmet-tcp: constify nvmet_tcp_ops

nvmet_tcp_ops is never modified and can be made const to allow the
compiler to put it in read-only memory, as done in other transports.

Before:
   text    data     bss     dec     hex filename
  16164     160      12   16336    3fd0 drivers/nvme/target/tcp.o

After:
   text    data     bss     dec     hex filename
  16277      64      12   16353    3fe1 drivers/nvme/target/tcp.o

Signed-off-by: Max Gurtovoy <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Reviewed-by: Israel Rukshin <[email protected]>
Acked-by: Sagi Grimberg <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
4 years agonvme-tcp: constify nvme_tcp_mq_ops and nvme_tcp_admin_mq_ops
Rikard Falkeborn [Thu, 28 May 2020 22:25:07 +0000 (00:25 +0200)]
nvme-tcp: constify nvme_tcp_mq_ops and nvme_tcp_admin_mq_ops

nvme_tcp_mq_ops and nvme_tcp_admin_mq_ops are never modified and can be
made const to allow the compiler to put them in read-only memory.

Before:
   text    data     bss     dec     hex filename
  53102    6885     576   60563    ec93 drivers/nvme/host/tcp.o

After:
   text    data     bss     dec     hex filename
  53422    6565     576   60563    ec93 drivers/nvme/host/tcp.o

Signed-off-by: Rikard Falkeborn <[email protected]>
Acked-by: Sagi Grimberg <[email protected]>
Reviewed-by: Max Gurtovoy <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
4 years agonvme: do not call del_gendisk() on a disk that was never added
Niklas Cassel [Sun, 7 Jun 2020 11:45:20 +0000 (13:45 +0200)]
nvme: do not call del_gendisk() on a disk that was never added

device_add_disk() is negated by del_gendisk().
alloc_disk_node() is negated by put_disk().

In nvme_alloc_ns(), device_add_disk() is one of the last things being
called in the success case, and only void functions are being called
after this. Therefore this call should not be negated in the error path.

The superfluous call to del_gendisk() leads to the following prints:
[    7.839975] kobject: '(null)' (000000001ff73734): is not initialized, yet kobject_put() is being called.
[    7.840865] WARNING: CPU: 2 PID: 361 at lib/kobject.c:736 kobject_put+0x70/0x120

Fixes: 33cfdc2aa696 ("nvme: enforce extended LBA format for fabrics metadata")
Signed-off-by: Niklas Cassel <[email protected]>
Reviewed-by: Sagi Grimberg <[email protected]>
Reviewed-by: Max Gurtovoy <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
4 years agotools, bpftool: Fix memory leak in codegen error cases
Tobias Klauser [Wed, 10 Jun 2020 13:08:04 +0000 (15:08 +0200)]
tools, bpftool: Fix memory leak in codegen error cases

Free the memory allocated for the template on error paths in function
codegen.

Signed-off-by: Tobias Klauser <[email protected]>
Signed-off-by: Daniel Borkmann <[email protected]>
Acked-by: Andrii Nakryiko <[email protected]>
Link: https://lore.kernel.org/bpf/[email protected]
4 years agoselftests/bpf: Add cgroup_skb/egress test for load_bytes_relative
YiFei Zhu [Wed, 10 Jun 2020 18:41:40 +0000 (13:41 -0500)]
selftests/bpf: Add cgroup_skb/egress test for load_bytes_relative

When cgroup_skb/egress triggers the MAC header is not set. Added a
test that asserts reading MAC header is a -EFAULT but NET header
succeeds. The test result from within the eBPF program is stored in
an 1-element array map that the userspace then reads and asserts on.

Another assertion is added that reading from a large offset, past
the end of packet, returns -EFAULT.

Signed-off-by: YiFei Zhu <[email protected]>
Signed-off-by: Daniel Borkmann <[email protected]>
Reviewed-by: Stanislav Fomichev <[email protected]>
Link: https://lore.kernel.org/bpf/9028ccbea4385a620e69c0a104f469ffd655c01e.1591812755.git.zhuyifei@google.com
4 years agonet/filter: Permit reading NET in load_bytes_relative when MAC not set
YiFei Zhu [Wed, 10 Jun 2020 18:41:39 +0000 (13:41 -0500)]
net/filter: Permit reading NET in load_bytes_relative when MAC not set

Added a check in the switch case on start_header that checks for
the existence of the header, and in the case that MAC is not set
and the caller requests for MAC, -EFAULT. If the caller requests
for NET then MAC's existence is completely ignored.

There is no function to check NET header's existence and as far
as cgroup_skb/egress is concerned it should always be set.

Removed for ptr >= the start of header, considering offset is
bounded unsigned and should always be true. len <= end - mac is
redundant to ptr + len <= end.

Fixes: 3eee1f75f2b9 ("bpf: fix bpf_skb_load_bytes_relative pkt length check")
Signed-off-by: YiFei Zhu <[email protected]>
Signed-off-by: Daniel Borkmann <[email protected]>
Reviewed-by: Stanislav Fomichev <[email protected]>
Link: https://lore.kernel.org/bpf/76bb820ddb6a95f59a772ecbd8c8a336f646b362.1591812755.git.zhuyifei@google.com
4 years agox86/mce/dev-mcelog: Fix -Wstringop-truncation warning about strncpy()
Tony Luck [Wed, 27 May 2020 18:28:08 +0000 (11:28 -0700)]
x86/mce/dev-mcelog: Fix -Wstringop-truncation warning about strncpy()

The kbuild test robot reported this warning:

  arch/x86/kernel/cpu/mce/dev-mcelog.c: In function 'dev_mcelog_init_device':
  arch/x86/kernel/cpu/mce/dev-mcelog.c:346:2: warning: 'strncpy' output \
    truncated before terminating nul copying 12 bytes from a string of the \
    same length [-Wstringop-truncation]

This is accurate, but I don't care that the trailing NUL character isn't
copied. The string being copied is just a magic number signature so that
crash dump tools can be sure they are decoding the right blob of memory.

Use memcpy() instead of strncpy().

Fixes: d8ecca4043f2 ("x86/mce/dev-mcelog: Dynamically allocate space for machine check records")
Reported-by: kbuild test robot <[email protected]>
Signed-off-by: Tony Luck <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agox86/{mce,mm}: Unmap the entire page if the whole page is affected and poisoned
Tony Luck [Wed, 20 May 2020 16:35:46 +0000 (09:35 -0700)]
x86/{mce,mm}: Unmap the entire page if the whole page is affected and poisoned

An interesting thing happened when a guest Linux instance took a machine
check. The VMM unmapped the bad page from guest physical space and
passed the machine check to the guest.

Linux took all the normal actions to offline the page from the process
that was using it. But then guest Linux crashed because it said there
was a second machine check inside the kernel with this stack trace:

do_memory_failure
    set_mce_nospec
         set_memory_uc
              _set_memory_uc
                   change_page_attr_set_clr
                        cpa_flush
                             clflush_cache_range_opt

This was odd, because a CLFLUSH instruction shouldn't raise a machine
check (it isn't consuming the data). Further investigation showed that
the VMM had passed in another machine check because is appeared that the
guest was accessing the bad page.

Fix is to check the scope of the poison by checking the MCi_MISC register.
If the entire page is affected, then unmap the page. If only part of the
page is affected, then mark the page as uncacheable.

This assumes that VMMs will do the logical thing and pass in the "whole
page scope" via the MCi_MISC register (since they unmapped the entire
page).

  [ bp: Adjust to x86/entry changes. ]

Fixes: 284ce4011ba6 ("x86/memory_failure: Introduce {set, clear}_mce_nospec()")
Reported-by: Jue Wang <[email protected]>
Signed-off-by: Tony Luck <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Jue Wang <[email protected]>
Cc: <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agoMerge branch 'x86/entry' into ras/core
Thomas Gleixner [Thu, 11 Jun 2020 13:17:57 +0000 (15:17 +0200)]
Merge branch 'x86/entry' into ras/core

to fixup conflicts in arch/x86/kernel/cpu/mce/core.c so MCE specific follow
up patches can be applied without creating a horrible merge conflict
afterwards.

4 years agox86/entry: Unbreak __irqentry_text_start/end magic
Thomas Gleixner [Wed, 10 Jun 2020 06:37:01 +0000 (08:37 +0200)]
x86/entry: Unbreak __irqentry_text_start/end magic

The entry rework moved interrupt entry code from the irqentry to the
noinstr section which made the irqentry section empty.

This breaks boundary checks which rely on the __irqentry_text_start/end
markers to find out whether a function in a stack trace is
interrupt/exception entry code. This affects the function graph tracer and
filter_irq_stacks().

As the IDT entry points are all sequentialy emitted this is rather simple
to unbreak by injecting __irqentry_text_start/end as global labels.

To make this work correctly:

  - Remove the IRQENTRY_TEXT section from the x86 linker script
  - Define __irqentry so it breaks the build if it's used
  - Adjust the entry mirroring in PTI
  - Remove the redundant kprobes and unwinder bound checks

Reported-by: Qian Cai <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
4 years agox86/entry: __always_inline CR2 for noinstr
Peter Zijlstra [Wed, 3 Jun 2020 11:40:22 +0000 (13:40 +0200)]
x86/entry: __always_inline CR2 for noinstr

vmlinux.o: warning: objtool: exc_page_fault()+0x9: call to read_cr2() leaves .noinstr.text section
vmlinux.o: warning: objtool: exc_page_fault()+0x24: call to prefetchw() leaves .noinstr.text section
vmlinux.o: warning: objtool: exc_page_fault()+0x21: call to kvm_handle_async_pf.isra.0() leaves .noinstr.text section
vmlinux.o: warning: objtool: exc_nmi()+0x1cc: call to write_cr2() leaves .noinstr.text section

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agolockdep: __always_inline more for noinstr
Peter Zijlstra [Wed, 3 Jun 2020 11:40:21 +0000 (13:40 +0200)]
lockdep: __always_inline more for noinstr

vmlinux.o: warning: objtool: debug_locks_off()+0xd: call to __debug_locks_off() leaves .noinstr.text section
vmlinux.o: warning: objtool: match_held_lock()+0x6a: call to look_up_lock_class.isra.0() leaves .noinstr.text section
vmlinux.o: warning: objtool: lock_is_held_type()+0x90: call to lockdep_recursion_finish() leaves .noinstr.text section

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agox86/entry: Re-order #DB handler to avoid *SAN instrumentation
Peter Zijlstra [Wed, 3 Jun 2020 11:40:20 +0000 (13:40 +0200)]
x86/entry: Re-order #DB handler to avoid *SAN instrumentation

vmlinux.o: warning: objtool: exc_debug()+0xbb: call to clear_ti_thread_flag.constprop.0() leaves .noinstr.text section
vmlinux.o: warning: objtool: noist_exc_debug()+0x55: call to clear_ti_thread_flag.constprop.0() leaves .noinstr.text section

Rework things so that handle_debug() looses the noinstr and move the
clear_thread_flag() into that.

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agox86/entry: __always_inline arch_atomic_* for noinstr
Peter Zijlstra [Wed, 3 Jun 2020 11:40:19 +0000 (13:40 +0200)]
x86/entry: __always_inline arch_atomic_* for noinstr

vmlinux.o: warning: objtool: rcu_dynticks_eqs_exit()+0x33: call to arch_atomic_and.constprop.0() leaves .noinstr.text section

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agox86/entry: __always_inline irqflags for noinstr
Peter Zijlstra [Wed, 3 Jun 2020 11:40:18 +0000 (13:40 +0200)]
x86/entry: __always_inline irqflags for noinstr

vmlinux.o: warning: objtool: lockdep_hardirqs_on()+0x65: call to arch_local_save_flags() leaves .noinstr.text section
vmlinux.o: warning: objtool: lockdep_hardirqs_off()+0x5d: call to arch_local_save_flags() leaves .noinstr.text section
vmlinux.o: warning: objtool: lock_is_held_type()+0x35: call to arch_local_irq_save() leaves .noinstr.text section
vmlinux.o: warning: objtool: check_preemption_disabled()+0x31: call to arch_local_save_flags() leaves .noinstr.text section
vmlinux.o: warning: objtool: check_preemption_disabled()+0x33: call to arch_irqs_disabled_flags() leaves .noinstr.text section
vmlinux.o: warning: objtool: lock_is_held_type()+0x2f: call to native_irq_disable() leaves .noinstr.text section

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agox86/entry: __always_inline debugreg for noinstr
Peter Zijlstra [Wed, 3 Jun 2020 11:40:17 +0000 (13:40 +0200)]
x86/entry: __always_inline debugreg for noinstr

vmlinux.o: warning: objtool: exc_debug()+0x21: call to native_get_debugreg() leaves .noinstr.text section

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agox86/idt: Consolidate idt functionality
Thomas Gleixner [Thu, 28 May 2020 14:53:20 +0000 (16:53 +0200)]
x86/idt: Consolidate idt functionality

 - Move load_current_idt() out of line and replace the hideous comment with
   a lockdep assert. This allows to make idt_table and idt_descr static.

 - Mark idt_table read only after the IDT initialization is complete.

 - Shuffle code around to consolidate the #ifdef sections into one.

 - Adapt the F00F bug code.

Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agox86/idt: Cleanup trap_init()
Thomas Gleixner [Thu, 28 May 2020 14:53:19 +0000 (16:53 +0200)]
x86/idt: Cleanup trap_init()

No point in having all the IDT cruft in trap_init(). Move it into the IDT
code and fixup the comments.

Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agox86/idt: Use proper constants for table size
Thomas Gleixner [Thu, 28 May 2020 14:53:18 +0000 (16:53 +0200)]
x86/idt: Use proper constants for table size

Use the actual struct size to calculate the IDT table size instead of
hardcoded values.

Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agox86/idt: Add comments about early #PF handling
Thomas Gleixner [Thu, 28 May 2020 14:53:17 +0000 (16:53 +0200)]
x86/idt: Add comments about early #PF handling

The difference between 32 and 64 bit vs. early #PF handling is not
documented. Replace the FIXME at idt_setup_early_pf() with proper comments.

Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agox86/idt: Mark init only functions __init
Thomas Gleixner [Thu, 28 May 2020 14:53:16 +0000 (16:53 +0200)]
x86/idt: Mark init only functions __init

Since 8175cfbbbfcb ("x86/idt: Remove update_intr_gate()") set_intr_gate()
and idt_setup_from_table() are only called from __init functions. Mark them
as well.

Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
4 years agox86/entry: Rename trace_hardirqs_off_prepare()
Peter Zijlstra [Fri, 29 May 2020 21:27:40 +0000 (23:27 +0200)]
x86/entry: Rename trace_hardirqs_off_prepare()

The typical pattern for trace_hardirqs_off_prepare() is:

  ENTRY
    lockdep_hardirqs_off(); // because hardware
    ... do entry magic
    instrumentation_begin();
    trace_hardirqs_off_prepare();
    ... do actual work
    trace_hardirqs_on_prepare();
    lockdep_hardirqs_on_prepare();
    instrumentation_end();
    ... do exit magic
    lockdep_hardirqs_on();

which shows that it's named wrong, rename it to
trace_hardirqs_off_finish(), as it concludes the hardirq_off transition.

Also, given that the above is the only correct order, make the traditional
all-in-one trace_hardirqs_off() follow suit.

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
This page took 0.115274 seconds and 4 git commands to generate.