| Date | Mon, 18 Dec 2023 22:50:57 +0100 | Subject | Re: [PATCH v2 11/34] media: iris: introduce packetization layer for creating HFI packets | From | Konrad Dybcio <> |
| |
On 12/18/23 12:32, Dikshita Agarwal wrote: > Host firmware interface (HFI) is well defined set of interfaces > for communication between host driver and firmware. > The command and responses are exchanged in form of packets. > One or multiple packets are grouped under packet header. > Each packet has packet type which describes the specific HFI > and payload which holds the corresponding value for that HFI. > > Sys_init is the first packets sent to firmware, which initializes > the firmware. Sys_image_version packet is to get the firmware > version string. > > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- [...]
> struct iris_core { > @@ -65,6 +70,11 @@ struct iris_core { > struct mem_desc sfr; > struct mutex lock; /* lock for core structure */ > unsigned int use_tz; > + u8 *packet; > + u32 packet_size; I'm not sure it's necessary to always keep a reference to the last packet in the core struct, especially since it needs to be allocated first anyway
> + u32 sys_init_id; This looks like a hyper-defensive measure against some firmware overtaking attacks.. Or a way to spot random/unwanted resets of the firmware core..
Is it actually necessary, or does this just serve as a debug feature?
> + u32 header_id; Similar to above..
> + u32 packet_id; And here.
I performed some quick CTRL-F-agge around the series and this is never reset.. Can the firmware cope with this? What if I watch a veeeery long youtube video that ends up creating more than (1<<32)-1 HFI packets while playing?
> + > +enum hfi_packet_host_flags { > + HFI_HOST_FLAGS_NONE = 0x00000000, > + HFI_HOST_FLAGS_INTR_REQUIRED = 0x00000001, > + HFI_HOST_FLAGS_RESPONSE_REQUIRED = 0x00000002, > + HFI_HOST_FLAGS_NON_DISCARDABLE = 0x00000004, > + HFI_HOST_FLAGS_GET_PROPERTY = 0x00000008, BIT(n)?
> +}; > + > +enum hfi_packet_firmware_flags { > + HFI_FW_FLAGS_NONE = 0x00000000, > + HFI_FW_FLAGS_SUCCESS = 0x00000001, > + HFI_FW_FLAGS_INFORMATION = 0x00000002, > + HFI_FW_FLAGS_SESSION_ERROR = 0x00000004, > + HFI_FW_FLAGS_SYSTEM_ERROR = 0x00000008, BIT(n)?
Konrad
|