Skip to contentSkip to conversation

ALPHA: TITAN - check for allocated memory

PATCH In review Cyrill Gorcunov 2 participants 3 messages May 16, 2007 – May 17, 2007 Alpha port

lore
Share & tools
lore

Cyrill Gorcunov

author
#0 lore raw

PATCH 1/1 ALPHA: TITAN - check for allocated memory

This patch adds checking for allocated memory which is used to hold AGP info. Also some whitespace cleanup.

Signed-off-by Cyrill Gorcunov <[email protected]>

---

 arch/alpha/kernel/core_titan.c |   99 +++++++++++++++++++++-------------------
 1 files changed, 52 insertions(+), 47 deletions(-)
arch/alpha/kernel/core_titan.c +52 -47
index 3662fef..419dbc8 100644
@@ -46,7 +46,7 @@ struct
 # define DBG_CFG(args)
 #endif
 
-
+
 /*
  * Routines to access TIG registers.
  */

@@ -56,21 +56,21 @@ mk_tig_addr(int offset)
 	return (volatile unsigned long *)(TITAN_TIG_SPACE + (offset << 6));
 }
 
-static inline u8 
+static inline u8
 titan_read_tig(int offset, u8 value)
 {
 	volatile unsigned long *tig_addr = mk_tig_addr(offset);
 	return (u8)(*tig_addr & 0xff);
 }
 
-static inline void 
+static inline void
 titan_write_tig(int offset, u8 value)
 {
 	volatile unsigned long *tig_addr = mk_tig_addr(offset);
 	*tig_addr = (unsigned long)value;
 }
 
-
+
 /*
  * Given a bus, device, and function number, compute resulting
  * configuration space address

@@ -84,7 +84,7 @@ titan_write_tig(int offset, u8 value)
  *
  * Type 1:
  *
- *  3 3|3 3 2 2|2 2 2 2|2 2 2 2|1 1 1 1|1 1 1 1|1 1 
+ *  3 3|3 3 2 2|2 2 2 2|2 2 2 2|1 1 1 1|1 1 1 1|1 1
  *  3 2|1 0 9 8|7 6 5 4|3 2 1 0|9 8 7 6|5 4 3 2|1 0 9 8|7 6 5 4|3 2 1 0
  * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  * | | | | | | | | | | |B|B|B|B|B|B|B|B|D|D|D|D|D|F|F|F|R|R|R|R|R|R|0|1|

@@ -95,11 +95,11 @@ titan_write_tig(int offset, u8 value)
  *	15:11	Device number (5 bits)
  *	10:8	function number
  *	 7:2	register number
- *  
+ *
  * Notes:
- *	The function number selects which function of a multi-function device 
+ *	The function number selects which function of a multi-function device
  *	(e.g., SCSI and Ethernet).
- * 
+ *
  *	The register selects a DWORD (32 bit) register offset.  Hence it
  *	doesn't get shifted by 2 bits as we want to "drop" the bottom two
  *	bits.

@@ -123,7 +123,7 @@ mk_conf_addr(struct pci_bus *pbus, unsigned int device_fn, int where,
 
         addr = (bus << 16) | (device_fn << 8) | where;
 	addr |= hose->config_space_base;
-		
+
 	*pci_addr = addr;
 	DBG_CFG(("mk_conf_addr: returning pci_addr 0x%lx\n", addr));
 	return 0;

@@ -154,7 +154,7 @@ titan_read_config(struct pci_bus *bus, unsigned int devfn, int where,
 	return PCIBIOS_SUCCESSFUL;
 }
 
-static int 
+static int
 titan_write_config(struct pci_bus *bus, unsigned int devfn, int where,
 		   int size, u32 value)
 {

@@ -185,17 +185,17 @@ titan_write_config(struct pci_bus *bus, unsigned int devfn, int where,
 	return PCIBIOS_SUCCESSFUL;
 }
 
-struct pci_ops titan_pci_ops = 
+struct pci_ops titan_pci_ops =
 {
 	.read =		titan_read_config,
 	.write =	titan_write_config,
 };
 
-
+
 void
 titan_pci_tbi(struct pci_controller *hose, dma_addr_t start, dma_addr_t end)
 {
-	titan_pachip *pachip = 
+	titan_pachip *pachip =
 	  (hose->index & 1) ? TITAN_pachip1 : TITAN_pachip0;
 	titan_pachip_port *port;
 	volatile unsigned long *csr;

@@ -203,11 +203,11 @@ titan_pci_tbi(struct pci_controller *hose, dma_addr_t start, dma_addr_t end)
 
 	/* Get the right hose.  */
 	port = &pachip->g_port;
