Re: [PATCH v2 2/3] media: cedrus: Fix H264 default reference index count
On Thu 03 Oct 19, 23:19, Jernej Škrabec wrote: > Dne četrtek, 03. oktober 2019 ob 22:58:57 CEST je Paul Kocialkowski > napisal(a): > > Hi, > > > > On Thu 03 Oct 19, 22:44, Jernej Škrabec wrote: > > > Dne četrtek, 03. oktober 2019 ob 22:28:46 CEST je Paul Kocialkowski > > > > > > napisal(a): > > > > Hi, > > > > > > > > On Thu 03 Oct 19, 07:16, Jernej Škrabec wrote: > > > > > Dne četrtek, 03. oktober 2019 ob 00:06:50 CEST je Paul Kocialkowski > > > > > > > > > > napisal(a): > > > > > > Hi, > > > > > > > > > > > > On Wed 02 Oct 19, 21:35, Jernej Skrabec wrote: > > > > > > > Reference index count in VE_H264_PPS should come from PPS control. > > > > > > > However, this is not really important, because reference index > > > > > > > count > > > > > > > is > > > > > > > in our case always overridden by that from slice header. > > > > > > > > > > > > Thanks for the fixup! > > > > > > > > > > > > Our libva userspace and v4l2-request testing tool currently don't > > > > > > provide > > > > > > this, but I have a pending merge request adding it for the hantro so > > > > > > it's > > > > > > good to go. > > > > > > > > > > Actually, I think this is just cosmetic and it would work even if it > > > > > would > > > > > be always 0. We always override this number in SHS2 register with > > > > > VE_H264_SHS2_NUM_REF_IDX_ACTIVE_OVRD flag and recently there was a > > > > > patch > > > > > merged to clarify that value in slice parameters should be the one > > > > > that's > > > > > set on default value if override flag is not set in bitstream: > > > > > https://git.linuxtv.org/media_tree.git/commit/? > > > > > id=187ef7c5c78153acdce8c8714e5918b1018c710b > > > > > > > > > > Well, we could always compare default and value in slice parameters, > > > > > but I > > > > > really don't see the benefit of doing that extra work. > > > > > > > > Thanks for the detailed explanation! So I just realized that for HEVC, I > > > > didn't even include the default value in PPS and only went for the > > > > per-slice value. The HEVC hardware block apparently only needs the > > > > fields > > > > once at slice level, and by looking at the spec, only one of the two set > > > > of > > > > fields will be used. > > > > > > > > So perhaps we could do the same for H.264 and only have the set of > > > > fields > > > > once in the slice params, so that both codecs are consistent. Userspace > > > > can > > > > just check the flag to know whether it should put the PPS default or > > > > slice-specific value in the slice-specific control. > > > > > > > > What do you think? > > > > > > I think that there would be less confusion if only value in slice params > > > would exists. But since Philipp rather made clarification in > > > documentation, maybe he sees benefit having both values? > > > > Actually I just caught up with the discussion from thread: > > media: uapi: h264: Add num_ref_idx_active_override_flag > > > > which explains that we need to pass the default fields for hardware that > > parses the slice header itself and we need the non-default fields and flag > > for other cases. > > > > To cover the case of hardware that does slice header parsing, I guess it > > would also work to use the slice-specific values in place of the pps > > default values in the hardware register for that. But it feels quite > > confusing and a lot less straightforward than having all the fields and the > > override flag exposed. > > I wasn't aware of that patch and related discussion. Ok, so it make sense to > have both values. However, does it make sense to use default values in Cedrus? Well, since the hardware exposes fields for both and the flag for H264, let's do the straightforward thing and just pass the values through, even though we can easily predict which will get used in the end. For HEVC, we'll just have to check for the flag and put the right set of values in the slice-specific register. > > So I think I should fix HEVC support accordingly, just in case the same > > situation arises for HEVC. > > Seems reasonable. Does that mean there will be another revision of HEVC > patches? If so, I think slice_segment_addr should also be included in slice > params, so multi-slice frames can be supported at later time. I would be in favor of fixing this as a follow-up patch instead, so that we don't delay getting the series in. As you said, more work will be needed anyway for multi-slice support, so I don't see the point of holding the series for this particular improvment. Cheers, Paul > Best regards, > Jernej > > > > > Cheers, > > > > Paul > > > > > Best regards, > > > Jernej > > > > > > > Cheers, > > > > > > > > Paul > > > > > > > > > Best regards, > > > > > Jernej > > > > > > > > > > > Acked-by: Paul Kocialkowski > > > > > > > > > > > > Cheers, > > > > > > > > > > > > Paul > > > > > > > > > > > > > Signed-off-by: Jernej Skrabec > > > > > > > --- > > > > > > > > > > > > > > drivers/staging/media/sunxi/cedrus/cedrus_h264.c |
Re: [PATCH v2 2/3] media: cedrus: Fix H264 default reference index count
Dne četrtek, 03. oktober 2019 ob 22:58:57 CEST je Paul Kocialkowski napisal(a): > Hi, > > On Thu 03 Oct 19, 22:44, Jernej Škrabec wrote: > > Dne četrtek, 03. oktober 2019 ob 22:28:46 CEST je Paul Kocialkowski > > > > napisal(a): > > > Hi, > > > > > > On Thu 03 Oct 19, 07:16, Jernej Škrabec wrote: > > > > Dne četrtek, 03. oktober 2019 ob 00:06:50 CEST je Paul Kocialkowski > > > > > > > > napisal(a): > > > > > Hi, > > > > > > > > > > On Wed 02 Oct 19, 21:35, Jernej Skrabec wrote: > > > > > > Reference index count in VE_H264_PPS should come from PPS control. > > > > > > However, this is not really important, because reference index > > > > > > count > > > > > > is > > > > > > in our case always overridden by that from slice header. > > > > > > > > > > Thanks for the fixup! > > > > > > > > > > Our libva userspace and v4l2-request testing tool currently don't > > > > > provide > > > > > this, but I have a pending merge request adding it for the hantro so > > > > > it's > > > > > good to go. > > > > > > > > Actually, I think this is just cosmetic and it would work even if it > > > > would > > > > be always 0. We always override this number in SHS2 register with > > > > VE_H264_SHS2_NUM_REF_IDX_ACTIVE_OVRD flag and recently there was a > > > > patch > > > > merged to clarify that value in slice parameters should be the one > > > > that's > > > > set on default value if override flag is not set in bitstream: > > > > https://git.linuxtv.org/media_tree.git/commit/? > > > > id=187ef7c5c78153acdce8c8714e5918b1018c710b > > > > > > > > Well, we could always compare default and value in slice parameters, > > > > but I > > > > really don't see the benefit of doing that extra work. > > > > > > Thanks for the detailed explanation! So I just realized that for HEVC, I > > > didn't even include the default value in PPS and only went for the > > > per-slice value. The HEVC hardware block apparently only needs the > > > fields > > > once at slice level, and by looking at the spec, only one of the two set > > > of > > > fields will be used. > > > > > > So perhaps we could do the same for H.264 and only have the set of > > > fields > > > once in the slice params, so that both codecs are consistent. Userspace > > > can > > > just check the flag to know whether it should put the PPS default or > > > slice-specific value in the slice-specific control. > > > > > > What do you think? > > > > I think that there would be less confusion if only value in slice params > > would exists. But since Philipp rather made clarification in > > documentation, maybe he sees benefit having both values? > > Actually I just caught up with the discussion from thread: > media: uapi: h264: Add num_ref_idx_active_override_flag > > which explains that we need to pass the default fields for hardware that > parses the slice header itself and we need the non-default fields and flag > for other cases. > > To cover the case of hardware that does slice header parsing, I guess it > would also work to use the slice-specific values in place of the pps > default values in the hardware register for that. But it feels quite > confusing and a lot less straightforward than having all the fields and the > override flag exposed. I wasn't aware of that patch and related discussion. Ok, so it make sense to have both values. However, does it make sense to use default values in Cedrus? > > So I think I should fix HEVC support accordingly, just in case the same > situation arises for HEVC. Seems reasonable. Does that mean there will be another revision of HEVC patches? If so, I think slice_segment_addr should also be included in slice params, so multi-slice frames can be supported at later time. Best regards, Jernej > > Cheers, > > Paul > > > Best regards, > > Jernej > > > > > Cheers, > > > > > > Paul > > > > > > > Best regards, > > > > Jernej > > > > > > > > > Acked-by: Paul Kocialkowski > > > > > > > > > > Cheers, > > > > > > > > > > Paul > > > > > > > > > > > Signed-off-by: Jernej Skrabec > > > > > > --- > > > > > > > > > > > > drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 8 ++-- > > > > > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > > > > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > > > > b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index > > > > > > bd848146eada..4a0e69855c7f 100644 > > > > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > > > > @@ -364,12 +364,8 @@ static void cedrus_set_params(struct > > > > > > cedrus_ctx > > > > > > *ctx, > > > > > > > > > > > > // picture parameters > > > > > > reg = 0; > > > > > > > > > > > > - /* > > > > > > -* FIXME: the kernel headers are allowing the default value to > > > > > > -* be passed, but the libva doesn't give us that. > > > > > > -*/ > > > > > > - reg |= (slice->num_ref_idx_l0_active_minus1 & 0x1f) << 10; > > >
Re: [PATCH] staging: rtl8188eu: fix null dereference when kzalloc fails
On 10/1/19 6:11 AM, Dan Carpenter wrote:
There is another one earlier in the function as well.
drivers/staging/rtl8188eu/os_dep/usb_intf.c
336
337 pnetdev = rtw_init_netdev(padapter);
338 if (!pnetdev)
339 goto free_adapter;
340 SET_NETDEV_DEV(pnetdev, dvobj_to_dev(dvobj));
341 padapter = rtw_netdev_priv(pnetdev);
342
343 if (padapter->registrypriv.monitor_enable) {
344 pmondev = rtl88eu_mon_init();
345 if (!pmondev)
346 netdev_warn(pnetdev, "Failed to initialize monitor
interface");
goto free_adapter.
347 padapter->pmondev = pmondev;
348 }
349
350 padapter->HalData = kzalloc(sizeof(struct hal_data_8188e),
GFP_KERNEL);
351 if (!padapter->HalData)
352 DBG_88E("cant not alloc memory for HAL DATA\n");
353
padapter->HalData = kzalloc(sizeof(struct hal_data_8188e), GFP_KERNEL);
- if (!padapter->HalData)
- DBG_88E("cant not alloc memory for HAL DATA\n");
+ if (!padapter->HalData) {
+ DBG_88E("Failed to allocate memory for HAL data\n");
Remove this debug printk.
+ goto free_adapter;
+ }
Hi Dan,
Sorry for such a late response! By the time I saw the e-mail with your
feedback I also saw another e-mail saying this patch was accepted into a
staging-linus tree. I'll address your comments in a separate patch.
Thank you,
Connor
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [PATCH] staging: vc04_services: Avoid typedef
On Thu, 3 Oct 2019, Nachammai Karuppiah wrote:
> Avoid typedefs to maintain kernel coding style. Issue found by
> checkpatch.pl
>
> Replace the enum typedef VCHIQ_REASON_T with vchiq_reason.
Would it be possible to get rid of them all? They seem to all go
together, since they start with the same prefix, and they all don't look
nice at all.
The changes so far seem to be going in the right direction.
julia
>
> Signed-off-by: Nachammai Karuppiah
> ---
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 8
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++--
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h| 6 +++---
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h | 2 +-
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c | 2 +-
> 5 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index b1595b1..280e237 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -516,7 +516,7 @@ vchiq_blocking_bulk_transfer(VCHIQ_SERVICE_HANDLE_T
> handle, void *data,
> ***/
>
> static VCHIQ_STATUS_T
> -add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T reason,
> +add_completion(VCHIQ_INSTANCE_T instance, enum vchiq_reason reason,
> struct vchiq_header *header, struct user_service *user_service,
> void *bulk_userdata)
> {
> @@ -583,7 +583,7 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T
> reason,
> ***/
>
> static VCHIQ_STATUS_T
> -service_callback(VCHIQ_REASON_T reason, struct vchiq_header *header,
> +service_callback(enum vchiq_reason reason, struct vchiq_header *header,
>VCHIQ_SERVICE_HANDLE_T handle, void *bulk_userdata)
> {
> /* How do we ensure the callback goes to the right client?
> @@ -1666,7 +1666,7 @@ vchiq_compat_ioctl_queue_bulk(struct file *file,
> }
>
> struct vchiq_completion_data32 {
> - VCHIQ_REASON_T reason;
> + enum vchiq_reason reason;
> compat_uptr_t header;
> compat_uptr_t service_userdata;
> compat_uptr_t bulk_userdata;
> @@ -2271,7 +2271,7 @@ vchiq_videocore_wanted(struct vchiq_state *state)
> }
>
> static VCHIQ_STATUS_T
> -vchiq_keepalive_vchiq_callback(VCHIQ_REASON_T reason,
> +vchiq_keepalive_vchiq_callback(enum vchiq_reason reason,
> struct vchiq_header *header,
> VCHIQ_SERVICE_HANDLE_T service_user,
> void *bulk_user)
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index 56a23a2..b0e0653 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -355,7 +355,7 @@ mark_service_closing(struct vchiq_service *service)
> }
>
> static inline VCHIQ_STATUS_T
> -make_service_callback(struct vchiq_service *service, VCHIQ_REASON_T reason,
> +make_service_callback(struct vchiq_service *service, enum vchiq_reason
> reason,
> struct vchiq_header *header, void *bulk_userdata)
> {
> VCHIQ_STATUS_T status;
> @@ -1230,7 +1230,7 @@ notify_bulks(struct vchiq_service *service, struct
> vchiq_bulk_queue *queue,
> spin_unlock(_waiter_spinlock);
> } else if (bulk->mode ==
> VCHIQ_BULK_MODE_CALLBACK) {
> - VCHIQ_REASON_T reason = (bulk->dir ==
> + enum vchiq_reason reason = (bulk->dir ==
> VCHIQ_BULK_TRANSMIT) ?
> ((bulk->actual ==
> VCHIQ_BULK_ACTUAL_ABORTED) ?
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
> index c23bd10..f911612 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
> @@ -15,7 +15,7 @@
> #define VCHIQ_GET_SERVICE_USERDATA(service)
> vchiq_get_service_userdata(service)
> #define VCHIQ_GET_SERVICE_FOURCC(service) vchiq_get_service_fourcc(service)
>
> -typedef enum {
> +enum vchiq_reason {
> VCHIQ_SERVICE_OPENED, /* service, -, - */
> VCHIQ_SERVICE_CLOSED, /* service, -, - */
> VCHIQ_MESSAGE_AVAILABLE, /* service, header, -*/
> @@ -23,7 +23,7 @@ typedef enum {
> VCHIQ_BULK_RECEIVE_DONE, /* service,
Re: [PATCH v2 2/3] media: cedrus: Fix H264 default reference index count
Hi, On Thu 03 Oct 19, 22:44, Jernej Škrabec wrote: > Dne četrtek, 03. oktober 2019 ob 22:28:46 CEST je Paul Kocialkowski > napisal(a): > > Hi, > > > > On Thu 03 Oct 19, 07:16, Jernej Škrabec wrote: > > > Dne četrtek, 03. oktober 2019 ob 00:06:50 CEST je Paul Kocialkowski > > > > > > napisal(a): > > > > Hi, > > > > > > > > On Wed 02 Oct 19, 21:35, Jernej Skrabec wrote: > > > > > Reference index count in VE_H264_PPS should come from PPS control. > > > > > However, this is not really important, because reference index count > > > > > is > > > > > in our case always overridden by that from slice header. > > > > > > > > Thanks for the fixup! > > > > > > > > Our libva userspace and v4l2-request testing tool currently don't > > > > provide > > > > this, but I have a pending merge request adding it for the hantro so > > > > it's > > > > good to go. > > > > > > Actually, I think this is just cosmetic and it would work even if it would > > > be always 0. We always override this number in SHS2 register with > > > VE_H264_SHS2_NUM_REF_IDX_ACTIVE_OVRD flag and recently there was a patch > > > merged to clarify that value in slice parameters should be the one that's > > > set on default value if override flag is not set in bitstream: > > > https://git.linuxtv.org/media_tree.git/commit/? > > > id=187ef7c5c78153acdce8c8714e5918b1018c710b > > > > > > Well, we could always compare default and value in slice parameters, but I > > > really don't see the benefit of doing that extra work. > > > > Thanks for the detailed explanation! So I just realized that for HEVC, I > > didn't even include the default value in PPS and only went for the > > per-slice value. The HEVC hardware block apparently only needs the fields > > once at slice level, and by looking at the spec, only one of the two set of > > fields will be used. > > > > So perhaps we could do the same for H.264 and only have the set of fields > > once in the slice params, so that both codecs are consistent. Userspace can > > just check the flag to know whether it should put the PPS default or > > slice-specific value in the slice-specific control. > > > > What do you think? > > I think that there would be less confusion if only value in slice params > would > exists. But since Philipp rather made clarification in documentation, maybe > he > sees benefit having both values? Actually I just caught up with the discussion from thread: media: uapi: h264: Add num_ref_idx_active_override_flag which explains that we need to pass the default fields for hardware that parses the slice header itself and we need the non-default fields and flag for other cases. To cover the case of hardware that does slice header parsing, I guess it would also work to use the slice-specific values in place of the pps default values in the hardware register for that. But it feels quite confusing and a lot less straightforward than having all the fields and the override flag exposed. So I think I should fix HEVC support accordingly, just in case the same situation arises for HEVC. Cheers, Paul > Best regards, > Jernej > > > > > Cheers, > > > > Paul > > > > > Best regards, > > > Jernej > > > > > > > Acked-by: Paul Kocialkowski > > > > > > > > Cheers, > > > > > > > > Paul > > > > > > > > > Signed-off-by: Jernej Skrabec > > > > > --- > > > > > > > > > > drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 8 ++-- > > > > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > > > b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index > > > > > bd848146eada..4a0e69855c7f 100644 > > > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > > > @@ -364,12 +364,8 @@ static void cedrus_set_params(struct cedrus_ctx > > > > > *ctx, > > > > > > > > > > // picture parameters > > > > > reg = 0; > > > > > > > > > > - /* > > > > > - * FIXME: the kernel headers are allowing the default value to > > > > > - * be passed, but the libva doesn't give us that. > > > > > - */ > > > > > - reg |= (slice->num_ref_idx_l0_active_minus1 & 0x1f) << 10; > > > > > - reg |= (slice->num_ref_idx_l1_active_minus1 & 0x1f) << 5; > > > > > + reg |= (pps->num_ref_idx_l0_default_active_minus1 & 0x1f) << 10; > > > > > + reg |= (pps->num_ref_idx_l1_default_active_minus1 & 0x1f) << 5; > > > > > > > > > > reg |= (pps->weighted_bipred_idc & 0x3) << 2; > > > > > if (pps->flags & V4L2_H264_PPS_FLAG_ENTROPY_CODING_MODE) > > > > > > > > > > reg |= VE_H264_PPS_ENTROPY_CODING_MODE; > > > > -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org
Re: [PATCH v2 2/3] media: cedrus: Fix H264 default reference index count
Dne četrtek, 03. oktober 2019 ob 22:28:46 CEST je Paul Kocialkowski napisal(a): > Hi, > > On Thu 03 Oct 19, 07:16, Jernej Škrabec wrote: > > Dne četrtek, 03. oktober 2019 ob 00:06:50 CEST je Paul Kocialkowski > > > > napisal(a): > > > Hi, > > > > > > On Wed 02 Oct 19, 21:35, Jernej Skrabec wrote: > > > > Reference index count in VE_H264_PPS should come from PPS control. > > > > However, this is not really important, because reference index count > > > > is > > > > in our case always overridden by that from slice header. > > > > > > Thanks for the fixup! > > > > > > Our libva userspace and v4l2-request testing tool currently don't > > > provide > > > this, but I have a pending merge request adding it for the hantro so > > > it's > > > good to go. > > > > Actually, I think this is just cosmetic and it would work even if it would > > be always 0. We always override this number in SHS2 register with > > VE_H264_SHS2_NUM_REF_IDX_ACTIVE_OVRD flag and recently there was a patch > > merged to clarify that value in slice parameters should be the one that's > > set on default value if override flag is not set in bitstream: > > https://git.linuxtv.org/media_tree.git/commit/? > > id=187ef7c5c78153acdce8c8714e5918b1018c710b > > > > Well, we could always compare default and value in slice parameters, but I > > really don't see the benefit of doing that extra work. > > Thanks for the detailed explanation! So I just realized that for HEVC, I > didn't even include the default value in PPS and only went for the > per-slice value. The HEVC hardware block apparently only needs the fields > once at slice level, and by looking at the spec, only one of the two set of > fields will be used. > > So perhaps we could do the same for H.264 and only have the set of fields > once in the slice params, so that both codecs are consistent. Userspace can > just check the flag to know whether it should put the PPS default or > slice-specific value in the slice-specific control. > > What do you think? I think that there would be less confusion if only value in slice params would exists. But since Philipp rather made clarification in documentation, maybe he sees benefit having both values? Best regards, Jernej > > Cheers, > > Paul > > > Best regards, > > Jernej > > > > > Acked-by: Paul Kocialkowski > > > > > > Cheers, > > > > > > Paul > > > > > > > Signed-off-by: Jernej Skrabec > > > > --- > > > > > > > > drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 8 ++-- > > > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > > b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index > > > > bd848146eada..4a0e69855c7f 100644 > > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > > @@ -364,12 +364,8 @@ static void cedrus_set_params(struct cedrus_ctx > > > > *ctx, > > > > > > > > // picture parameters > > > > reg = 0; > > > > > > > > - /* > > > > -* FIXME: the kernel headers are allowing the default value to > > > > -* be passed, but the libva doesn't give us that. > > > > -*/ > > > > - reg |= (slice->num_ref_idx_l0_active_minus1 & 0x1f) << 10; > > > > - reg |= (slice->num_ref_idx_l1_active_minus1 & 0x1f) << 5; > > > > + reg |= (pps->num_ref_idx_l0_default_active_minus1 & 0x1f) << 10; > > > > + reg |= (pps->num_ref_idx_l1_default_active_minus1 & 0x1f) << 5; > > > > > > > > reg |= (pps->weighted_bipred_idc & 0x3) << 2; > > > > if (pps->flags & V4L2_H264_PPS_FLAG_ENTROPY_CODING_MODE) > > > > > > > > reg |= VE_H264_PPS_ENTROPY_CODING_MODE; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/3] media: cedrus: Fix H264 default reference index count
Hi, On Thu 03 Oct 19, 07:16, Jernej Škrabec wrote: > Dne četrtek, 03. oktober 2019 ob 00:06:50 CEST je Paul Kocialkowski > napisal(a): > > Hi, > > > > On Wed 02 Oct 19, 21:35, Jernej Skrabec wrote: > > > Reference index count in VE_H264_PPS should come from PPS control. > > > However, this is not really important, because reference index count is > > > in our case always overridden by that from slice header. > > > > Thanks for the fixup! > > > > Our libva userspace and v4l2-request testing tool currently don't provide > > this, but I have a pending merge request adding it for the hantro so it's > > good to go. > > Actually, I think this is just cosmetic and it would work even if it would be > always 0. We always override this number in SHS2 register with > VE_H264_SHS2_NUM_REF_IDX_ACTIVE_OVRD flag and recently there was a patch > merged > to clarify that value in slice parameters should be the one that's set on > default value if override flag is not set in bitstream: > https://git.linuxtv.org/media_tree.git/commit/? > id=187ef7c5c78153acdce8c8714e5918b1018c710b > > Well, we could always compare default and value in slice parameters, but I > really don't see the benefit of doing that extra work. Thanks for the detailed explanation! So I just realized that for HEVC, I didn't even include the default value in PPS and only went for the per-slice value. The HEVC hardware block apparently only needs the fields once at slice level, and by looking at the spec, only one of the two set of fields will be used. So perhaps we could do the same for H.264 and only have the set of fields once in the slice params, so that both codecs are consistent. Userspace can just check the flag to know whether it should put the PPS default or slice-specific value in the slice-specific control. What do you think? Cheers, Paul > Best regards, > Jernej > > > > > Acked-by: Paul Kocialkowski > > > > Cheers, > > > > Paul > > > > > Signed-off-by: Jernej Skrabec > > > --- > > > > > > drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 8 ++-- > > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index > > > bd848146eada..4a0e69855c7f 100644 > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c > > > @@ -364,12 +364,8 @@ static void cedrus_set_params(struct cedrus_ctx *ctx, > > > > > > // picture parameters > > > reg = 0; > > > > > > - /* > > > - * FIXME: the kernel headers are allowing the default value to > > > - * be passed, but the libva doesn't give us that. > > > - */ > > > - reg |= (slice->num_ref_idx_l0_active_minus1 & 0x1f) << 10; > > > - reg |= (slice->num_ref_idx_l1_active_minus1 & 0x1f) << 5; > > > + reg |= (pps->num_ref_idx_l0_default_active_minus1 & 0x1f) << 10; > > > + reg |= (pps->num_ref_idx_l1_default_active_minus1 & 0x1f) << 5; > > > > > > reg |= (pps->weighted_bipred_idc & 0x3) << 2; > > > if (pps->flags & V4L2_H264_PPS_FLAG_ENTROPY_CODING_MODE) > > > > > > reg |= VE_H264_PPS_ENTROPY_CODING_MODE; > > > > -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 6/8] arm64: hyperv: Initialize hypervisor on boot
Add ARM64-specific code to initialize the Hyper-V
hypervisor when booting as a guest VM. Provide functions
and data structures indicating hypervisor status that
are needed by VMbus driver.
This code is built only when CONFIG_HYPERV is enabled.
Signed-off-by: Michael Kelley
---
arch/arm64/hyperv/hv_init.c | 153
1 file changed, 153 insertions(+)
diff --git a/arch/arm64/hyperv/hv_init.c b/arch/arm64/hyperv/hv_init.c
index 67350ec..86e4621 100644
--- a/arch/arm64/hyperv/hv_init.c
+++ b/arch/arm64/hyperv/hv_init.c
@@ -13,15 +13,48 @@
#include
#include
#include
+#include
#include
+#include
+#include
#include
#include
#include
#include
+#include
+#include
+#include
+#include
#include
#include
#include
+#include
+#include
+static boolhyperv_initialized;
+
+struct ms_hyperv_info ms_hyperv __ro_after_init;
+EXPORT_SYMBOL_GPL(ms_hyperv);
+
+u32*hv_vp_index;
+EXPORT_SYMBOL_GPL(hv_vp_index);
+
+u32hv_max_vp_index;
+EXPORT_SYMBOL_GPL(hv_max_vp_index);
+
+static int hv_cpu_init(unsigned int cpu)
+{
+ u64 msr_vp_index;
+
+ hv_get_vp_index(msr_vp_index);
+
+ hv_vp_index[smp_processor_id()] = msr_vp_index;
+
+ if (msr_vp_index > hv_max_vp_index)
+ hv_max_vp_index = msr_vp_index;
+
+ return 0;
+}
/*
* Functions for allocating and freeing memory with size and
@@ -88,6 +121,120 @@ void hv_free_hyperv_page(unsigned long addr)
/*
+ * This function is invoked via the ACPI clocksource probe mechanism. We
+ * don't actually use any values from the ACPI GTDT table, but we set up
+ * the Hyper-V synthetic clocksource and do other initialization for
+ * interacting with Hyper-V the first time. Using early_initcall to invoke
+ * this function is too late because interrupts are already enabled at that
+ * point, and hv_init_clocksource() must run before interrupts are enabled.
+ *
+ * 1. Setup the guest ID.
+ * 2. Get features and hints info from Hyper-V
+ * 3. Setup per-cpu VP indices.
+ * 4. Initialize the Hyper-V clocksource.
+ */
+
+static int __init hyperv_init(struct acpi_table_header *table)
+{
+ struct hv_get_vp_register_output result;
+ u32 a, b, c, d;
+ u64 guest_id;
+ int i;
+
+ /*
+* If we're in a VM on Hyper-V, the ACPI hypervisor_id field will
+* have the string "MsHyperV".
+*/
+ if (strncmp((char *)_gbl_FADT.hypervisor_id, "MsHyperV", 8))
+ return -EINVAL;
+
+ /* Setup the guest ID */
+ guest_id = generate_guest_id(0, LINUX_VERSION_CODE, 0);
+ hv_set_vpreg(HV_REGISTER_GUEST_OSID, guest_id);
+
+ /* Get the features and hints from Hyper-V */
+ hv_get_vpreg_128(HV_REGISTER_PRIVILEGES_AND_FEATURES, );
+ ms_hyperv.features = lower_32_bits(result.registervaluelow);
+ ms_hyperv.misc_features = upper_32_bits(result.registervaluehigh);
+
+ hv_get_vpreg_128(HV_REGISTER_FEATURES, );
+ ms_hyperv.hints = lower_32_bits(result.registervaluelow);
+
+ pr_info("Hyper-V: Features 0x%x, hints 0x%x\n",
+ ms_hyperv.features, ms_hyperv.hints);
+
+ /*
+* Direct mode is the only option for STIMERs provided Hyper-V
+* on ARM64, so Hyper-V doesn't actually set the flag. But add
+* the flag so the architecture independent code in
+* drivers/clocksource/hyperv_timer.c will correctly use that mode.
+*/
+ ms_hyperv.misc_features |= HV_STIMER_DIRECT_MODE_AVAILABLE;
+
+ /*
+* Hyper-V on ARM64 doesn't support AutoEOI. Add the hint
+* that tells architecture independent code not to use this
+* feature.
+*/
+ ms_hyperv.hints |= HV_DEPRECATING_AEOI_RECOMMENDED;
+
+ /* Get information about the Hyper-V host version */
+ hv_get_vpreg_128(HV_REGISTER_HYPERVISOR_VERSION, );
+ a = lower_32_bits(result.registervaluelow);
+ b = upper_32_bits(result.registervaluelow);
+ c = lower_32_bits(result.registervaluehigh);
+ d = upper_32_bits(result.registervaluehigh);
+ pr_info("Hyper-V: Host Build %d.%d.%d.%d-%d-%d\n",
+ b >> 16, b & 0x, a, d & 0xFF, c, d >> 24);
+
+ /* Allocate and initialize percpu VP index array */
+ hv_vp_index = kmalloc_array(num_possible_cpus(), sizeof(*hv_vp_index),
+ GFP_KERNEL);
+ if (!hv_vp_index)
+ return -ENOMEM;
+
+ for (i = 0; i < num_possible_cpus(); i++)
+ hv_vp_index[i] = VP_INVAL;
+
+ if (cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm64/hyperv_init:online",
+ hv_cpu_init, NULL) < 0)
+ goto free_vp_index;
+
+ hv_init_clocksource();
+
+ hyperv_initialized = true;
+ return 0;
+
+free_vp_index:
+ kfree(hv_vp_index);
+ hv_vp_index = NULL;
+ return -EINVAL;
+}
+TIMER_ACPI_DECLARE(hyperv,
[PATCH v5 8/8] Drivers: hv: Enable Hyper-V code to be built on ARM64
Update drivers/hv/Kconfig so CONFIG_HYPERV can be selected on ARM64, causing the Hyper-V specific code to be built. Signed-off-by: Michael Kelley --- drivers/hv/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig index 79e5356..1113e49 100644 --- a/drivers/hv/Kconfig +++ b/drivers/hv/Kconfig @@ -4,7 +4,8 @@ menu "Microsoft Hyper-V guest support" config HYPERV tristate "Microsoft Hyper-V client drivers" - depends on X86 && ACPI && X86_LOCAL_APIC && HYPERVISOR_GUEST + depends on ACPI && \ + ((X86 && X86_LOCAL_APIC && HYPERVISOR_GUEST) || ARM64) select PARAVIRT select X86_HV_CALLBACK_VECTOR help -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 7/8] Drivers: hv: vmbus: Add hooks for per-CPU IRQ
Add hooks to enable/disable a per-CPU IRQ for VMbus. These hooks
are in the architecture independent setup and shutdown paths for
Hyper-V, and are needed by Linux guests on Hyper-V on ARM64. The
x86/x64 implementation is null because VMbus interrupts on x86/x64
don't use an IRQ.
Signed-off-by: Michael Kelley
---
arch/x86/include/asm/mshyperv.h | 4
drivers/hv/hv.c | 3 +++
2 files changed, 7 insertions(+)
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index f4138ae..583e1ce 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -56,6 +56,10 @@ typedef int (*hyperv_fill_flush_list_func)(
#endif
void hyperv_vector_handler(struct pt_regs *regs);
+/* On x86/x64, there isn't a real IRQ to be enabled/disable */
+static inline void hv_enable_vmbus_irq(void) {}
+static inline void hv_disable_vmbus_irq(void) {}
+
/*
* Routines for stimer0 Direct Mode handling.
* On x86/x64, there are no percpu actions to take.
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index fcc5279..51d8f8a 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -180,6 +180,7 @@ void hv_synic_enable_regs(unsigned int cpu)
hv_set_siefp(siefp.as_uint64);
/* Setup the shared SINT. */
+ hv_enable_vmbus_irq();
hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
shared_sint.vector = HYPERVISOR_CALLBACK_VECTOR;
@@ -241,6 +242,8 @@ void hv_synic_disable_regs(unsigned int cpu)
hv_get_synic_state(sctrl.as_uint64);
sctrl.enable = 0;
hv_set_synic_state(sctrl.as_uint64);
+
+ hv_disable_vmbus_irq();
}
int hv_synic_cleanup(unsigned int cpu)
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 1/8] arm64: hyperv: Add core Hyper-V include files
hyperv-tlfs.h defines Hyper-V interfaces from the Hyper-V Top Level Functional Spec (TLFS). The TLFS is distinctly oriented to x86/x64, and Hyper-V has not separated out the architecture-dependent parts into x86/x64 vs. ARM64. So hyperv-tlfs.h includes information for ARM64 that is not yet formally published. The TLFS is available here: docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/tlfs mshyperv.h defines Linux-specific structures and routines for interacting with Hyper-V on ARM64, and #includes the architecture- independent part of mshyperv.h in include/asm-generic. Signed-off-by: Michael Kelley --- MAINTAINERS | 2 + arch/arm64/include/asm/hyperv-tlfs.h | 408 +++ arch/arm64/include/asm/mshyperv.h| 105 + 3 files changed, 515 insertions(+) create mode 100644 arch/arm64/include/asm/hyperv-tlfs.h create mode 100644 arch/arm64/include/asm/mshyperv.h diff --git a/MAINTAINERS b/MAINTAINERS index f04f081..d464067 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7564,6 +7564,8 @@ F:arch/x86/include/asm/trace/hyperv.h F: arch/x86/include/asm/hyperv-tlfs.h F: arch/x86/kernel/cpu/mshyperv.c F: arch/x86/hyperv +F: arch/arm64/include/asm/hyperv-tlfs.h +F: arch/arm64/include/asm/mshyperv.h F: drivers/clocksource/hyperv_timer.c F: drivers/hid/hid-hyperv.c F: drivers/hv/ diff --git a/arch/arm64/include/asm/hyperv-tlfs.h b/arch/arm64/include/asm/hyperv-tlfs.h new file mode 100644 index 000..fe167c4 --- /dev/null +++ b/arch/arm64/include/asm/hyperv-tlfs.h @@ -0,0 +1,408 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* + * This file contains definitions from the Hyper-V Hypervisor Top-Level + * Functional Specification (TLFS): + * https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/tlfs + * + * Copyright (C) 2019, Microsoft, Inc. + * + * Author : Michael Kelley + */ + +#ifndef _ASM_HYPERV_TLFS_H +#define _ASM_HYPERV_TLFS_H + +#include + +/* + * All data structures defined in the TLFS that are shared between Hyper-V + * and a guest VM use Little Endian byte ordering. This matches the default + * byte ordering of Linux running on ARM64, so no special handling is required. + */ + + +/* + * While not explicitly listed in the TLFS, Hyper-V always runs with a page + * size of 4096. These definitions are used when communicating with Hyper-V + * using guest physical pages and guest physical page addresses, since the + * guest page size may not be 4096 on ARM64. + */ +#define HV_HYP_PAGE_SHIFT 12 +#define HV_HYP_PAGE_SIZE (1 << HV_HYP_PAGE_SHIFT) +#define HV_HYP_PAGE_MASK (~(HV_HYP_PAGE_SIZE - 1)) + +/* + * These Hyper-V registers provide information equivalent to the CPUID + * instruction on x86/x64. + */ +#define HV_REGISTER_HYPERVISOR_VERSION 0x0100 /*CPUID 0x4002 */ +#defineHV_REGISTER_PRIVILEGES_AND_FEATURES 0x0200 /*CPUID 0x4003 */ +#defineHV_REGISTER_FEATURES0x0201 /*CPUID 0x4004 */ +#defineHV_REGISTER_IMPLEMENTATION_LIMITS 0x0202 /*CPUID 0x4005 */ +#define HV_ARM64_REGISTER_INTERFACE_VERSION0x00090006 /*CPUID 0x4001 */ + +/* + * Feature identification. HvRegisterPrivilegesAndFeaturesInfo returns a + * 128-bit value with flags indicating which features are available to the + * partition based upon the current partition privileges. The 128-bit + * value is broken up with different portions stored in different 32-bit + * fields in the ms_hyperv structure. + */ + +/* Partition Reference Counter available*/ +#define HV_MSR_TIME_REF_COUNT_AVAILABLEBIT(1) + +/* + * Synthetic Timers available + */ +#define HV_MSR_SYNTIMER_AVAILABLE BIT(3) + +/* Frequency MSRs available */ +#define HV_FEATURE_FREQUENCY_MSRS_AVAILABLEBIT(8) + +/* Reference TSC available */ +#define HV_MSR_REFERENCE_TSC_AVAILABLE BIT(9) + +/* Crash MSR available */ +#define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE BIT(10) + + +/* + * This group of flags is in the high order 64-bits of the returned + * 128-bit value. + */ + +/* STIMER direct mode is available */ +#define HV_STIMER_DIRECT_MODE_AVAILABLEBIT(19) + +/* + * Implementation recommendations in register + * HvRegisterFeaturesInfo. Indicates which behaviors the hypervisor + * recommends the OS implement for optimal performance. + */ + +/* + * Recommend not using Auto EOI + */ +#define HV_DEPRECATING_AEOI_RECOMMENDEDBIT(9) + +/* + * Synthetic register definitions equivalent to MSRs on x86/x64 + */ +#define HV_REGISTER_CRASH_P0 0x0210 +#define HV_REGISTER_CRASH_P1 0x0211 +#define HV_REGISTER_CRASH_P2 0x0212 +#define HV_REGISTER_CRASH_P3 0x0213 +#define HV_REGISTER_CRASH_P4 0x0214 +#define HV_REGISTER_CRASH_CTL 0x0215 + +#define HV_REGISTER_GUEST_OSID
[PATCH v5 3/8] arm64: hyperv: Add memory alloc/free functions for Hyper-V size pages
Add ARM64-specific code to allocate memory with HV_HYP_PAGE_SIZE
size and alignment. These are for use when pages need to be shared
with Hyper-V. Separate functions are needed as the page size used
by Hyper-V may not be the same as the guest page size. Free
operations are rarely done, so no attempt is made to combine
freed pages into larger chunks.
This code is built only when CONFIG_HYPERV is enabled.
Signed-off-by: Michael Kelley
---
arch/arm64/hyperv/hv_init.c| 68 ++
include/asm-generic/mshyperv.h | 5
2 files changed, 73 insertions(+)
diff --git a/arch/arm64/hyperv/hv_init.c b/arch/arm64/hyperv/hv_init.c
index 6808bc8..9c294f6 100644
--- a/arch/arm64/hyperv/hv_init.c
+++ b/arch/arm64/hyperv/hv_init.c
@@ -15,10 +15,78 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
#include
+
+/*
+ * Functions for allocating and freeing memory with size and
+ * alignment HV_HYP_PAGE_SIZE. These functions are needed because
+ * the guest page size may not be the same as the Hyper-V page
+ * size. And while kalloc() could allocate the memory, it does not
+ * guarantee the required alignment. So a separate small memory
+ * allocator is needed. The free function is rarely used, so it
+ * does not try to combine freed pages into larger chunks.
+ *
+ * These functions are used by arm64 specific code as well as
+ * arch independent Hyper-V drivers.
+ */
+
+static DEFINE_SPINLOCK(free_list_lock);
+static struct list_head free_list = LIST_HEAD_INIT(free_list);
+
+void *hv_alloc_hyperv_page(void)
+{
+ int i;
+ struct list_head *hv_page;
+ unsigned long addr;
+
+ BUILD_BUG_ON(HV_HYP_PAGE_SIZE > PAGE_SIZE);
+
+ spin_lock(_list_lock);
+ if (list_empty(_list)) {
+ spin_unlock(_list_lock);
+ addr = __get_free_page(GFP_KERNEL);
+ spin_lock(_list_lock);
+ for (i = 0; i < PAGE_SIZE; i += HV_HYP_PAGE_SIZE)
+ list_add_tail((struct list_head *)(addr + i),
+ _list);
+ }
+ hv_page = free_list.next;
+ list_del(hv_page);
+ spin_unlock(_list_lock);
+
+ return hv_page;
+}
+EXPORT_SYMBOL_GPL(hv_alloc_hyperv_page);
+
+void *hv_alloc_hyperv_zeroed_page(void)
+{
+ void *memp;
+
+ memp = hv_alloc_hyperv_page();
+ memset(memp, 0, HV_HYP_PAGE_SIZE);
+
+ return memp;
+}
+EXPORT_SYMBOL_GPL(hv_alloc_hyperv_zeroed_page);
+
+
+void hv_free_hyperv_page(unsigned long addr)
+{
+ if (!addr)
+ return;
+ spin_lock(_list_lock);
+ list_add((struct list_head *)addr, _list);
+ spin_unlock(_list_lock);
+}
+EXPORT_SYMBOL_GPL(hv_free_hyperv_page);
+
+
/*
* hv_do_hypercall- Invoke the specified hypercall
*/
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 18d8e2d..f9f3b66 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -99,6 +99,11 @@ static inline void vmbus_signal_eom(struct hv_message *msg,
u32 old_msg_type)
void hv_setup_crash_handler(void (*handler)(struct pt_regs *regs));
void hv_remove_crash_handler(void);
+void *hv_alloc_hyperv_page(void);
+void *hv_alloc_hyperv_zeroed_page(void);
+void hv_free_hyperv_page(unsigned long addr);
+
+
#if IS_ENABLED(CONFIG_HYPERV)
/*
* Hypervisor's notion of virtual processor ID is different from
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 0/8] Enable Linux guests on Hyper-V on ARM64
This series enables Linux guests running on Hyper-V on ARM64
hardware. New ARM64-specific code in arch/arm64/hyperv initializes
Hyper-V, including its interrupts and hypercall mechanism.
Existing architecture independent drivers for Hyper-V's VMbus and
synthetic devices just work when built for ARM64. Hyper-V code is
built and included in the image and modules only if CONFIG_HYPERV
is enabled.
The eight patches are organized as follows:
1) Add include files that define the Hyper-V interface as
described in the Hyper-V Top Level Functional Spec (TLFS), plus
additional definitions specific to Linux running on Hyper-V.
2 thru 6) Add core Hyper-V support on ARM64, including hypercalls,
interrupt handlers, kexec & panic handlers, and core hypervisor
initialization.
7) Update the existing VMbus driver to generalize interrupt
management across x86/x64 and ARM64.
8) Make CONFIG_HYPERV selectable on ARM64 in addition to x86/x64.
Some areas of Linux guests on Hyper-V on ARM64 are a work-
in-progress:
* Hyper-V on ARM64 currently runs with a 4 Kbyte page size, but
allows guests with 16K/64K page size. However, the Linux drivers
for Hyper-V synthetic devices assume the guest page size is 4K.
This patch set lays the groundwork for larger guest page sizes,
but the main page size changes are in a different patch stream
that is underway to update these drivers.
* The Hyper-V vPCI driver at drivers/pci/host/pci-hyperv.c has
x86/x64-specific code and is not being built for ARM64. Fixing
this driver to enable vPCI devices on ARM64 will be done later.
In a few cases, terminology from the x86/x64 world has been carried
over into the ARM64 code ("MSR", "TSC"). Hyper-V still uses the
x86/x64 terminology and has not replaced it with something more
generic, so the code uses the Hyper-V terminology. This will be
fixed when Hyper-V updates the usage in the TLFS.
This patch set is based on the 5.4-rc1-next-20191001 tree.
Changes in v5:
* Minor fixups to rebase to 5.4-rc1 linux-next
Changes in v4:
* Moved clock-related code into an architecture independent
Hyper-V clocksource driver that is already upstream. Clock
related code is removed from this patch set except for the
ARM64 specific interrupt handler. [Marc Zyngier]
* Separately upstreamed the split of mshyperv.h into arch independent
and arch dependent portions. The arch independent portion has been
removed from this patch set.
* Divided patch #2 of the series into multiple smaller patches
[Marc Zyngier]
* Changed a dozen or so smaller things based on feedback
[Marc Zyngier, Will Deacon]
* Added functions to alloc/free Hyper-V size pages for use by
drivers for Hyper-V synthetic devices when updated to not assume
guest page size and Hyper-v page size are the same
Changes in v3:
* Added initialization of hv_vp_index array like was recently
added on x86 branch [KY Srinivasan]
* Changed Hyper-V ARM64 register symbols to be all uppercase
instead of mixed case [KY Srinivasan]
* Separated mshyperv.h into two files, one architecture
independent and one architecture dependent. After this code
is upstream, will make changes to the x86 code to use the
architecture independent file and remove duplication. And
once we have a multi-architecture Hyper-V TLFS, will do a
separate patch to split hyperv-tlfs.h in the same way.
[KY Srinivasan]
* Minor tweaks to rebase to latest linux-next code
Changes in v2:
* Removed patch to implement slow_virt_to_phys() on ARM64.
Use of slow_virt_to_phys() in arch independent Hyper-V
drivers has been eliminated by commit 6ba34171bcbd
("Drivers: hv: vmbus: Remove use of slow_virt_to_phys()")
* Minor tweaks to rebase to latest linux-next code
Michael Kelley (8):
arm64: hyperv: Add core Hyper-V include files
arm64: hyperv: Add hypercall and register access functions
arm64: hyperv: Add memory alloc/free functions for Hyper-V size pages
arm64: hyperv: Add interrupt handlers for VMbus and stimer
arm64: hyperv: Add kexec and panic handlers
arm64: hyperv: Initialize hypervisor on boot
Drivers: hv: vmbus: Add hooks for per-CPU IRQ
Drivers: hv: Enable Hyper-V code to be built on ARM64
MAINTAINERS | 3 +
arch/arm64/Kbuild| 1 +
arch/arm64/hyperv/Makefile | 2 +
arch/arm64/hyperv/hv_hvc.S | 44
arch/arm64/hyperv/hv_init.c | 415 +++
arch/arm64/hyperv/mshyperv.c | 165 ++
arch/arm64/include/asm/hyperv-tlfs.h | 408 ++
arch/arm64/include/asm/mshyperv.h| 105 +
arch/x86/include/asm/mshyperv.h | 4 +
drivers/hv/Kconfig | 3 +-
drivers/hv/hv.c | 3 +
include/asm-generic/mshyperv.h | 5 +
12 files changed, 1157 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/hyperv/Makefile
create mode 100644 arch/arm64/hyperv/hv_hvc.S
create
[PATCH v5 2/8] arm64: hyperv: Add hypercall and register access functions
Add ARM64-specific code to make Hyper-V hypercalls and to
access virtual processor synthetic registers via hypercalls.
Hypercalls use a Hyper-V specific calling sequence with a non-zero
immediate value per Section 2.9 of the SMC Calling Convention
spec.
This code is architecture dependent and is mostly driven by
architecture independent code in the VMbus driver and the
Hyper-V timer clocksource driver.
This code is built only when CONFIG_HYPERV is enabled.
Signed-off-by: Michael Kelley
---
MAINTAINERS | 1 +
arch/arm64/Kbuild | 1 +
arch/arm64/hyperv/Makefile | 2 +
arch/arm64/hyperv/hv_hvc.S | 44 +++
arch/arm64/hyperv/hv_init.c | 133
5 files changed, 181 insertions(+)
create mode 100644 arch/arm64/hyperv/Makefile
create mode 100644 arch/arm64/hyperv/hv_hvc.S
create mode 100644 arch/arm64/hyperv/hv_init.c
diff --git a/MAINTAINERS b/MAINTAINERS
index d464067..84f76f9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7566,6 +7566,7 @@ F:arch/x86/kernel/cpu/mshyperv.c
F: arch/x86/hyperv
F: arch/arm64/include/asm/hyperv-tlfs.h
F: arch/arm64/include/asm/mshyperv.h
+F: arch/arm64/hyperv
F: drivers/clocksource/hyperv_timer.c
F: drivers/hid/hid-hyperv.c
F: drivers/hv/
diff --git a/arch/arm64/Kbuild b/arch/arm64/Kbuild
index d646582..2469421 100644
--- a/arch/arm64/Kbuild
+++ b/arch/arm64/Kbuild
@@ -3,4 +3,5 @@ obj-y += kernel/ mm/
obj-$(CONFIG_NET) += net/
obj-$(CONFIG_KVM) += kvm/
obj-$(CONFIG_XEN) += xen/
+obj-$(CONFIG_HYPERV) += hyperv/
obj-$(CONFIG_CRYPTO) += crypto/
diff --git a/arch/arm64/hyperv/Makefile b/arch/arm64/hyperv/Makefile
new file mode 100644
index 000..6bd8439
--- /dev/null
+++ b/arch/arm64/hyperv/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-y := hv_init.o hv_hvc.o
diff --git a/arch/arm64/hyperv/hv_hvc.S b/arch/arm64/hyperv/hv_hvc.S
new file mode 100644
index 000..09324ac
--- /dev/null
+++ b/arch/arm64/hyperv/hv_hvc.S
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Microsoft Hyper-V hypervisor invocation routines
+ *
+ * Copyright (C) 2018, Microsoft, Inc.
+ *
+ * Author : Michael Kelley
+ */
+
+#include
+
+ .text
+/*
+ * Do the HVC instruction. For Hyper-V the argument is always 1.
+ * x0 contains the hypercall control value, while additional registers
+ * vary depending on the hypercall, and whether the hypercall arguments
+ * are in memory or in registers (a "fast" hypercall per the Hyper-V
+ * TLFS). When the arguments are in memory x1 is the guest physical
+ * address of the input arguments, and x2 is the guest physical
+ * address of the output arguments. When the arguments are in
+ * registers, the register values depends on the hypercall. Note
+ * that this version cannot return any values in registers.
+ */
+ENTRY(hv_do_hvc)
+ hvc #1
+ ret
+ENDPROC(hv_do_hvc)
+
+/*
+ * This variant of HVC invocation is for hv_get_vpreg and
+ * hv_get_vpreg_128. The input parameters are passed in registers
+ * along with a pointer in x4 to where the output result should
+ * be stored. The output is returned in x15 and x16. x18 is used as
+ * scratch space to avoid buildng a stack frame, as Hyper-V does
+ * not preserve registers x0-x17.
+ */
+ENTRY(hv_do_hvc_fast_get)
+ mov x18, x4
+ hvc #1
+ str x15,[x18]
+ str x16,[x18,#8]
+ ret
+ENDPROC(hv_do_hvc_fast_get)
diff --git a/arch/arm64/hyperv/hv_init.c b/arch/arm64/hyperv/hv_init.c
new file mode 100644
index 000..6808bc8
--- /dev/null
+++ b/arch/arm64/hyperv/hv_init.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Initialization of the interface with Microsoft's Hyper-V hypervisor,
+ * and various low level utility routines for interacting with Hyper-V.
+ *
+ * Copyright (C) 2019, Microsoft, Inc.
+ *
+ * Author : Michael Kelley
+ */
+
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/*
+ * hv_do_hypercall- Invoke the specified hypercall
+ */
+u64 hv_do_hypercall(u64 control, void *input, void *output)
+{
+ u64 input_address;
+ u64 output_address;
+
+ input_address = input ? virt_to_phys(input) : 0;
+ output_address = output ? virt_to_phys(output) : 0;
+ return hv_do_hvc(control, input_address, output_address);
+}
+EXPORT_SYMBOL_GPL(hv_do_hypercall);
+
+/*
+ * hv_do_fast_hypercall8 -- Invoke the specified hypercall
+ * with arguments in registers instead of physical memory.
+ * Avoids the overhead of virt_to_phys for simple hypercalls.
+ */
+
+u64 hv_do_fast_hypercall8(u16 code, u64 input)
+{
+ u64 control;
+
+ control = (u64)code | HV_HYPERCALL_FAST_BIT;
+ return hv_do_hvc(control, input);
+}
+EXPORT_SYMBOL_GPL(hv_do_fast_hypercall8);
+
+
+/*
+ * Set a single VP register to a 64-bit value.
+ */
+void hv_set_vpreg(u32 msr, u64 value)
+{
+ union
[PATCH v5 5/8] arm64: hyperv: Add kexec and panic handlers
Add functions to set up and remove kexec and panic
handlers, and to inform Hyper-V about a guest panic.
These functions are called from architecture independent
code in the VMbus driver.
This code is built only when CONFIG_HYPERV is enabled.
Signed-off-by: Michael Kelley
---
arch/arm64/hyperv/hv_init.c | 61
arch/arm64/hyperv/mshyperv.c | 26 +++
2 files changed, 87 insertions(+)
diff --git a/arch/arm64/hyperv/hv_init.c b/arch/arm64/hyperv/hv_init.c
index 9c294f6..67350ec 100644
--- a/arch/arm64/hyperv/hv_init.c
+++ b/arch/arm64/hyperv/hv_init.c
@@ -199,3 +199,64 @@ void hv_get_vpreg_128(u32 msr, struct
hv_get_vp_register_output *result)
}
EXPORT_SYMBOL_GPL(hv_get_vpreg_128);
+
+void hyperv_report_panic(struct pt_regs *regs, long err)
+{
+ static bool panic_reported;
+ u64 guest_id;
+
+ /*
+* We prefer to report panic on 'die' chain as we have proper
+* registers to report, but if we miss it (e.g. on BUG()) we need
+* to report it on 'panic'.
+*/
+ if (panic_reported)
+ return;
+ panic_reported = true;
+
+ guest_id = hv_get_vpreg(HV_REGISTER_GUEST_OSID);
+
+ /*
+* Hyper-V provides the ability to store only 5 values.
+* Pick the passed in error value, the guest_id, and the PC.
+* The first two general registers are added arbitrarily.
+*/
+ hv_set_vpreg(HV_REGISTER_CRASH_P0, err);
+ hv_set_vpreg(HV_REGISTER_CRASH_P1, guest_id);
+ hv_set_vpreg(HV_REGISTER_CRASH_P2, regs->pc);
+ hv_set_vpreg(HV_REGISTER_CRASH_P3, regs->regs[0]);
+ hv_set_vpreg(HV_REGISTER_CRASH_P4, regs->regs[1]);
+
+ /*
+* Let Hyper-V know there is crash data available
+*/
+ hv_set_vpreg(HV_REGISTER_CRASH_CTL, HV_CRASH_CTL_CRASH_NOTIFY);
+}
+EXPORT_SYMBOL_GPL(hyperv_report_panic);
+
+/*
+ * hyperv_report_panic_msg - report panic message to Hyper-V
+ * @pa: physical address of the panic page containing the message
+ * @size: size of the message in the page
+ */
+void hyperv_report_panic_msg(phys_addr_t pa, size_t size)
+{
+ /*
+* P3 to contain the physical address of the panic page & P4 to
+* contain the size of the panic data in that page. Rest of the
+* registers are no-op when the NOTIFY_MSG flag is set.
+*/
+ hv_set_vpreg(HV_REGISTER_CRASH_P0, 0);
+ hv_set_vpreg(HV_REGISTER_CRASH_P1, 0);
+ hv_set_vpreg(HV_REGISTER_CRASH_P2, 0);
+ hv_set_vpreg(HV_REGISTER_CRASH_P3, pa);
+ hv_set_vpreg(HV_REGISTER_CRASH_P4, size);
+
+ /*
+* Let Hyper-V know there is crash data available along with
+* the panic message.
+*/
+ hv_set_vpreg(HV_REGISTER_CRASH_CTL,
+ (HV_CRASH_CTL_CRASH_NOTIFY | HV_CRASH_CTL_CRASH_NOTIFY_MSG));
+}
+EXPORT_SYMBOL_GPL(hyperv_report_panic_msg);
diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
index ae6ece6..c58940d 100644
--- a/arch/arm64/hyperv/mshyperv.c
+++ b/arch/arm64/hyperv/mshyperv.c
@@ -23,6 +23,8 @@
static void (*vmbus_handler)(void);
static void (*hv_stimer0_handler)(void);
+static void (*hv_kexec_handler)(void);
+static void (*hv_crash_handler)(struct pt_regs *regs);
static int vmbus_irq;
static long __percpu *vmbus_evt;
@@ -137,3 +139,27 @@ void hv_remove_stimer0_irq(int irq)
}
}
EXPORT_SYMBOL_GPL(hv_remove_stimer0_irq);
+
+void hv_setup_kexec_handler(void (*handler)(void))
+{
+ hv_kexec_handler = handler;
+}
+EXPORT_SYMBOL_GPL(hv_setup_kexec_handler);
+
+void hv_remove_kexec_handler(void)
+{
+ hv_kexec_handler = NULL;
+}
+EXPORT_SYMBOL_GPL(hv_remove_kexec_handler);
+
+void hv_setup_crash_handler(void (*handler)(struct pt_regs *regs))
+{
+ hv_crash_handler = handler;
+}
+EXPORT_SYMBOL_GPL(hv_setup_crash_handler);
+
+void hv_remove_crash_handler(void)
+{
+ hv_crash_handler = NULL;
+}
+EXPORT_SYMBOL_GPL(hv_remove_crash_handler);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 4/8] arm64: hyperv: Add interrupt handlers for VMbus and stimer
Add ARM64-specific code to set up and handle the interrupts
generated by Hyper-V for VMbus messages and for stimer expiration.
This code is architecture dependent and is mostly driven by
architecture independent code in the VMbus driver and the
Hyper-V timer clocksource driver.
This code is built only when CONFIG_HYPERV is enabled.
Signed-off-by: Michael Kelley
---
arch/arm64/hyperv/Makefile | 2 +-
arch/arm64/hyperv/mshyperv.c | 139 +++
2 files changed, 140 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/hyperv/mshyperv.c
diff --git a/arch/arm64/hyperv/Makefile b/arch/arm64/hyperv/Makefile
index 6bd8439..988eda5 100644
--- a/arch/arm64/hyperv/Makefile
+++ b/arch/arm64/hyperv/Makefile
@@ -1,2 +1,2 @@
# SPDX-License-Identifier: GPL-2.0
-obj-y := hv_init.o hv_hvc.o
+obj-y := hv_init.o hv_hvc.o mshyperv.o
diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
new file mode 100644
index 000..ae6ece6
--- /dev/null
+++ b/arch/arm64/hyperv/mshyperv.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Core routines for interacting with Microsoft's Hyper-V hypervisor,
+ * including setting up VMbus and STIMER interrupts, and handling
+ * crashes and kexecs. These interactions are through a set of
+ * static "handler" variables set by the architecture independent
+ * VMbus and STIMER drivers.
+ *
+ * Copyright (C) 2019, Microsoft, Inc.
+ *
+ * Author : Michael Kelley
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static void (*vmbus_handler)(void);
+static void (*hv_stimer0_handler)(void);
+
+static int vmbus_irq;
+static long __percpu *vmbus_evt;
+static long __percpu *stimer0_evt;
+
+irqreturn_t hyperv_vector_handler(int irq, void *dev_id)
+{
+ vmbus_handler();
+ return IRQ_HANDLED;
+}
+
+/* Must be done just once */
+void hv_setup_vmbus_irq(void (*handler)(void))
+{
+ int result;
+
+ vmbus_handler = handler;
+ vmbus_irq = acpi_register_gsi(NULL, HYPERVISOR_CALLBACK_VECTOR,
+ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_HIGH);
+ if (vmbus_irq <= 0) {
+ pr_err("Can't register Hyper-V VMBus GSI. Error %d",
+ vmbus_irq);
+ vmbus_irq = 0;
+ return;
+ }
+ vmbus_evt = alloc_percpu(long);
+ result = request_percpu_irq(vmbus_irq, hyperv_vector_handler,
+ "Hyper-V VMbus", vmbus_evt);
+ if (result) {
+ pr_err("Can't request Hyper-V VMBus IRQ %d. Error %d",
+ vmbus_irq, result);
+ free_percpu(vmbus_evt);
+ acpi_unregister_gsi(vmbus_irq);
+ vmbus_irq = 0;
+ }
+}
+EXPORT_SYMBOL_GPL(hv_setup_vmbus_irq);
+
+/* Must be done just once */
+void hv_remove_vmbus_irq(void)
+{
+ if (vmbus_irq) {
+ free_percpu_irq(vmbus_irq, vmbus_evt);
+ free_percpu(vmbus_evt);
+ acpi_unregister_gsi(vmbus_irq);
+ }
+}
+EXPORT_SYMBOL_GPL(hv_remove_vmbus_irq);
+
+/* Must be done by each CPU */
+void hv_enable_vmbus_irq(void)
+{
+ enable_percpu_irq(vmbus_irq, 0);
+}
+EXPORT_SYMBOL_GPL(hv_enable_vmbus_irq);
+
+/* Must be done by each CPU */
+void hv_disable_vmbus_irq(void)
+{
+ disable_percpu_irq(vmbus_irq);
+}
+EXPORT_SYMBOL_GPL(hv_disable_vmbus_irq);
+
+/* Routines to do per-architecture handling of STIMER0 when in Direct Mode */
+
+static irqreturn_t hv_stimer0_vector_handler(int irq, void *dev_id)
+{
+ if (hv_stimer0_handler)
+ hv_stimer0_handler();
+ return IRQ_HANDLED;
+}
+
+int hv_setup_stimer0_irq(int *irq, int *vector, void (*handler)(void))
+{
+ int localirq;
+ int result;
+
+ localirq = acpi_register_gsi(NULL, HV_STIMER0_IRQNR,
+ ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_HIGH);
+ if (localirq <= 0) {
+ pr_err("Can't register Hyper-V stimer0 GSI. Error %d",
+ localirq);
+ *irq = 0;
+ return -1;
+ }
+ stimer0_evt = alloc_percpu(long);
+ result = request_percpu_irq(localirq, hv_stimer0_vector_handler,
+"Hyper-V stimer0", stimer0_evt);
+ if (result) {
+ pr_err("Can't request Hyper-V stimer0 IRQ %d. Error %d",
+ localirq, result);
+ free_percpu(stimer0_evt);
+ acpi_unregister_gsi(localirq);
+ *irq = 0;
+ return -1;
+ }
+
+ hv_stimer0_handler = handler;
+ *vector = HV_STIMER0_IRQNR;
+ *irq = localirq;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(hv_setup_stimer0_irq);
+
+void hv_remove_stimer0_irq(int irq)
+{
+ hv_stimer0_handler = NULL;
+ if (irq) {
+ free_percpu_irq(irq, stimer0_evt);
+ free_percpu(stimer0_evt);
+
[PATCH] staging: vc04_services: Avoid typedef
Avoid typedefs to maintain kernel coding style. Issue found by
checkpatch.pl
Replace the enum typedef VCHIQ_REASON_T with vchiq_reason.
Signed-off-by: Nachammai Karuppiah
---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 8
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++--
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h| 6 +++---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_ioctl.h | 2 +-
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c | 2 +-
5 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index b1595b1..280e237 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -516,7 +516,7 @@ vchiq_blocking_bulk_transfer(VCHIQ_SERVICE_HANDLE_T handle,
void *data,
***/
static VCHIQ_STATUS_T
-add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T reason,
+add_completion(VCHIQ_INSTANCE_T instance, enum vchiq_reason reason,
struct vchiq_header *header, struct user_service *user_service,
void *bulk_userdata)
{
@@ -583,7 +583,7 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T
reason,
***/
static VCHIQ_STATUS_T
-service_callback(VCHIQ_REASON_T reason, struct vchiq_header *header,
+service_callback(enum vchiq_reason reason, struct vchiq_header *header,
VCHIQ_SERVICE_HANDLE_T handle, void *bulk_userdata)
{
/* How do we ensure the callback goes to the right client?
@@ -1666,7 +1666,7 @@ vchiq_compat_ioctl_queue_bulk(struct file *file,
}
struct vchiq_completion_data32 {
- VCHIQ_REASON_T reason;
+ enum vchiq_reason reason;
compat_uptr_t header;
compat_uptr_t service_userdata;
compat_uptr_t bulk_userdata;
@@ -2271,7 +2271,7 @@ vchiq_videocore_wanted(struct vchiq_state *state)
}
static VCHIQ_STATUS_T
-vchiq_keepalive_vchiq_callback(VCHIQ_REASON_T reason,
+vchiq_keepalive_vchiq_callback(enum vchiq_reason reason,
struct vchiq_header *header,
VCHIQ_SERVICE_HANDLE_T service_user,
void *bulk_user)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 56a23a2..b0e0653 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -355,7 +355,7 @@ mark_service_closing(struct vchiq_service *service)
}
static inline VCHIQ_STATUS_T
-make_service_callback(struct vchiq_service *service, VCHIQ_REASON_T reason,
+make_service_callback(struct vchiq_service *service, enum vchiq_reason reason,
struct vchiq_header *header, void *bulk_userdata)
{
VCHIQ_STATUS_T status;
@@ -1230,7 +1230,7 @@ notify_bulks(struct vchiq_service *service, struct
vchiq_bulk_queue *queue,
spin_unlock(_waiter_spinlock);
} else if (bulk->mode ==
VCHIQ_BULK_MODE_CALLBACK) {
- VCHIQ_REASON_T reason = (bulk->dir ==
+ enum vchiq_reason reason = (bulk->dir ==
VCHIQ_BULK_TRANSMIT) ?
((bulk->actual ==
VCHIQ_BULK_ACTUAL_ABORTED) ?
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
index c23bd10..f911612 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_if.h
@@ -15,7 +15,7 @@
#define VCHIQ_GET_SERVICE_USERDATA(service) vchiq_get_service_userdata(service)
#define VCHIQ_GET_SERVICE_FOURCC(service) vchiq_get_service_fourcc(service)
-typedef enum {
+enum vchiq_reason {
VCHIQ_SERVICE_OPENED, /* service, -, - */
VCHIQ_SERVICE_CLOSED, /* service, -, - */
VCHIQ_MESSAGE_AVAILABLE, /* service, header, -*/
@@ -23,7 +23,7 @@ typedef enum {
VCHIQ_BULK_RECEIVE_DONE, /* service, -, bulk_userdata */
VCHIQ_BULK_TRANSMIT_ABORTED, /* service, -, bulk_userdata */
VCHIQ_BULK_RECEIVE_ABORTED/* service, -, bulk_userdata */
-} VCHIQ_REASON_T;
+};
typedef enum {
VCHIQ_ERROR = -1,
@@ -63,7 +63,7 @@ struct vchiq_element {
typedef unsigned int VCHIQ_SERVICE_HANDLE_T;
-typedef VCHIQ_STATUS_T (*VCHIQ_CALLBACK_T)(VCHIQ_REASON_T,
+typedef VCHIQ_STATUS_T
Re: [PATCH] staging: wlan-ng: fix uninitialized variable
Hi, On 10/3/19 2:26 PM, Dan Carpenter wrote: > On Wed, Oct 02, 2019 at 08:41:03PM +0300, Denis Efremov wrote: >> The result variable in prism2_connect() can be used uninitialized on path >> !channel --> ... --> is_wep --> sme->key --> sme->key_idx >= NUM_WEPKEYS. >> This patch initializes result with 0. >> >> Cc: Greg Kroah-Hartman >> Cc: sta...@vger.kernel.org >> Signed-off-by: Denis Efremov >> --- >> drivers/staging/wlan-ng/cfg80211.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/staging/wlan-ng/cfg80211.c >> b/drivers/staging/wlan-ng/cfg80211.c >> index eee1998c4b18..d426905e187e 100644 >> --- a/drivers/staging/wlan-ng/cfg80211.c >> +++ b/drivers/staging/wlan-ng/cfg80211.c >> @@ -441,7 +441,7 @@ static int prism2_connect(struct wiphy *wiphy, struct >> net_device *dev, >> int chan = -1; >> int is_wep = (sme->crypto.cipher_group == WLAN_CIPHER_SUITE_WEP40) || >> (sme->crypto.cipher_group == WLAN_CIPHER_SUITE_WEP104); >> -int result; >> +int result = 0; >> int err = 0; >> > > I can't see any reason why we should have both "err" and "result". > Maybe in olden times "result" used to save positive error codes instead > of negative error codes but now it's just negatives and zero on success. > There is no reason for the exit label either, we could just return > directly. > > So could you redo it and get rid of "result" entirely? Otherwise it > just causes more bugs like this. > Yes, of course. I will prepare v2. Thanks, Denis ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: fix boundary condition for n
On Wed, Oct 02, 2019 at 10:35:19PM +0530, Rohit Sarkar wrote: > Now that snprintf is replaced by scnprintf n >= MAX_WPA_IE_LEN doesn't > make sense as the maximum value n can take is MAX_WPA_IE_LEN. > > Signed-off-by: Rohit Sarkar > --- Thanks! Reviewed-by: Dan Carpenter regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] staging: rtl8188eu: cleanup comments in update_hw_ht_param
Cleanup comments in update_hw_ht_param to follow kernel coding style
and avoid line length over 80 characters.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_ap.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c
b/drivers/staging/rtl8188eu/core/rtw_ap.c
index 97cab71cef23..9aa44c921aca 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -567,20 +567,17 @@ static void update_hw_ht_param(struct adapter *padapter)
DBG_88E("%s\n", __func__);
- /* handle A-MPDU parameter field */
- /*
- ampdu_params_info [1:0]:Max AMPDU Len => 0:8k , 1:16k, 2:32k,
3:64k
- ampdu_params_info [4:2]:Min MPDU Start Spacing
- */
+ /* handle A-MPDU parameter field
+* ampdu_params_info [1:0]:Max AMPDU Len => 0:8k , 1:16k, 2:32k, 3:64k
+* ampdu_params_info [4:2]:Min MPDU Start Spacing
+*/
max_ampdu_len = pmlmeinfo->HT_caps.ampdu_params_info & 0x03;
min_mpdu_spacing = (pmlmeinfo->HT_caps.ampdu_params_info & 0x1c) >> 2;
rtw_hal_set_hwreg(padapter, HW_VAR_AMPDU_MIN_SPACE, _mpdu_spacing);
rtw_hal_set_hwreg(padapter, HW_VAR_AMPDU_FACTOR, _ampdu_len);
- /* */
- /* Config SM Power Save setting */
- /* */
+ /* Config SM Power Save setting */
pmlmeinfo->SM_PS = (le16_to_cpu(pmlmeinfo->HT_caps.cap_info) & 0x0C) >>
2;
if (pmlmeinfo->SM_PS == WLAN_HT_CAP_SM_PS_STATIC)
DBG_88E("%s(): WLAN_HT_CAP_SM_PS_STATIC\n", __func__);
--
2.23.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] staging: rtl8188eu: convert variables from unsigned char to u8
Convert the local variables max_AMPDU_len and min_MPDU_spacing from
unsigned char to u8 and remove unnecessary castings to u8 pointer.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_ap.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c
b/drivers/staging/rtl8188eu/core/rtw_ap.c
index 51a5b71f8c25..1e4461a74474 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -560,8 +560,8 @@ void update_sta_info_apmode(struct adapter *padapter,
struct sta_info *psta)
static void update_hw_ht_param(struct adapter *padapter)
{
- unsigned char max_AMPDU_len;
- unsigned char min_MPDU_spacing;
+ u8 max_AMPDU_len;
+ u8 min_MPDU_spacing;
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
struct mlme_ext_info*pmlmeinfo = >mlmext_info;
@@ -576,9 +576,9 @@ static void update_hw_ht_param(struct adapter *padapter)
min_MPDU_spacing = (pmlmeinfo->HT_caps.ampdu_params_info & 0x1c) >> 2;
- rtw_hal_set_hwreg(padapter, HW_VAR_AMPDU_MIN_SPACE, (u8
*)(_MPDU_spacing));
+ rtw_hal_set_hwreg(padapter, HW_VAR_AMPDU_MIN_SPACE, _MPDU_spacing);
- rtw_hal_set_hwreg(padapter, HW_VAR_AMPDU_FACTOR, (u8
*)(_AMPDU_len));
+ rtw_hal_set_hwreg(padapter, HW_VAR_AMPDU_FACTOR, _AMPDU_len);
/* */
/* Config SM Power Save setting */
--
2.23.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] staging: rtl8188eu: cleanup whitespace in update_hw_ht_param
Replace tabs with spaces in declarations and reomve two blank lines in
update_hw_ht_param to cleanup whitespace and improve readability.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_ap.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c
b/drivers/staging/rtl8188eu/core/rtw_ap.c
index 75c34e8f2335..97cab71cef23 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -562,8 +562,8 @@ static void update_hw_ht_param(struct adapter *padapter)
{
u8 max_ampdu_len;
u8 min_mpdu_spacing;
- struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = >mlmext_info;
+ struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
+ struct mlme_ext_info *pmlmeinfo = >mlmext_info;
DBG_88E("%s\n", __func__);
@@ -573,11 +573,9 @@ static void update_hw_ht_param(struct adapter *padapter)
ampdu_params_info [4:2]:Min MPDU Start Spacing
*/
max_ampdu_len = pmlmeinfo->HT_caps.ampdu_params_info & 0x03;
-
min_mpdu_spacing = (pmlmeinfo->HT_caps.ampdu_params_info & 0x1c) >> 2;
rtw_hal_set_hwreg(padapter, HW_VAR_AMPDU_MIN_SPACE, _mpdu_spacing);
-
rtw_hal_set_hwreg(padapter, HW_VAR_AMPDU_FACTOR, _ampdu_len);
/* */
--
2.23.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] staging: rtl8188eu: cleanups in update_hw_ht_param()
Cleanup code in function update_hw_ht_param(). Michael Straube (4): staging: rtl8188eu: convert variables from unsigned char to u8 staging: rtl8188eu: rename variables to avoid mixed case staging: rtl8188eu: cleanup whitespace in update_hw_ht_param staging: rtl8188eu: cleanup comments in update_hw_ht_param drivers/staging/rtl8188eu/core/rtw_ap.c | 31 +++-- 1 file changed, 13 insertions(+), 18 deletions(-) -- 2.23.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] staging: rtl8188eu: rename variables to avoid mixed case
Rename the local varibles max_AMPDU_len and min_MPDU_spacing to avoid
mixed case.
max_AMPDU_len -> max_ampdu_len
min_MPDU_spacing -> min_mpdu_spacing
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_ap.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c
b/drivers/staging/rtl8188eu/core/rtw_ap.c
index 1e4461a74474..75c34e8f2335 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -560,8 +560,8 @@ void update_sta_info_apmode(struct adapter *padapter,
struct sta_info *psta)
static void update_hw_ht_param(struct adapter *padapter)
{
- u8 max_AMPDU_len;
- u8 min_MPDU_spacing;
+ u8 max_ampdu_len;
+ u8 min_mpdu_spacing;
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
struct mlme_ext_info*pmlmeinfo = >mlmext_info;
@@ -572,13 +572,13 @@ static void update_hw_ht_param(struct adapter *padapter)
ampdu_params_info [1:0]:Max AMPDU Len => 0:8k , 1:16k, 2:32k,
3:64k
ampdu_params_info [4:2]:Min MPDU Start Spacing
*/
- max_AMPDU_len = pmlmeinfo->HT_caps.ampdu_params_info & 0x03;
+ max_ampdu_len = pmlmeinfo->HT_caps.ampdu_params_info & 0x03;
- min_MPDU_spacing = (pmlmeinfo->HT_caps.ampdu_params_info & 0x1c) >> 2;
+ min_mpdu_spacing = (pmlmeinfo->HT_caps.ampdu_params_info & 0x1c) >> 2;
- rtw_hal_set_hwreg(padapter, HW_VAR_AMPDU_MIN_SPACE, _MPDU_spacing);
+ rtw_hal_set_hwreg(padapter, HW_VAR_AMPDU_MIN_SPACE, _mpdu_spacing);
- rtw_hal_set_hwreg(padapter, HW_VAR_AMPDU_FACTOR, _AMPDU_len);
+ rtw_hal_set_hwreg(padapter, HW_VAR_AMPDU_FACTOR, _ampdu_len);
/* */
/* Config SM Power Save setting */
--
2.23.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: exfat: use bdev_sync function directly where needed
Or we could apply your other patch which trumps both this patch and the patch to the TODO. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: exfat: use bdev_sync function directly where needed
I replied to your other thread and I added Saiyam Doshi to the CC list there. Just to be clear this patch is a good cleanup and doesn't affect runtime at all. In the other thread, I suggested that we leave fs_sync() as a marker even though it's dead code. But looking at it now, I think that it's not really useful. Future auditors should look for places which call fs_set_vol_flags(sb, VOL_CLEAN); instead. That's exactly the places which call fs_sync(). regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers/staging/exfat - explain the fs_sync() issue in TODO
On Wed, Oct 02, 2019 at 03:01:35PM -0400, Valdis Klētnieks wrote:
> We've seen several incorrect patches for fs_sync() calls in the exfat driver.
> Add code to the TODO that explains this isn't just a delete code and refactor,
> but that actual analysis of when the filesystem should be flushed to disk
> needs to be done.
>
This doesn't help at all because no one can be expected to read it.
Put a comment in the code which says something like:
diff --git a/drivers/staging/exfat/exfat_super.c
b/drivers/staging/exfat/exfat_super.c
index 229ecabe7a93..c1710d99875e 100644
--- a/drivers/staging/exfat/exfat_super.c
+++ b/drivers/staging/exfat/exfat_super.c
@@ -287,6 +287,13 @@ static DEFINE_SEMAPHORE(z_sem);
static inline void fs_sync(struct super_block *sb, bool do_sync)
{
+ /*
+* Oct 2019: Please, do not delete this code or the callers. This
+* code is obviously bogus and many of the callers are dead code, yes,
+* but it may hold clues as to when syncing is required. Someone needs
+* to go through and audit it really carefully.
+*
+*/
if (do_sync)
bdev_sync(sb);
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wlan-ng: fix uninitialized variable
On Wed, Oct 02, 2019 at 08:41:03PM +0300, Denis Efremov wrote: > The result variable in prism2_connect() can be used uninitialized on path > !channel --> ... --> is_wep --> sme->key --> sme->key_idx >= NUM_WEPKEYS. > This patch initializes result with 0. > > Cc: Greg Kroah-Hartman > Cc: sta...@vger.kernel.org > Signed-off-by: Denis Efremov > --- > drivers/staging/wlan-ng/cfg80211.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/wlan-ng/cfg80211.c > b/drivers/staging/wlan-ng/cfg80211.c > index eee1998c4b18..d426905e187e 100644 > --- a/drivers/staging/wlan-ng/cfg80211.c > +++ b/drivers/staging/wlan-ng/cfg80211.c > @@ -441,7 +441,7 @@ static int prism2_connect(struct wiphy *wiphy, struct > net_device *dev, > int chan = -1; > int is_wep = (sme->crypto.cipher_group == WLAN_CIPHER_SUITE_WEP40) || > (sme->crypto.cipher_group == WLAN_CIPHER_SUITE_WEP104); > - int result; > + int result = 0; > int err = 0; > I can't see any reason why we should have both "err" and "result". Maybe in olden times "result" used to save positive error codes instead of negative error codes but now it's just negatives and zero on success. There is no reason for the exit label either, we could just return directly. So could you redo it and get rid of "result" entirely? Otherwise it just causes more bugs like this. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: exfat: use bdev_sync function directly where needed
On Wed, Oct 02, 2019 at 08:47:03PM +0530, Saiyam Doshi wrote: > fs_sync() is wrapper to bdev_sync(). When fs_sync is called with > non-zero argument, bdev_sync gets called. > > Most instances of fs_sync is called with false and very few with > true. Refactor this and makes direct call to bdev_sync() where > needed and removes fs_sync definition. > > Signed-off-by: Saiyam Doshi Reviewed-by: Dan Carpenter regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
