From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vijay Kilari Subject: Re: [PATCH v5 09/22] xen/arm: ITS: Export ITS info to Virtual ITS Date: Fri, 31 Jul 2015 12:31:43 +0530 Message-ID: References: <1437995524-19772-1-git-send-email-vijay.kilari@gmail.com> <1437995524-19772-10-git-send-email-vijay.kilari@gmail.com> <55B7C693.3080805@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55B7C693.3080805@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: Ian Campbell , Stefano Stabellini , Prasun Kapoor , manish.jaggi@caviumnetworks.com, Tim Deegan , "xen-devel@lists.xen.org" , Stefano Stabellini , Vijaya Kumar K List-Id: xen-devel@lists.xenproject.org On Tue, Jul 28, 2015 at 11:44 PM, Julien Grall wrote: > Hi Vijay, > > On 27/07/15 12:11, vijay.kilari@gmail.com wrote: >> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c >> index aa4d3c5..e16fa03 100644 >> --- a/xen/arch/arm/gic-v3-its.c >> +++ b/xen/arch/arm/gic-v3-its.c >> @@ -94,6 +94,7 @@ static LIST_HEAD(its_nodes); >> static DEFINE_SPINLOCK(its_lock); >> static struct rdist_prop *gic_rdists; >> static struct rb_root rb_its_dev; >> +static struct gic_its_info its_data; >> static DEFINE_SPINLOCK(rb_its_dev_lock); >> >> #define gic_data_rdist() (this_cpu(rdist)) >> @@ -942,6 +943,8 @@ static int its_probe(struct dt_device_node *node) >> its->phys_size = its_size; >> typer = readl_relaxed(its_base + GITS_TYPER); >> its->ite_size = ((typer >> 4) & 0xf) + 1; >> + its_data.eventid_bits = GITS_TYPER_IDBITS(typer); >> + its_data.dev_bits = GITS_TYPER_DEVBITS(typer); >> >> its->cmd_base = xzalloc_bytes(ITS_CMD_QUEUE_SZ); >> if ( !its->cmd_base ) >> @@ -1032,7 +1035,10 @@ int its_cpu_init(void) >> >> int __init its_init(struct rdist_prop *rdists) >> { >> + struct its_node *its; >> + struct its_node_info *info; >> struct dt_device_node *np = NULL; >> + uint32_t i, nr_its = 0; >> >> static const struct dt_device_match its_device_ids[] __initconst = >> { >> @@ -1042,7 +1048,10 @@ int __init its_init(struct rdist_prop *rdists) >> >> for (np = dt_find_matching_node(NULL, its_device_ids); np; >> np = dt_find_matching_node(np, its_device_ids)) >> - its_probe(np); >> + { >> + if ( !its_probe(np) ) >> + nr_its++; >> + } >> if ( list_empty(&its_nodes) ) >> { >> @@ -1050,6 +1059,22 @@ int __init its_init(struct rdist_prop *rdists) >> return -ENXIO; >> } >> >> + info = xzalloc_array(struct its_node_info, nr_its); >> + if ( !info ) >> + return -ENOMEM; >> + >> + i = 0; >> + list_for_each_entry(its, &its_nodes, entry) >> + { >> + info[i].phys_base = its->phys_base; >> + info[i].phys_size = its->phys_size; >> + i++; >> + } >> + >> + its_data.nr_its = nr_its; >> + its_data.its_hw = info; >> + vits_setup_hw(&its_data); >> + > > The vITS will only expose 1 ITS to dom0. So there is no need to try to > allocate memory in order to expose all the ITS. > > We can extend vits_setup_hw if we ever want to support more ITS in the > future. > > Furthermore I'd like to see all the ITS data pass one by one rather than > in a structure. This would help to catch error if we ever extend the > structure. > > Something like: > > void vits_hw_setup(paddr_t its0_base, unsigned long its0_size, > uint32_t eventID_bits, uint32_t devID_bits); > Passing every value as argument makes this function take too many arguments. Validation can be done member variable to catch errors. >> gic_rdists = rdists; >> its_lpi_init(rdists->id_bits); >> its_alloc_lpi_tables(); >> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c >> index dfa3435..3a003d4 100644 >> --- a/xen/arch/arm/vgic-v3-its.c >> +++ b/xen/arch/arm/vgic-v3-its.c >> @@ -51,6 +51,15 @@ static void dump_cmd(its_cmd_block *cmd) >> static void dump_cmd(its_cmd_block *cmd) { do {} while ( 0 ); } >> #endif >> >> +static struct { >> + struct gic_its_info *info; >> +} vits_hw; > > Please add a boolean valid in the structure. It would help to know if > the vITS can be used or not later. > > Regards, > > -- > Julien Grall