Test verifier/direct_stack_access_wraparound.c automatically converted to use inline assembly.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20230325025524.144043-18-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
prog_tests/verifier.c would be used as a host for verifier/*.c tests
migrated to use inline assembly and run from test_progs.
The run_test_aux() function mimics the test_verifier behavior
dropping CAP_SYS_ADMIN upon entry.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20230325025524.144043-6-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Extends test_loader.c:test_loader__run_subtests() by allowing to
execute BPF_PROG_TEST_RUN bpf command for selected programs.
This is similar to functionality provided by test_verifier.
Adds the following new attributes controlling test_loader behavior:
__retval(...)
__retval_unpriv(...)
* If any of these attributes is present, the annotated program would
be executed using libbpf's bpf_prog_test_run_opts() function.
* If __retval is present, the test run would be done for program
loaded in privileged mode.
* If __retval_unpriv is present, the test run would be done for
program loaded in unprivileged mode.
* To mimic test_verifier behavior, the actual run is initiated in
privileged mode.
* The value returned by a test run is compared against retval
parameter.
The retval attribute takes one of the following parameters:
- a decimal number
- a hexadecimal number (must start from '0x')
- any of a three special literals (provided for compatibility with
test_verifier):
- INT_MIN
- POINTER_VALUE
- TEST_DATA_LEN
An example of the attribute usage:
SEC("socket")
__description("return 42")
__success __success_unpriv __retval(42)
__naked void the_42_test(void)
{
asm volatile (" \
r0 = 42; \
exit; \
" ::: __clobber_all);
}
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20230325025524.144043-5-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Extends test_loader.c:test_loader__run_subtests() by allowing to
execute tests in unprivileged mode, similar to test_verifier.c.
Adds the following new attributes controlling test_loader behavior:
__msg_unpriv
__success_unpriv
__failure_unpriv
* If any of these attributes is present the test would be loaded in
unprivileged mode.
* If only "privileged" attributes are present the test would be loaded
only in privileged mode.
* If both "privileged" and "unprivileged" attributes are present the
test would be loaded in both modes.
* If test has to be executed in both modes, __msg(text) is specified
and __msg_unpriv is not specified the behavior is the same as if
__msg_unpriv(text) is specified.
* For test filtering purposes the name of the program loaded in
unprivileged mode is derived from the usual program name by adding
`@unpriv' suffix.
Also adds attribute '__description'. This attribute specifies text to
be used instead of a program name for display and filtering purposes.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20230325025524.144043-4-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Change test_loader.c:run_subtest() behavior to show BPF program name
when test spec for that program can't be parsed.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20230325025524.144043-2-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
David Vernet says:
====================
When a map value is being freed, we loop over all of the fields of the
corresponding BPF object and issue the appropriate cleanup calls
corresponding to the field's type. If the field is a referenced kptr, we
atomically xchg the value out of the map, and invoke the kptr's
destructor on whatever was there before.
Currently, we always invoke the destructor (or bpf_obj_drop() for a
local kptr) on any kptr, including if no value was xchg'd out of the
map. This means that any function serving as the kptr's KF_RELEASE
destructor must always treat the argument as possibly NULL, and we
invoke unnecessary (and seemingly unsafe) cleanup logic for the local
kptr path as well.
This is an odd requirement -- KF_RELEASE kfuncs that are invoked by BPF
programs do not have this restriction, and the verifier will fail to
load the program if the register containing the to-be-released type has
any untrusted modifiers (e.g. PTR_UNTRUSTED or PTR_MAYBE_NULL). So as to
simplify the expectations required for a KF_RELEASE kfunc, this patch
set updates the KPTR_REF destructor logic to only be invoked when a
non-NULL value is xchg'd out of the map.
Additionally, the patch removes now-unnecessary KF_RELEASE calls from
several kfuncs, and finally, updates the verifier to have KF_RELEASE
automatically imply KF_TRUSTED_ARGS. This restriction was already
implicitly happening because of the aforementioned logic in the verifier
to reject any regs with untrusted modifiers, and to enforce that
KF_RELEASE args are passed with a 0 offset. This change just updates the
behavior to match that of other trusted args. This patch is left to the
end of the series in case it happens to be controversial, as it arguably
is slightly orthogonal to the purpose of the rest of the series.
====================
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
KF_RELEASE kfuncs are not currently treated as having KF_TRUSTED_ARGS,
even though they have a superset of the requirements of KF_TRUSTED_ARGS.
Like KF_TRUSTED_ARGS, KF_RELEASE kfuncs require a 0-offset argument, and
don't allow NULL-able arguments. Unlike KF_TRUSTED_ARGS which require
_either_ an argument with ref_obj_id > 0, _or_ (ref->type &
BPF_REG_TRUSTED_MODIFIERS) (and no unsafe modifiers allowed), KF_RELEASE
only allows for ref_obj_id > 0. Because KF_RELEASE today doesn't
automatically imply KF_TRUSTED_ARGS, some of these requirements are
enforced in different ways that can make the behavior of the verifier
feel unpredictable. For example, a KF_RELEASE kfunc with a NULL-able
argument will currently fail in the verifier with a message like, "arg#0
is ptr_or_null_ expected ptr_ or socket" rather than "Possibly NULL
pointer passed to trusted arg0". Our intention is the same, but the
semantics are different due to implemenetation details that kfunc authors
and BPF program writers should not need to care about.
Let's make the behavior of the verifier more consistent and intuitive by
having KF_RELEASE kfuncs imply the presence of KF_TRUSTED_ARGS. Our
eventual goal is to have all kfuncs assume KF_TRUSTED_ARGS by default
anyways, so this takes us a step in that direction.
Note that it does not make sense to assume KF_TRUSTED_ARGS for all
KF_ACQUIRE kfuncs. KF_ACQUIRE kfuncs can have looser semantics than
KF_RELEASE, with e.g. KF_RCU | KF_RET_NULL. We may want to have
KF_ACQUIRE imply KF_TRUSTED_ARGS _unless_ KF_RCU is specified, but that
can be left to another patch set, and there are no such subtleties to
address for KF_RELEASE.
Signed-off-by: David Vernet <void@manifault.com>
Link: https://lore.kernel.org/r/20230325213144.486885-4-void@manifault.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Now that we're not invoking kfunc destructors when the kptr in a map was
NULL, we no longer require NULL checks in many of our KF_RELEASE kfuncs.
This patch removes those NULL checks.
Signed-off-by: David Vernet <void@manifault.com>
Link: https://lore.kernel.org/r/20230325213144.486885-3-void@manifault.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
When a map value is being freed, we loop over all of the fields of the
corresponding BPF object and issue the appropriate cleanup calls
corresponding to the field's type. If the field is a referenced kptr, we
atomically xchg the value out of the map, and invoke the kptr's
destructor on whatever was there before (or bpf_obj_drop() it if it was
a local kptr).
Currently, we always invoke the destructor (either bpf_obj_drop() or the
kptr's registered destructor) on any KPTR_REF-type field in a map, even
if there wasn't a value in the map. This means that any function serving
as the kptr's KF_RELEASE destructor must always treat the argument as
possibly NULL, as the following can and regularly does happen:
void *xchgd_field;
/* No value was in the map, so xchgd_field is NULL */
xchgd_field = (void *)xchg(unsigned long *field_ptr, 0);
field->kptr.dtor(xchgd_field);
These are odd semantics to impose on KF_RELEASE kfuncs -- BPF programs
are prohibited by the verifier from passing NULL pointers to KF_RELEASE
kfuncs, so it doesn't make sense to require this of BPF programs, but
not the main kernel destructor path. It's also unnecessary to invoke any
cleanup logic for local kptrs. If there is no object there, there's
nothing to drop.
So as to allow KF_RELEASE kfuncs to fully assume that an argument is
non-NULL, this patch updates a KPTR_REF's destructor to only be invoked
when a non-NULL value is xchg'd out of the kptr map field.
Signed-off-by: David Vernet <void@manifault.com>
Link: https://lore.kernel.org/r/20230325213144.486885-2-void@manifault.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Now that everything that controls which LRU an obj lives in *except* the
backing pages is protected by the LRU lock, add a special path to unpin
in the job_run() path, where we are assured that we already have backing
pages and will not be racing against eviction (because the GEM object's
dma_resv contains the fence that will be signaled when the submit/job
completes).
Signed-off-by: Rob Clark <robdclark@chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/527845/
Link: https://lore.kernel.org/r/20230320144356.803762-10-robdclark@gmail.com
Since the LRU lock is already acquired when moving an obj between LRUs,
we can use it to protect pin_count and madv, without any significant
change in locking (ie. it just expands the scope of the lock by a hand-
ful of instructions). This prepares the way to decrement the pin_count
in the job_run() path without needing to hold the obj lock, to avoid a
potential deadlock (or rather stall) caused by the fence-signaling path
(job_run()) blocking on shrinker/reclaim. (Only a stall because the
wait for fence signaling wait_for_idle() is not infinite.)
Signed-off-by: Rob Clark <robdclark@chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/527843/
Link: https://lore.kernel.org/r/20230320144356.803762-9-robdclark@gmail.com
We need to use the inuse count to track that a BO is pinned until
we have the hw_fence. But we want to remove the obj lock from the
job_run() path as this could deadlock against reclaim/shrinker
(because it is blocking the hw_fence from eventually being signaled).
So split that tracking out into a per-vma lock with narrower scope.
Signed-off-by: Rob Clark <robdclark@chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/527839/
Link: https://lore.kernel.org/r/20230320144356.803762-5-robdclark@gmail.com
The flags are only accessed (1) when submit is constructed, before
enqueuing to gpu sched (ie. when still visible to only the task calling
the submit ioctl), (2) here, where we own a reference to the submit and
are serialized on the gpu sched thread, and (3) after the submit is
retired and last reference is dropped, which is serialized on the
submit's reference count. Hence locking is unneeded here.
Signed-off-by: Rob Clark <robdclark@chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/527830/
Link: https://lore.kernel.org/r/20230320144356.803762-3-robdclark@gmail.com
The drmm_encoder_alloc() function returns error pointers. It never
returns NULL. Fix the check accordingly.
Fixes: 7a1adbd239 ("drm: rcar-du: Use drmm_encoder_alloc() to manage encoder")
Signed-off-by: Dan Carpenter <error27@gmail.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
The DORCR register controls the routing of clocks and data between DU
channels within a group. For groups that contain a single channel,
there's no routing option to control, and some fields of the register
are then reserved. On Gen2 those reserved fields are documented as
required to be set to 0, while on Gen3 and newer the PG1T, DK1S and PG1D
reserved fields must be set to 1.
The DU driver initializes the DORCR register in rcar_du_group_setup(),
where it ignores the PG1T, DK1S and PG1D, and then configures those
fields to the correct value in rcar_du_group_set_routing(). This hasn't
been shown to cause any issue, but prevents certifying that the driver
complies with the documentation in safety-critical use cases.
As there is no reasonable change that the documentation will be updated
to clarify that those reserved fields can be written to 0 temporarily
before starting the hardware, make sure that the registers are always
set to valid values.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
The DORCR fields were documented in the R-Car H1 datasheet with 1-based
named, and then got renamed to 0-based in Gen2. The 0-based names are
used for Gen3 and Gen4, making H1 an outlier. Rename the field macros to
make them 0-based, in order to increase readability of the code when
comparing it with the documentation.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
When the input to a DU channel comes from a VSP, the DU doesn't perform
any blending operation. Select XRGB8888 instead of ARGB8888 to ensure
that the corresponding registers don't get written with invalid values.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
The ESCR and OTAR registers are not present in all DU channels on Gen3
SoCs. ESCR only exists in channels that can be routed to an LVDS or
DPAD, and OTAR in channels that can be routed to a DPAD. Skip writing
those registers for other channels. This replaces the DU gen check, as
Gen4 doesn't have LVDS or DPAD outputs.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
On R-Car D3 and E3, the LVDS encoder provides the dot (pixel) clock to
the DU, regardless of whether the LVDS output is used or not. When using
the DPAD (RGB) output, the DU driver thus enables and disables the LVDS
PLL manually, while when using the LVDS output, it lets the LVDS bridge
driver handle the PLL configuration internally as part of the atomic
enable and disable operations.
This causes an issue when using the LVDS output. As bridges are disabled
before CRTCs, the current implementation violates the enable/disable
sequences documented in the hardware datasheet, which requires the dot
clock to be enabled before the CRTC is started and disabled after it
gets stopped.
Fix the problem by enabling/disabling the LVDS PLL manually from the DU
regardless of which output is used, and skipping the PLL handling in the
LVDS bridge atomic enable and disable operations.
This is however not enough. Disabling the LVDS encoder while leaving the
PLL on still results in a vertical blanking wait timeout when disabling
the DU. Investigation showed that the culprit is the LVEN bit. For an
unclear reason, clearing the bit when disabling the LVDS encoder blocks
vertical blanking interrupts. We thus have to delay disabling the whole
LVDS encoder, not just disabling the PLL, until the DU is disabled.
We could split the LVDS disable sequence by clearing the LVRES bit in
the LVDS bridge atomic disable handler, and delaying the rest of the
operations, in order to disable the LVDS output at bridge atomic disable
time, before stopping the CRTC. This would make the code more complex,
without a clear benefit, so keep the implementation simple(r).
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
To prepare for a rework of the LVDS disable code, which will need to be
called from rcar_lvds_pclk_disable(), move the LVDS enable code,
currently stored in the __rcar_lvds_atomic_enable() function, to a
separate code section separate from bridge operations. It will be then
extended with the LVDS disable code.
As part of this rework the __rcar_lvds_atomic_enable() function is
renamed to rcar_lvds_enable() to more clearly indicate its purpose.
No functional change intended.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
When disabling the companion bridge in rcar_lvds_atomic_disable(),
there's no need to go through the bridge's operations to call
.atomic_disable(). Call rcar_lvds_atomic_disable() on the companion
directly.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