-	if (hose->index & 2) 
+	if (hose->index & 2)
 		port = &pachip->a_port;
 
 	/* We can invalidate up to 8 tlb entries in a go.  The flush
-	   matches against <31:16> in the pci address.  
+	   matches against <31:16> in the pci address.
 	   Note that gtlbi* and atlbi* are in the same place in the g_port
 	   and a_port, respectively, so the g_port offset can be used
 	   even if hose is an a_port */

@@ -215,7 +215,7 @@ titan_pci_tbi(struct pci_controller *hose, dma_addr_t start, dma_addr_t end)
 	if (((start ^ end) & 0xffff0000) == 0)
 		csr = &port->port_specific.g.gtlbiv.csr;
 
-	/* For TBIA, it doesn't matter what value we write.  For TBI, 
+	/* For TBIA, it doesn't matter what value we write.  For TBI,
 	   it's the shifted tag bits.  */
 	value = (start & 0xffff0000) >> 12;
 

@@ -249,11 +249,11 @@ titan_init_one_pachip_port(titan_pachip_port *port, int index)
 	hose->mem_space = alloc_resource();
 
 	/*
-	 * This is for userland consumption.  The 40-bit PIO bias that we 
-	 * use in the kernel through KSEG doesn't work in the page table 
+	 * This is for userland consumption.  The 40-bit PIO bias that we
+	 * use in the kernel through KSEG doesn't work in the page table
 	 * based user mappings. (43-bit KSEG sign extends the physical
 	 * address from bit 40 to hit the I/O bit - mapped addresses don't).
-	 * So make sure we get the 43-bit PIO bias.  
+	 * So make sure we get the 43-bit PIO bias.
 	 */
 	hose->sparse_mem_base = 0;
 	hose->sparse_io_base = 0;

@@ -281,7 +281,7 @@ titan_init_one_pachip_port(titan_pachip_port *port, int index)
 		printk(KERN_ERR "Failed to request MEM on hose %d\n", index);
 
 	/*
-	 * Save the existing PCI window translations.  SRM will 
+	 * Save the existing PCI window translations.  SRM will
 	 * need them when we go to reboot.
 	 */
 	saved_config[index].wsba[0] = port->wsba[0].csr;

@@ -335,7 +335,7 @@ titan_init_one_pachip_port(titan_pachip_port *port, int index)
 	/*
 	 * If it's an AGP port, initialize agplastwr.
 	 */
-	if (titan_query_agp(port)) 
+	if (titan_query_agp(port))
 		port->port_specific.a.agplastwr.csr = __direct_map_base;
 
 	titan_pci_tbi(hose, 0, -1);

@@ -457,7 +457,7 @@ titan_kill_arch(int mode)
 	titan_kill_pachips(TITAN_pachip0, TITAN_pachip1);
 }
 
-
+
 /*
  * IO map support.
  */

@@ -468,7 +468,7 @@ titan_ioremap(unsigned long addr, unsigned long size)
 	int h = (addr & TITAN_HOSE_MASK) >> TITAN_HOSE_SHIFT;
 	unsigned long baddr = addr & ~TITAN_HOSE_MASK;
 	unsigned long last = baddr + size - 1;
-	struct pci_controller *hose;	
+	struct pci_controller *hose;
 	struct vm_struct *area;
 	unsigned long vaddr;
 	unsigned long *ptes;

