On Mon, 17 Apr 2023 15:04:41 +0200 Thomas Huth <th...@redhat.com> wrote:
> On 17/04/2023 13.29, Michael S. Tsirkin wrote: > > On Mon, Apr 17, 2023 at 01:22:51PM +0200, Thomas Huth wrote: > >> On 11/04/2023 12.26, Peter Maydell wrote: > >>> On Wed, 8 Mar 2023 at 01:14, Michael S. Tsirkin <m...@redhat.com> wrote: > >>>> > >>>> From: Jonathan Cameron <jonathan.came...@huawei.com> > >>>> > >>>> The CXL r3.0 specification allows for there to be no HDM decoders on CXL > >>>> Host Bridges if they have only a single root port. Instead, all accesses > >>>> directed to the host bridge (as specified in CXL Fixed Memory Windows) > >>>> are assumed to be routed to the single root port. > >>> > >>> Hi; in issue https://gitlab.com/qemu-project/qemu/-/issues/1586 > >>> it's been pointed out that this commit causes an assertion > >>> failure during 'make check' if you configure with > >>> --enable-qom-cast-debug. You can repro by doing that and running: > >>> > >>> qemu-system-i386 -display none -machine q35,cxl=on -device > >>> pxb-cxl,bus=pcie.0 > ... > >>> The problem is here: > >>> > >>>> -static void pxb_dev_reset(DeviceState *dev) > >>>> +static void pxb_cxl_dev_reset(DeviceState *dev) > >>> > >>> This function is called from pxb_cxl_dev_realize(), > >>> which is the realize function for TYPE_PXB_CXL_DEVICE. > >>> That type's parent is TYPE_PCI_DEVICE. > >>> > >>>> { > >>>> CXLHost *cxl = PXB_CXL_DEV(dev)->cxl.cxl_host_bridge; > >>>> CXLComponentState *cxl_cstate = &cxl->cxl_cstate; > >>>> + PCIHostState *hb = PCI_HOST_BRIDGE(cxl); > >>>> uint32_t *reg_state = cxl_cstate->crb.cache_mem_registers; > >>>> uint32_t *write_msk = cxl_cstate->crb.cache_mem_regs_write_mask; > >>>> + int dsp_count = 0; > >>>> > >>>> cxl_component_register_init_common(reg_state, write_msk, > >>>> CXL2_ROOT_PORT); > >>>> - ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, > >>>> TARGET_COUNT, 8); > >>>> + /* > >>>> + * The CXL specification allows for host bridges with no HDM > >>>> decoders > >>>> + * if they only have a single root port. > >>>> + */ > >>>> + if (!PXB_DEV(dev)->hdm_for_passthrough) { > >>> > >>> However, here we try to cast the device pointer to PXB_DEV. > >>> That is not permitted because dev is not of type TYPE_PXB_DEVICE > >>> (either directly or as a parent class). So if you have the QOM > >>> debugging enabled then the attempt to cast causes an assertion > >>> failure. > >>> > >>>> + dsp_count = pcie_count_ds_ports(hb->bus); > >>>> + } > >>>> + /* Initial reset will have 0 dsp so wait until > 0 */ > >>>> + if (dsp_count == 1) { > >>>> + cxl->passthrough = true; > >>>> + /* Set Capability ID in header to NONE */ > >>>> + ARRAY_FIELD_DP32(reg_state, CXL_HDM_CAPABILITY_HEADER, ID, 0); > >>>> + } else { > >>>> + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, > >>>> TARGET_COUNT, > >>>> + 8); > >>>> + } > >>>> } > >>> > >>> What was the intention here with the type hierarchy? > >>> Should TYPE_PXB_CXL_DEVICE be a subclass of TYPE_PXB_DEVICE, > >>> or should the cxl-related functions not be trying to treat > >>> it as a PXB device ? > >> > >> Maybe we should simply revert the commit for the time being (once the 8.1 > >> cycle begins), 'til this has properly been solved, so we can enable > >> qom-debug by default again? > > > > Can you post a revert? > > Sure, done here now: > https://lore.kernel.org/qemu-devel/20230417130037.236747-2-th...@redhat.com/ > Sorry - I missed this until today. Indeed revert makes sense. I'll chase this down in the new cycle. Jonathan > Thomas >