[PATCH] drm/i915/gvt: use the correct length of child_device_config
Li, Weinan Z
weinan.z.li at intel.com
Fri Aug 31 03:25:53 UTC 2018
> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw at linux.intel.com]
> Sent: Friday, August 31, 2018 10:46 AM
> To: Li, Weinan Z <weinan.z.li at intel.com>
> Cc: intel-gvt-dev at lists.freedesktop.org; Zhang, Xiaolin
> <xiaolin.zhang at intel.com>
> Subject: Re: [PATCH] drm/i915/gvt: use the correct length of
> child_device_config
>
> On 2018.08.29 12:17:56 +0800, Weinan Li wrote:
> > GVT-g emualte the opregion for guest with bdb version as '186' which
> > child_device_config length should be '33'.
> >
> > CC: Xiaolin Zhang <xiaolin.zhang at intel.com>
> > Signed-off-by: Weinan Li <weinan.z.li at intel.com>
> > ---
> > drivers/gpu/drm/i915/gvt/opregion.c | 142
> > ++++++++++++++++++++++--------------
> > 1 file changed, 88 insertions(+), 54 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/opregion.c
> > b/drivers/gpu/drm/i915/gvt/opregion.c
> > index 82586c8..5895869 100644
> > --- a/drivers/gpu/drm/i915/gvt/opregion.c
> > +++ b/drivers/gpu/drm/i915/gvt/opregion.c
> > @@ -42,8 +42,6 @@
> > #define DEVICE_TYPE_EFP3 0x20
> > #define DEVICE_TYPE_EFP4 0x10
> >
> > -#define DEV_SIZE 38
> > -
>
> Looks what this patch requires is just to fix DEV_SIZE as 33, then other change
> seems not necessary? Or we have to remove extra fields in struct
> child_dev_config?
>
The size should be the length of child_device_config, otherwise it will impact the guest opregion parser to parse the child_device_config, then may get unexpected data. So remove the extra fields is necessary I think.
> As new added config fields seem not to be used, why we have to add that?
>
> > struct opregion_header {
> > u8 signature[16];
> > u32 size;
> > @@ -63,58 +61,92 @@ struct bdb_data_header {
> > u16 size; /* data size */
> > } __packed;
> >
> > +/* For supporting windows guest with opregion, here hardcode the
> > +emulated
> > + * bdb header version as '186', and the corresponding
> > +child_device_config
> > + * length should be '33' but not '38'. Define one new struct named as
> > + * efp_child_device_config which is one copy of child_device_config
> > +but
> > + * without fields which added from version '195'.
> > + */
> > struct efp_child_device_config {
> > u16 handle;
> > u16 device_type;
> > - u16 device_class;
> > - u8 i2c_speed;
> > - u8 dp_onboard_redriver; /* 158 */
> > - u8 dp_ondock_redriver; /* 158 */
> > - u8 hdmi_level_shifter_value:4; /* 169 */
> > - u8 hdmi_max_data_rate:4; /* 204 */
> > - u16 dtd_buf_ptr; /* 161 */
> > - u8 edidless_efp:1; /* 161 */
> > - u8 compression_enable:1; /* 198 */
> > - u8 compression_method:1; /* 198 */
> > - u8 ganged_edp:1; /* 202 */
> > - u8 skip0:4;
> > - u8 compression_structure_index:4; /* 198 */
> > - u8 skip1:4;
> > - u8 slave_port; /* 202 */
> > - u8 skip2;
> > +
> > + union {
> > + u8 device_id[10]; /* ascii string */
> > + struct {
> > + u8 i2c_speed;
> > + u8 dp_onboard_redriver; /* 158 */
> > + u8 dp_ondock_redriver; /* 158 */
> > + u8 hdmi_level_shifter_value:4; /* 169 */
> > + u8 hdmi_max_data_rate:4; /* 204 */
> > + u16 dtd_buf_ptr; /* 161 */
> > + u8 edidless_efp:1; /* 161 */
> > + u8 compression_enable:1; /* 198 */
> > + u8 compression_method:1; /* 198 */
> > + u8 ganged_edp:1; /* 202 */
> > + u8 reserved0:4;
> > + u8 compression_structure_index:4; /* 198 */
> > + u8 reserved1:4;
> > + u8 slave_port; /* 202 */
> > + u8 reserved2;
> > + } __packed;
> > + } __packed;
> > +
> > + u16 addin_offset;
> > u8 dvo_port;
> > - u8 i2c_pin; /* for add-in card */
> > - u8 slave_addr; /* for add-in card */
> > + u8 i2c_pin;
> > + u8 slave_addr;
> > u8 ddc_pin;
> > u16 edid_ptr;
> > - u8 dvo_config;
> > - u8 efp_docked_port:1; /* 158 */
> > - u8 lane_reversal:1; /* 184 */
> > - u8 onboard_lspcon:1; /* 192 */
> > - u8 iboost_enable:1; /* 196 */
> > - u8 hpd_invert:1; /* BXT 196 */
> > - u8 slip3:3;
> > - u8 hdmi_compat:1;
> > - u8 dp_compat:1;
> > - u8 tmds_compat:1;
> > - u8 skip4:5;
> > - u8 aux_channel;
> > - u8 dongle_detect;
> > - u8 pipe_cap:2;
> > - u8 sdvo_stall:1; /* 158 */
> > - u8 hpd_status:2;
> > - u8 integrated_encoder:1;
> > - u8 skip5:2;
> > - u8 dvo_wiring;
> > - u8 mipi_bridge_type; /* 171 */
> > - u16 device_class_ext;
> > + u8 dvo_cfg; /* See DEVICE_CFG_* above */
> > +
> > + union {
> > + struct {
> > + u8 dvo2_port;
> > + u8 i2c2_pin;
> > + u8 slave2_addr;
> > + u8 ddc2_pin;
> > + } __packed;
> > + struct {
> > + u8 efp_routed:1; /* 158 */
> > + u8 lane_reversal:1; /* 184 */
> > + u8 lspcon:1; /* 192 */
> > + u8 iboost:1; /* 196 */
> > + u8 hpd_invert:1; /* 196 */
> > + u8 flag_reserved:3;
> > + u8 hdmi_support:1; /* 158 */
> > + u8 dp_support:1; /* 158 */
> > + u8 tmds_support:1; /* 158 */
> > + u8 support_reserved:5;
> > + u8 aux_channel;
> > + u8 dongle_detect;
> > + } __packed;
> > + } __packed;
> > +
> > + union {
> > + u8 capabilities;
> > + struct {
> > + u8 pipe_cap:2;
> > + u8 sdvo_stall:1;
> > + u8 hpd_status:2;
> > + u8 integrated_encoder:1; /* 158 */
> > + u8 skip5:2;
> > + } __packed;
> > + } __packed;
> > +
> > + u8 dvo_wiring; /* See DEVICE_WIRE_* above */
> > +
> > + union {
> > + u8 dvo2_wiring;
> > + u8 mipi_bridge_type; /* 171 */
> > + } __packed;
> > +
> > + u16 extended_type;
> > u8 dvo_function;
> > - u8 dp_usb_type_c:1; /* 195 */
> > - u8 skip6:7;
> > - u8 dp_usb_type_c_2x_gpio_index; /* 195 */
> > - u16 dp_usb_type_c_2x_gpio_pin; /* 195 */
> > - u8 iboost_dp:4; /* 196 */
> > - u8 iboost_hdmi:4; /* 196 */
> > + /* u8 flags2; 195 */
> > + /* u8 dp_gpio_index; 195 */
> > + /* u16 dp_gpio_pin_num; 195 */
> > + /* u8 iboost_level; 196 */
> > } __packed;
> >
> > struct vbt {
> > @@ -155,7 +187,7 @@ static void virt_vbt_generation(struct vbt *v)
> > v->header.bdb_offset = offsetof(struct vbt, bdb_header);
> >
> > strcpy(&v->bdb_header.signature[0], "BIOS_DATA_BLOCK");
> > - v->bdb_header.version = 186; /* child_dev_size = 38 */
> > + v->bdb_header.version = 186; /* child_dev_size = 33 */
> > v->bdb_header.header_size = sizeof(v->bdb_header);
> >
> > v->bdb_header.bdb_size = sizeof(struct vbt) - sizeof(struct
> > vbt_header) @@ -169,18 +201,20 @@ static void
> > virt_vbt_generation(struct vbt *v)
> >
> > /* child device */
> > num_child = 4; /* each port has one child */
> > + v->general_definitions.child_dev_size =
> > + sizeof(struct efp_child_device_config);
> > v->general_definitions_header.id = BDB_GENERAL_DEFINITIONS;
> > /* size will include child devices */
> > v->general_definitions_header.size =
> > - sizeof(struct bdb_general_definitions) + num_child * DEV_SIZE;
> > - v->general_definitions.child_dev_size = DEV_SIZE;
> > + sizeof(struct bdb_general_definitions) +
> > + num_child * v->general_definitions.child_dev_size;
> >
> > /* portA */
> > v->child0.handle = DEVICE_TYPE_EFP1;
> > v->child0.device_type = DEVICE_TYPE_DP;
> > v->child0.dvo_port = DVO_PORT_DPA;
> > v->child0.aux_channel = DP_AUX_A;
> > - v->child0.dp_compat = true;
> > + v->child0.dp_support = true;
> > v->child0.integrated_encoder = true;
> >
> > /* portB */
> > @@ -188,7 +222,7 @@ static void virt_vbt_generation(struct vbt *v)
> > v->child1.device_type = DEVICE_TYPE_DP;
> > v->child1.dvo_port = DVO_PORT_DPB;
> > v->child1.aux_channel = DP_AUX_B;
> > - v->child1.dp_compat = true;
> > + v->child1.dp_support = true;
> > v->child1.integrated_encoder = true;
> >
> > /* portC */
> > @@ -196,7 +230,7 @@ static void virt_vbt_generation(struct vbt *v)
> > v->child2.device_type = DEVICE_TYPE_DP;
> > v->child2.dvo_port = DVO_PORT_DPC;
> > v->child2.aux_channel = DP_AUX_C;
> > - v->child2.dp_compat = true;
> > + v->child2.dp_support = true;
> > v->child2.integrated_encoder = true;
> >
> > /* portD */
> > @@ -204,7 +238,7 @@ static void virt_vbt_generation(struct vbt *v)
> > v->child3.device_type = DEVICE_TYPE_DP;
> > v->child3.dvo_port = DVO_PORT_DPD;
> > v->child3.aux_channel = DP_AUX_D;
> > - v->child3.dp_compat = true;
> > + v->child3.dp_support = true;
> > v->child3.integrated_encoder = true;
> >
> > /* driver features */
> > --
> > 1.9.1
> >
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
>
> --
> Open Source Technology Center, Intel ltd.
>
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
More information about the intel-gvt-dev
mailing list