@@ -476,7 +476,7 @@ titan_ioremap(unsigned long addr, unsigned long size)
 
 	/*
 	 * Adjust the addr.
-	 */ 
+	 */
 #ifdef CONFIG_VGA_HOSE
 	if (pci_vga_hose && __titan_is_mem_vga(addr)) {
 		h = pci_vga_hose->index;

@@ -496,13 +496,13 @@ titan_ioremap(unsigned long addr, unsigned long size)
 	/*
 	 * Is it direct-mapped?
 	 */
-	if ((baddr >= __direct_map_base) && 
+	if ((baddr >= __direct_map_base) &&
 	    ((baddr + size - 1) < __direct_map_base + __direct_map_size)) {
 		vaddr = addr - __direct_map_base + TITAN_MEM_BIAS;
 		return (void __iomem *) vaddr;
 	}
 
-	/* 
+	/*
 	 * Check the scatter-gather arena.
 	 */
 	if (hose->sg_pci &&

@@ -525,8 +525,8 @@ titan_ioremap(unsigned long addr, unsigned long size)
 			return NULL;
 
 		ptes = hose->sg_pci->ptes;
-		for (vaddr = (unsigned long)area->addr; 
-		    baddr <= last; 
+		for (vaddr = (unsigned long)area->addr;
+		    baddr <= last;
 		    baddr += PAGE_SIZE, vaddr += PAGE_SIZE) {
 			pfn = ptes[baddr >> PAGE_SHIFT];
 			if (!(pfn & 1)) {

@@ -535,9 +535,9 @@ titan_ioremap(unsigned long addr, unsigned long size)
 				return NULL;
 			}
 			pfn >>= 1;	/* make it a true pfn */
-			
+
 			if (__alpha_remap_area_pages(vaddr,
-						     pfn << PAGE_SHIFT, 
+						     pfn << PAGE_SHIFT,
 						     PAGE_SIZE, 0)) {
 				printk("FAILED to map...\n");
 				vfree(area->addr);

@@ -559,7 +559,7 @@ titan_iounmap(volatile void __iomem *xaddr)
 {
 	unsigned long addr = (unsigned long) xaddr;
 	if (addr >= VMALLOC_START)
-		vfree((void *)(PAGE_MASK & addr)); 
+		vfree((void *)(PAGE_MASK & addr));
 }
 
 int

@@ -578,7 +578,7 @@ EXPORT_SYMBOL(titan_ioremap);
 EXPORT_SYMBOL(titan_iounmap);
 EXPORT_SYMBOL(titan_is_mmio);
 #endif
-
+
 /*
  * AGP GART Support.
  */

@@ -615,7 +615,7 @@ titan_agp_setup(alpha_agp_info *agp)
 		return -ENOMEM;
 	}
 
-	agp->aperture.bus_base = 
+	agp->aperture.bus_base =
 		aper->arena->dma_base + aper->pg_start * PAGE_SIZE;
 	agp->aperture.size = aper->pg_count * PAGE_SIZE;
 	agp->aperture.sysdata = aper;

@@ -631,10 +631,10 @@ titan_agp_cleanup(alpha_agp_info *agp)
 
 	status = iommu_release(aper->arena, aper->pg_start, aper->pg_count);
 	if (status == -EBUSY) {
-		printk(KERN_WARNING 
+		printk(KERN_WARNING
 		       "Attempted to release bound AGP memory - unbinding\n");
 		iommu_unbind(aper->arena, aper->pg_start, aper->pg_count);
-		status = iommu_release(aper->arena, aper->pg_start, 
+		status = iommu_release(aper->arena, aper->pg_start,
 				       aper->pg_count);
 	}
 	if (status < 0)

@@ -656,13 +656,13 @@ titan_agp_configure(alpha_agp_info *agp)
 
 	/* AGP Rate? */
 	pctl.pctl_r_bits.apctl_v_agp_rate = 0;		/* 1x */
-	if (agp->mode.bits.rate & 2) 
+	if (agp->mode.bits.rate & 2)
 		pctl.pctl_r_bits.apctl_v_agp_rate = 1;	/* 2x */
 #if 0
-	if (agp->mode.bits.rate & 4) 
+	if (agp->mode.bits.rate & 4)
 		pctl.pctl_r_bits.apctl_v_agp_rate = 2;	/* 4x */
 #endif
-	
+
 	/* RQ Depth? */
 	pctl.pctl_r_bits.apctl_v_agp_hp_rd = 2;
 	pctl.pctl_r_bits.apctl_v_agp_lp_rd = 7;

@@ -673,28 +673,28 @@ titan_agp_configure(alpha_agp_info *agp)
 	pctl.pctl_r_bits.apctl_v_agp_en = agp->mode.bits.enable;
 
 	/* Tell the user.  */
-	printk("Enabling AGP: %dX%s\n", 
+	printk("Enabling AGP: %dX%s\n",
 	       1 << pctl.pctl_r_bits.apctl_v_agp_rate,
 	       pctl.pctl_r_bits.apctl_v_agp_sba_en ? " - SBA" : "");
-	       
+
 	/* Write it.  */
 	port->pctl.csr = pctl.pctl_q_whole;
-	
+
 	/* And wait at least 5000 66MHz cycles (per Titan spec).  */
 	udelay(100);
 
 	return 0;
 }
 
-static int 
+static int
 titan_agp_bind_memory(alpha_agp_info *agp, off_t pg_start, struct agp_memory *mem)
 {
 	struct titan_agp_aperture *aper = agp->aperture.sysdata;
-	return iommu_bind(aper->arena, aper->pg_start + pg_start, 
+	return iommu_bind(aper->arena, aper->pg_start + pg_start,
 			  mem->page_count, mem->memory);
 }
 
-static int 
+static int
 titan_agp_unbind_memory(alpha_agp_info *agp, off_t pg_start, struct agp_memory *mem)
 {
 	struct titan_agp_aperture *aper = agp->aperture.sysdata;

@@ -749,10 +749,10 @@ titan_agp_info(void)
 	port = &TITAN_pachip0->a_port;
 	if (titan_query_agp(port))
 		hosenum = 2;
-	if (hosenum < 0 && 
-	    titan_query_agp(port = &TITAN_pachip1->a_port)) 
+	if (hosenum < 0 &&
+	    titan_query_agp(port = &TITAN_pachip1->a_port))
 		hosenum = 3;
-	
+
 	/*
 	 * Find the hose the port is on.
 	 */

@@ -767,6 +767,11 @@ titan_agp_info(void)
 	 * Allocate the info structure.
 	 */
 	agp = kmalloc(sizeof(*agp), GFP_KERNEL);
+	if (agp == NULL) {
+		printk(KERN_ERR "TITAN - cannot allocate struct of size %d bytes\n",
+		       sizeof(*agp));
+		return NULL;
+	}
 
 	/*
 	 * Fill it in.
#1 lore raw
Quoted 3 lines · Cyrill Gorcunov

This patch adds checking for allocated memory which is used to hold AGP info. Also some whitespace cleanup.

Could you please not do that again? Whitespace cleanup is nice as a separate followup, but as it is it just obscures the real changes in patch. Digging out

arch/alpha/kernel/core_titan.c titan_agp_info(void) · line 774

Quoted 9 lines
arch/alpha/kernel/core_titan.c +5 -0
@@ -767,6 +767,11 @@ titan_agp_info(void)
 	 * Allocate the info structure.
 	 */
 	agp = kmalloc(sizeof(*agp), GFP_KERNEL);
+	if (agp == NULL) {
+		printk(KERN_ERR "TITAN - cannot allocate struct of size %d bytes\n",
+		       sizeof(*agp));
+		return NULL;
+	}

out of hundreds of lines in patch just makes review harder. Incidentally, %d is not size_t, it's int. Use %zu, please.

author
#2 lore raw

[Al Viro - Wed, May 16, 2007 at 08:41:01PM +0100]

| On Wed, May 16, 2007 at 10:13:32PM +0400, Cyrill Gorcunov wrote:
| > This patch adds checking for allocated memory
| > which is used to hold AGP info. Also some whitespace
| > cleanup.
| 
| Could you please not do that again?  Whitespace cleanup is nice as a separate
| followup, but as it is it just obscures the real changes in patch.  Digging
| out
| 
| > @@ -767,6 +767,11 @@ titan_agp_info(void)
| >  	 * Allocate the info structure.
| >  	 */
| >  	agp = kmalloc(sizeof(*agp), GFP_KERNEL);
| > +	if (agp == NULL) {
| > +		printk(KERN_ERR "TITAN - cannot allocate struct of size %d bytes\n",
| > +		       sizeof(*agp));
| > +		return NULL;
| > +	}
| 
| out of hundreds of lines in patch just makes review harder.  Incidentally,
| %d is not size_t, it's int.  Use %zu, please.
|

I really sorry about this. Thanks for note.

Cyrill