From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7E8F2C48BCF for ; Sun, 13 Jun 2021 14:40:43 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DA6DF60FE5 for ; Sun, 13 Jun 2021 14:40:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DA6DF60FE5 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:46608 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lsRI2-0001TI-3d for qemu-devel@archiver.kernel.org; Sun, 13 Jun 2021 10:40:42 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:41850) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lsRH8-0000VV-Sw for qemu-devel@nongnu.org; Sun, 13 Jun 2021 10:39:46 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:33444) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lsRH2-0001Mm-Ae for qemu-devel@nongnu.org; Sun, 13 Jun 2021 10:39:46 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623595179; h=from:from: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=7pR9cTOQqmR5wHtO9q1C7ZXH7wUrAQlNjaFGUUli9KE=; b=SsEFV0DqYWI8wVe61NQzXnJMUovK05GLJL61M4KL85MgBy2d2+Dojf8G9C0sjKuiISJ+H7 8cJ/Af6vjm6jSElk9e4eNySyhmJUQGF5r45CR/d7iauS8FfHhyCGaGnNpsqgsFdcG6ijSw 7LMc2WC+q7pq6vGaoGlnXghvkF7h90s= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-506-f7Ag3HoGOLydW38pM2Qxew-1; Sun, 13 Jun 2021 10:39:38 -0400 X-MC-Unique: f7Ag3HoGOLydW38pM2Qxew-1 Received: by mail-wr1-f72.google.com with SMTP id h10-20020a5d688a0000b0290119c2ce2499so5574627wru.19 for ; Sun, 13 Jun 2021 07:39:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=7pR9cTOQqmR5wHtO9q1C7ZXH7wUrAQlNjaFGUUli9KE=; b=i656s0Sov5YrlJC3+kPcAdCt5Z9r3Uoi/hNeZrygxTZ+/r+fJOBug1sGk5TyBP2dL+ yyGrAS52QT3mQbDg1oUYc6gxh6QV7tYb/XBTjNO/bNWLUri3c8on4xJLzBE+wXOy5AVh W2ZejawfWaEmgFTFQIA2VULJRxGoeUs6YhBG223uiUF2RwATCG5td45t+wWatARJjYF4 MHhBqKPVWhMxUje/BjxahKu2IA75eWm4Rmi8mRmgJBZo78DgDW754bWED8j1fQX8y330 Fm1rU9HTVTvIEq5TUnIZFcSvqLw/+1l5X00i0II6y3PNtTkz+vqHddLXqh3QN4tNVwj1 9hqw== X-Gm-Message-State: AOAM530ZPkifkcptwRTDp7KvUMpWpt2Gyu2UwKUVjwJRf3Dluq1IKfW6 e7I4D87S1sBrF3nsqhmKuAusWA17NGB7TUgPY1vBQII8j0+cOgH31Vadto78V5PPeRbIETpStbx 4lCevU2QriRlqwPBs7OTNTIP7Q2Uv1Op/qOHfqTN3DG7STnxSJxhffpN1lIRDbxi2 X-Received: by 2002:a5d:47ce:: with SMTP id o14mr14002610wrc.273.1623595177073; Sun, 13 Jun 2021 07:39:37 -0700 (PDT) X-Google-Smtp-Source: ABdhPJySgy5AtNLJABFo+CWRxdca6GTuImycfuRsU8ny8lPyKNIHnEpCCvXnqNJnMcKSs0CaaX8GBA== X-Received: by 2002:a5d:47ce:: with SMTP id o14mr14002594wrc.273.1623595176839; Sun, 13 Jun 2021 07:39:36 -0700 (PDT) Received: from [192.168.43.95] ([37.170.26.89]) by smtp.gmail.com with ESMTPSA id l20sm10596810wmq.3.2021.06.13.07.39.34 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 13 Jun 2021 07:39:36 -0700 (PDT) Subject: Re: [PATCH v4 3/8] hw/intc: GICv3 ITS command queue framework To: Shashi Mallela , peter.maydell@linaro.org, leif@nuviainc.com, rad@semihalf.com References: <20210602180042.111347-1-shashi.mallela@linaro.org> <20210602180042.111347-4-shashi.mallela@linaro.org> From: Eric Auger Message-ID: Date: Sun, 13 Jun 2021 16:39:33 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <20210602180042.111347-4-shashi.mallela@linaro.org> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eauger@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=170.10.133.124; envelope-from=eauger@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.199, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, NICE_REPLY_A=-0.144, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_HELO_NONE=0.001, T_SPF_TEMPERROR=0.01 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-arm@nongnu.org, qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Hi, On 6/2/21 8:00 PM, Shashi Mallela wrote: > Added functionality to trigger ITS command queue processing on > write to CWRITE register and process each command queue entry to > identify the command type and handle commands like MAPD,MAPC,SYNC. > > Signed-off-by: Shashi Mallela > --- > hw/intc/arm_gicv3_its.c | 295 +++++++++++++++++++++++++++++++++++++++ > hw/intc/gicv3_internal.h | 37 +++++ > 2 files changed, 332 insertions(+) > > diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c > index af60f19c98..6551c577b3 100644 > --- a/hw/intc/arm_gicv3_its.c > +++ b/hw/intc/arm_gicv3_its.c > @@ -49,6 +49,295 @@ static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz) > return result; > } > > +static MemTxResult update_cte(GICv3ITSState *s, uint16_t icid, bool valid, > + uint64_t rdbase) > +{ > + AddressSpace *as = &s->gicv3->dma_as; > + uint64_t value; > + uint64_t l2t_addr; > + bool valid_l2t; > + uint32_t l2t_id; > + uint32_t max_l2_entries; > + uint64_t cte = 0; > + MemTxResult res = MEMTX_OK; > + > + if (!s->ct.valid) { > + return res; > + } > + > + if (valid) { > + /* add mapping entry to collection table */ > + cte = (valid & VALID_MASK) | > + ((rdbase & RDBASE_PROCNUM_MASK) << 1ULL); > + } > + > + /* > + * The specification defines the format of level 1 entries of a > + * 2-level table, but the format of level 2 entries and the format > + * of flat-mapped tables is IMPDEF. > + */ > + if (s->ct.indirect) { > + l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE); > + > + value = address_space_ldq_le(as, > + s->ct.base_addr + > + (l2t_id * L1TABLE_ENTRY_SIZE), > + MEMTXATTRS_UNSPECIFIED, &res); > + > + if (res != MEMTX_OK) { > + return res; > + } > + > + valid_l2t = (value >> VALID_SHIFT) & VALID_MASK; > + > + if (valid_l2t) { > + max_l2_entries = s->ct.page_sz / s->ct.entry_sz; > + > + l2t_addr = value & ((1ULL << 51) - 1); > + > + address_space_stq_le(as, l2t_addr + > + ((icid % max_l2_entries) * GITS_CTE_SIZE), > + cte, MEMTXATTRS_UNSPECIFIED, &res); > + } > + } else { > + /* Flat level table */ > + address_space_stq_le(as, s->ct.base_addr + (icid * GITS_CTE_SIZE), > + cte, MEMTXATTRS_UNSPECIFIED, &res); > + } > + return res; > +} Looking at your DevTableDesc and CollTableDesc types again, they are basically the same except max_devids/max_collids. You may use a single one and it may be possible to define helpers to access an entry in the DT or CT. update_cte/update_dte looks quite similar if you compute the cte and dte externally and pass a pointer to the associated TableDesc? Thanks Eric > + > +static MemTxResult process_mapc(GICv3ITSState *s, uint32_t offset) > +{ > + AddressSpace *as = &s->gicv3->dma_as; > + uint16_t icid; > + uint64_t rdbase; > + bool valid; > + MemTxResult res = MEMTX_OK; > + uint64_t value; > + > + offset += NUM_BYTES_IN_DW; > + offset += NUM_BYTES_IN_DW; > + > + value = address_space_ldq_le(as, s->cq.base_addr + offset, > + MEMTXATTRS_UNSPECIFIED, &res); > + > + if (res != MEMTX_OK) { > + return res; > + } > + > + icid = value & ICID_MASK; > + > + rdbase = (value >> R_MAPC_RDBASE_SHIFT) & RDBASE_PROCNUM_MASK; > + > + valid = (value >> VALID_SHIFT) & VALID_MASK; > + > + if ((icid > s->ct.max_collids) || (rdbase > s->gicv3->num_cpu)) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "ITS MAPC: invalid collection table attributes " > + "icid %d rdbase %lu\n", icid, rdbase); > + /* > + * in this implementation,in case of error > + * we ignore this command and move onto the next > + * command in the queue > + */ > + } else { > + res = update_cte(s, icid, valid, rdbase); > + } > + > + return res; > +} > + > +static MemTxResult update_dte(GICv3ITSState *s, uint32_t devid, bool valid, > + uint8_t size, uint64_t itt_addr) > +{ > + AddressSpace *as = &s->gicv3->dma_as; > + uint64_t value; > + uint64_t l2t_addr; > + bool valid_l2t; > + uint32_t l2t_id; > + uint32_t max_l2_entries; > + uint64_t dte = 0; > + MemTxResult res = MEMTX_OK; > + > + if (s->dt.valid) { > + if (valid) { > + /* add mapping entry to device table */ > + dte = (valid & VALID_MASK) | > + ((size & SIZE_MASK) << 1U) | > + ((itt_addr & ITTADDR_MASK) << 6ULL); > + } > + } else { > + return res; > + } > + > + /* > + * The specification defines the format of level 1 entries of a > + * 2-level table, but the format of level 2 entries and the format > + * of flat-mapped tables is IMPDEF. > + */ > + if (s->dt.indirect) { > + l2t_id = devid / (s->dt.page_sz / L1TABLE_ENTRY_SIZE); > + > + value = address_space_ldq_le(as, > + s->dt.base_addr + > + (l2t_id * L1TABLE_ENTRY_SIZE), > + MEMTXATTRS_UNSPECIFIED, &res); > + > + if (res != MEMTX_OK) { > + return res; > + } > + > + valid_l2t = (value >> VALID_SHIFT) & VALID_MASK; > + > + if (valid_l2t) { > + max_l2_entries = s->dt.page_sz / s->dt.entry_sz; > + > + l2t_addr = value & ((1ULL << 51) - 1); > + > + address_space_stq_le(as, l2t_addr + > + ((devid % max_l2_entries) * GITS_DTE_SIZE), > + dte, MEMTXATTRS_UNSPECIFIED, &res); > + } > + } else { > + /* Flat level table */ > + address_space_stq_le(as, s->dt.base_addr + (devid * GITS_DTE_SIZE), > + dte, MEMTXATTRS_UNSPECIFIED, &res); > + } > + return res; > +} > + > +static MemTxResult process_mapd(GICv3ITSState *s, uint64_t value, > + uint32_t offset) > +{ > + AddressSpace *as = &s->gicv3->dma_as; > + uint32_t devid; > + uint8_t size; > + uint64_t itt_addr; > + bool valid; > + MemTxResult res = MEMTX_OK; > + > + devid = (value >> DEVID_SHIFT) & DEVID_MASK; > + > + offset += NUM_BYTES_IN_DW; > + value = address_space_ldq_le(as, s->cq.base_addr + offset, > + MEMTXATTRS_UNSPECIFIED, &res); > + > + if (res != MEMTX_OK) { > + return res; > + } > + > + size = (value & SIZE_MASK); > + > + offset += NUM_BYTES_IN_DW; > + value = address_space_ldq_le(as, s->cq.base_addr + offset, > + MEMTXATTRS_UNSPECIFIED, &res); > + > + if (res != MEMTX_OK) { > + return res; > + } > + > + itt_addr = (value >> ITTADDR_SHIFT) & ITTADDR_MASK; > + > + valid = (value >> VALID_SHIFT) & VALID_MASK; > + > + if ((devid > s->dt.max_devids) || > + (size > FIELD_EX64(s->typer, GITS_TYPER, IDBITS))) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "ITS MAPD: invalid device table attributes " > + "devid %d or size %d\n", devid, size); > + /* > + * in this implementation, in case of error > + * we ignore this command and move onto the next > + * command in the queue > + */ > + } else { > + res = update_dte(s, devid, valid, size, itt_addr); > + } > + > + return res; > +} > + > +/* > + * Current implementation blocks until all > + * commands are processed > + */ > +static void process_cmdq(GICv3ITSState *s) > +{ > + uint32_t wr_offset = 0; > + uint32_t rd_offset = 0; > + uint32_t cq_offset = 0; > + uint64_t data; > + AddressSpace *as = &s->gicv3->dma_as; > + MemTxResult res = MEMTX_OK; > + uint8_t cmd; > + > + if (!(s->ctlr & ITS_CTLR_ENABLED)) { > + return; > + } > + > + wr_offset = FIELD_EX64(s->cwriter, GITS_CWRITER, OFFSET); > + > + if (wr_offset > s->cq.max_entries) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: invalid write offset " > + "%d\n", __func__, wr_offset); > + return; > + } > + > + rd_offset = FIELD_EX64(s->creadr, GITS_CREADR, OFFSET); > + > + while (wr_offset != rd_offset) { > + cq_offset = (rd_offset * GITS_CMDQ_ENTRY_SIZE); > + data = address_space_ldq_le(as, s->cq.base_addr + cq_offset, > + MEMTXATTRS_UNSPECIFIED, &res); > + cmd = (data & CMD_MASK); > + > + switch (cmd) { > + case GITS_CMD_INT: > + break; > + case GITS_CMD_CLEAR: > + break; > + case GITS_CMD_SYNC: > + /* > + * Current implementation makes a blocking synchronous call > + * for every command issued earlier, hence the internal state > + * is already consistent by the time SYNC command is executed. > + * Hence no further processing is required for SYNC command. > + */ > + break; > + case GITS_CMD_MAPD: > + res = process_mapd(s, data, cq_offset); > + break; > + case GITS_CMD_MAPC: > + res = process_mapc(s, cq_offset); > + break; > + case GITS_CMD_MAPTI: > + break; > + case GITS_CMD_MAPI: > + break; > + case GITS_CMD_DISCARD: > + break; > + default: > + break; > + } > + if (res == MEMTX_OK) { > + rd_offset++; > + rd_offset %= s->cq.max_entries; > + s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, OFFSET, rd_offset); > + } else { > + /* > + * in this implementation,in case of dma read/write error > + * we stall the command processing > + */ > + s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, STALLED, 1); > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: %x cmd processing failed!!\n", __func__, cmd); > + break; > + } > + } > +} > + > static void extract_table_params(GICv3ITSState *s) > { > uint16_t num_pages = 0; > @@ -226,6 +515,9 @@ static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset, > case GITS_CWRITER: > s->cwriter = deposit64(s->cwriter, 0, 32, > (value & ~R_GITS_CWRITER_RETRY_MASK)); > + if (s->cwriter != s->creadr) { > + process_cmdq(s); > + } > break; > case GITS_CWRITER + 4: > s->cwriter = deposit64(s->cwriter, 32, 32, > @@ -346,6 +638,9 @@ static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset, > break; > case GITS_CWRITER: > s->cwriter = value & ~R_GITS_CWRITER_RETRY_MASK; > + if (s->cwriter != s->creadr) { > + process_cmdq(s); > + } > break; > case GITS_CREADR: > case GITS_TYPER: > diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h > index d6aaa94e4c..0932a30560 100644 > --- a/hw/intc/gicv3_internal.h > +++ b/hw/intc/gicv3_internal.h > @@ -253,6 +253,9 @@ FIELD(GITS_CBASER, OUTERCACHE, 53, 3) > FIELD(GITS_CBASER, INNERCACHE, 59, 3) > FIELD(GITS_CBASER, VALID, 63, 1) > > +FIELD(GITS_CREADR, STALLED, 0, 1) > +FIELD(GITS_CREADR, OFFSET, 5, 15) > + > FIELD(GITS_CWRITER, RETRY, 0, 1) > FIELD(GITS_CWRITER, OFFSET, 5, 15) > > @@ -289,6 +292,40 @@ FIELD(GITS_TYPER, CIL, 36, 1) > #define L1TABLE_ENTRY_SIZE 8 > > #define GITS_CMDQ_ENTRY_SIZE 32 > +#define NUM_BYTES_IN_DW 8 > + > +#define CMD_MASK 0xff > + > +/* ITS Commands */ > +#define GITS_CMD_CLEAR 0x04 > +#define GITS_CMD_DISCARD 0x0F > +#define GITS_CMD_INT 0x03 > +#define GITS_CMD_MAPC 0x09 > +#define GITS_CMD_MAPD 0x08 > +#define GITS_CMD_MAPI 0x0B > +#define GITS_CMD_MAPTI 0x0A > +#define GITS_CMD_SYNC 0x05 > + > +/* MAPC command fields */ > +#define ICID_LENGTH 16 > +#define ICID_MASK ((1U << ICID_LENGTH) - 1) > +FIELD(MAPC, RDBASE, 16, 32) > + > +#define RDBASE_PROCNUM_LENGTH 16 > +#define RDBASE_PROCNUM_MASK ((1ULL << RDBASE_PROCNUM_LENGTH) - 1) > + > +#define DEVID_SHIFT 32 > +#define DEVID_LENGTH 32 > +#define DEVID_MASK ((1ULL << DEVID_LENGTH) - 1) > + > +/* MAPD command fields */ > +#define ITTADDR_LENGTH 44 > +#define ITTADDR_SHIFT 8 > +#define ITTADDR_MASK ((1ULL << ITTADDR_LENGTH) - 1) > +#define SIZE_MASK 0x1f > + > +#define VALID_SHIFT 63 > +#define VALID_MASK 1ULL > > /** > * Default features advertised by this version of ITS >