From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by 2002:a17:906:cf90:b0:9bd:85f7:2662 with SMTP id um16csp357828ejb; Thu, 19 Oct 2023 01:30:49 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEqbiPmwU5CBlH7JlSIc3njsXVv5/uAz+a7SDomoiB2o4o0PIdybOvv+J1SXO73AepIkR3n X-Received: by 2002:a5b:788:0:b0:d78:1502:9330 with SMTP id b8-20020a5b0788000000b00d7815029330mr1671312ybq.7.1697704248927; Thu, 19 Oct 2023 01:30:48 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1697704248; cv=none; d=google.com; s=arc-20160816; b=BisiXx5YQ45BXIdWKtm821LkVbnqBMIglnHidW71BvjXxn+3UT/iGUd9UCS6yvtEAH sTeH1CMCVeMOzmK5IUDIslFnEwkRJ5R7mwR4SQRSFTZU8gvjpWXAfzMbRAaN4BSer7iE r8n1ujZZLMtjIxcBor6F3tAb9zd0FIjzTnDVeCjMn370cr76170jjQUUGJwR4F7M4ejn xm5c3BcUkSyTiaJo3Cy6vWIHez+pSb5SAWxh2mWgrkYzUqY8JY+KQFUta8QstM8DnDLD B78AMfgouDrh6BOtkJmCMF14wcBecjgKmjkfvwqJ5BO69bYpFZUTSqhvr1J14tMXgzdy vDiA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:user-agent :in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :dkim-signature; bh=F7NjyhzIF+bfkK8LJE42urWGupvmh3O/r3UDDNbVG+w=; fh=/coRUL1wBKUBvSHKckqPd31uAjBR6MoeU+vSbjwwHlw=; b=ylzGnWDeru6j26X9xr0dJyGF5oar9DF7uW2uxc+vQ7Wz2MxUMGn623QJrv5+kUO8+2 fwFgxSickrX3Lu1DP7KcaOvF9ms2YERCMPNfgdBIY73/eErnX3x4IiCzKDcfdUUoDY06 zRHaQUoH1Wbk2KAHX1Xiokdm6Ows6KD2wuoiDX5NjO/k7sznJ4LHLw6jQ8/DoPdt9p3A +HW8+gauQ6SuBNWjXJvL1sDD7tICotBnxtEQDX9mM8z55WAKtgJcs46qHqsd4Eg3Imga FeehIWhJbiBDMXFAVgvwWgzlvIvsS+oN3R+qhvfuByAEqBoGBH5fq6PW24UQdj+Pfp3h PbNg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=TSwuK6Ah; spf=pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id on13-20020a056214448d00b0065ae5abb042si1289572qvb.85.2023.10.19.01.30.48 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Thu, 19 Oct 2023 01:30:48 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=TSwuK6Ah; spf=pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qtOQ9-0005ce-S4; Thu, 19 Oct 2023 04:30:21 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qtOQ5-0005at-Px for qemu-arm@nongnu.org; Thu, 19 Oct 2023 04:30:18 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qtOQ1-0000m4-96 for qemu-arm@nongnu.org; Thu, 19 Oct 2023 04:30:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1697704212; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=F7NjyhzIF+bfkK8LJE42urWGupvmh3O/r3UDDNbVG+w=; b=TSwuK6Ah3Igy1PKGG0y1oS/hSL3qLJ1dnvP71JteVZSc+B2xFqv66M0zl/e2lo/0rUoaRK I/BcGE+JfaF1fXxBoIUlcg/Of+SR33R3gMeuSNYeiIcnRjpiAJcofXc6VYzm/wiUXnJXjd s5bZzh0rbF3O4AMgtsyRzSY5XBn1B9g= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-617-hM1zhTwTOYG9mndDlXvbaA-1; Thu, 19 Oct 2023 04:30:08 -0400 X-MC-Unique: hM1zhTwTOYG9mndDlXvbaA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 07EB03C0F448; Thu, 19 Oct 2023 08:30:08 +0000 (UTC) Received: from redhat.com (unknown [10.42.28.60]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 93B2D2166B26; Thu, 19 Oct 2023 08:30:05 +0000 (UTC) Date: Thu, 19 Oct 2023 09:30:03 +0100 From: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= To: Ninad Palsule Cc: qemu-devel@nongnu.org, clg@kaod.org, peter.maydell@linaro.org, andrew@aj.id.au, joel@jms.id.au, pbonzini@redhat.com, marcandre.lureau@redhat.com, thuth@redhat.com, philmd@linaro.org, lvivier@redhat.com, qemu-arm@nongnu.org Subject: Re: [PATCH v5 05/10] hw/fsi: IBM's On-chip Peripheral Bus Message-ID: References: <20231011151339.2782132-1-ninad@linux.ibm.com> <20231011151339.2782132-6-ninad@linux.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20231011151339.2782132-6-ninad@linux.ibm.com> User-Agent: Mutt/2.2.9 (2022-11-12) X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.6 Received-SPF: pass client-ip=170.10.129.124; envelope-from=berrange@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-arm@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= Errors-To: qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org Sender: qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org X-TUID: pzwGkvRcSJXt On Wed, Oct 11, 2023 at 10:13:34AM -0500, Ninad Palsule wrote: > This is a part of patchset where IBM's Flexible Service Interface is > introduced. > > The On-Chip Peripheral Bus (OPB): A low-speed bus typically found in > POWER processors. This now makes an appearance in the ASPEED SoC due > to tight integration of the FSI master IP with the OPB, mainly the > existence of an MMIO-mapping of the CFAM address straight onto a > sub-region of the OPB address space. > > Signed-off-by: Andrew Jeffery > Signed-off-by: Cédric Le Goater > Signed-off-by: Ninad Palsule > Reviewed-by: Joel Stanley > --- > v2: > - Incorporated review comment by Joel. > v5: > - Incorporated review comments by Cedric. > --- > include/hw/fsi/opb.h | 43 ++++++++++ > hw/fsi/fsi-master.c | 3 +- > hw/fsi/opb.c | 185 +++++++++++++++++++++++++++++++++++++++++++ > hw/fsi/Kconfig | 4 + > hw/fsi/meson.build | 1 + > hw/fsi/trace-events | 2 + > 6 files changed, 236 insertions(+), 2 deletions(-) > create mode 100644 include/hw/fsi/opb.h > create mode 100644 hw/fsi/opb.c > > diff --git a/include/hw/fsi/opb.h b/include/hw/fsi/opb.h > new file mode 100644 > index 0000000000..f8ce00383e > --- /dev/null > +++ b/include/hw/fsi/opb.h > @@ -0,0 +1,43 @@ > +/* > + * SPDX-License-Identifier: GPL-2.0-or-later > + * Copyright (C) 2023 IBM Corp. > + * > + * IBM On-Chip Peripheral Bus > + */ > +#ifndef FSI_OPB_H > +#define FSI_OPB_H > + > +#include "exec/memory.h" > +#include "hw/fsi/fsi-master.h" > + > +#define TYPE_OP_BUS "opb" > +OBJECT_DECLARE_SIMPLE_TYPE(OPBus, OP_BUS) > + > +typedef struct OPBus { > + /*< private >*/ > + BusState bus; > + > + /*< public >*/ > + MemoryRegion mr; > + AddressSpace as; > + > + /* Model OPB as dumb enough just to provide an address-space */ > + /* TODO: Maybe don't store device state in the bus? */ > + FSIMasterState fsi; > +} OPBus; > + > +typedef struct OPBusClass { > + BusClass parent_class; > +} OPBusClass; > + > +uint8_t opb_read8(OPBus *opb, hwaddr addr); > +uint16_t opb_read16(OPBus *opb, hwaddr addr); > +uint32_t opb_read32(OPBus *opb, hwaddr addr); > +void opb_write8(OPBus *opb, hwaddr addr, uint8_t data); > +void opb_write16(OPBus *opb, hwaddr addr, uint16_t data); > +void opb_write32(OPBus *opb, hwaddr addr, uint32_t data); > + > +void opb_fsi_master_address(OPBus *opb, hwaddr addr); > +void opb_opb2fsi_address(OPBus *opb, hwaddr addr); > + > +#endif /* FSI_OPB_H */ > diff --git a/hw/fsi/fsi-master.c b/hw/fsi/fsi-master.c > index 8f4ae641c7..ede1a58e98 100644 > --- a/hw/fsi/fsi-master.c > +++ b/hw/fsi/fsi-master.c > @@ -11,8 +11,7 @@ > #include "trace.h" > > #include "hw/fsi/fsi-master.h" > - > -#define TYPE_OP_BUS "opb" > +#include "hw/fsi/opb.h" > > #define TO_REG(x) ((x) >> 2) > > diff --git a/hw/fsi/opb.c b/hw/fsi/opb.c > new file mode 100644 > index 0000000000..7ffd7730f7 > --- /dev/null > +++ b/hw/fsi/opb.c > @@ -0,0 +1,185 @@ > +/* > + * SPDX-License-Identifier: GPL-2.0-or-later > + * Copyright (C) 2023 IBM Corp. > + * > + * IBM On-chip Peripheral Bus > + */ > + > +#include "qemu/osdep.h" > + > +#include "qapi/error.h" > +#include "qemu/log.h" > +#include "trace.h" > + > +#include "hw/fsi/opb.h" > + > +static MemTxResult opb_read(OPBus *opb, hwaddr addr, void *data, size_t len) > +{ > + MemTxResult tx; > + tx = address_space_read(&opb->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > + if (tx) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: OPB read failed: 0x%"HWADDR_PRIx" for %lu\n", > + __func__, addr, len); > + } > + return tx; > +} Use a tracepoint rather than an always-on log > + > +uint8_t opb_read8(OPBus *opb, hwaddr addr) > +{ > + uint8_t data = -1; > + > + (void)opb_read(opb, addr, &data, sizeof(data)); > + > + return data; > +} > + > +uint16_t opb_read16(OPBus *opb, hwaddr addr) > +{ > + uint16_t data = -1; > + > + (void)opb_read(opb, addr, &data, sizeof(data)); > + > + return data; > +} > + > +uint32_t opb_read32(OPBus *opb, hwaddr addr) > +{ > + uint32_t data = -1; > + > + (void)opb_read(opb, addr, &data, sizeof(data)); > + > + return data; > +} > + > +static void opb_write(OPBus *opb, hwaddr addr, void *data, size_t len) > +{ > + MemTxResult tx; > + tx = address_space_write(&opb->as, addr, MEMTXATTRS_UNSPECIFIED, data, > + len); > + if (tx) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: OPB write failed: %"HWADDR_PRIx" for %lu\n", > + __func__, addr, len); > + } > +} Again tracepoint > +static void opb_realize(BusState *bus, Error **errp) > +{ > + OPBus *opb = OP_BUS(bus); > + Error *err = NULL; > + > + memory_region_init_io(&opb->mr, OBJECT(opb), &opb_unimplemented_ops, opb, > + NULL, UINT32_MAX); > + address_space_init(&opb->as, &opb->mr, "opb"); > + > + object_property_set_bool(OBJECT(&opb->fsi), "realized", true, &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } Redundant local error object > + memory_region_add_subregion(&opb->mr, 0x80000000, &opb->fsi.iomem); > + > + /* OPB2FSI region */ > + /* > + * Avoid endianness issues by mapping each slave's memory region directly. > + * Manually bridging multiple address-spaces causes endian swapping > + * headaches as memory_region_dispatch_read() and > + * memory_region_dispatch_write() correct the endianness based on the > + * target machine endianness and not relative to the device endianness on > + * either side of the bridge. > + */ > + /* > + * XXX: This is a bit hairy and will need to be fixed when I sort out the > + * bus/slave relationship and any changes to the CFAM modelling (multiple > + * slaves, LBUS) > + */ > + memory_region_add_subregion(&opb->mr, 0xa0000000, &opb->fsi.opb2fsi); > +} > + > diff --git a/hw/fsi/trace-events b/hw/fsi/trace-events > index d7afef0460..ec9bab2fe8 100644 > --- a/hw/fsi/trace-events > +++ b/hw/fsi/trace-events > @@ -9,3 +9,5 @@ fsi_slave_read(uint64_t addr, uint32_t size) "@0x%" PRIx64 " size=%d" > fsi_slave_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size=%d value=0x%"PRIx64 > fsi_master_read(uint64_t addr, uint32_t size) "@0x%" PRIx64 " size=%d" > fsi_master_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size=%d value=0x%"PRIx64 > +opb_unimplemented_read(uint64_t addr, uint32_t size) "@0x%" PRIx64 " size=%d" > +opb_unimplemented_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size=%d value=0x%"PRIx64 Please consistently use an 'fsi_' prefix for all trace points, as it makes it possible to enable all tracing for this subsystem using a wildcard match With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|