On Fri, Feb 06, 2015 at 11:58:25AM +1100, David Gibson wrote: >On Thu, Feb 05, 2015 at 04:50:06PM +1100, Gavin Shan wrote: >> On Thu, Feb 05, 2015 at 02:19:49PM +1100, David Gibson wrote: >> >On Wed, Feb 04, 2015 at 01:27:35PM +1100, Gavin Shan wrote: >> >> The emulation for EEH RTAS requests from guest isn't covered >> >> by QEMU yet and the patch implements them. >> >> >> >> The patch defines constants used by EEH RTAS calls and adds >> >> callback sPAPRPHBClass::eeh_handler, which is going to be used >> >> this way: >> >> >> >> * RTAS calls are received in spapr_pci.c, sanity check is done >> >> there. >> >> * RTAS handlers handle what they can. If there is something it >> >> cannot handle and sPAPRPHBClass::eeh_handler callback is defined, >> >> it is called. >> >> * sPAPRPHBClass::eeh_handler is only implemented for VFIO now. It >> >> does ioctl() to the IOMMU container fd to complete the call. Error >> >> codes from that ioctl() are transferred back to the guest. >> >> >> >> [aik: defined RTAS tokens for EEH RTAS calls] >> >> Signed-off-by: Gavin Shan <gws...@linux.vnet.ibm.com> >> >> --- >> >> hw/ppc/spapr_pci.c | 310 >> >> ++++++++++++++++++++++++++++++++++++++++++++ >> >> include/hw/pci-host/spapr.h | 7 + >> >> include/hw/ppc/spapr.h | 43 +++++- >> >> 3 files changed, 358 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >> >> index 6deeb19..3fac5a9 100644 >> >> --- a/hw/ppc/spapr_pci.c >> >> +++ b/hw/ppc/spapr_pci.c >> >> @@ -406,6 +406,297 @@ static void >> >> rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu, >> >> rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */ >> >> } >> >> >> >> +static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu, >> >> + sPAPREnvironment *spapr, >> >> + uint32_t token, uint32_t nargs, >> >> + target_ulong args, uint32_t nret, >> >> + target_ulong rets) >> >> +{ >> >> + sPAPRPHBState *sphb; >> >> + sPAPRPHBClass *spc; >> >> + uint32_t addr, option; >> >> + uint64_t buid; >> >> + int ret; >> >> + >> >> + if ((nargs != 4) || (nret != 1)) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); >> >> + addr = rtas_ld(args, 0); >> >> + option = rtas_ld(args, 3); >> >> + >> >> + sphb = find_phb(spapr, buid); >> >> + if (!sphb) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); >> >> + if (!spc->eeh_handler) { >> > >> >Should this case be RTAS_OUT_PARAM_ERROR or RTAS_OUT_NOT_SUPPORTED? >> > >> >> Those 2 macros have equal values. I'm refering to PAPR spec version 2.7 >> for all the code. RTAS call "ibm,set-eeh-option" doesn't support >> RTAS_OUT_NOT_SUPPORTED as its return value. So RTAS_OUT_PARAM_ERROR >> is correct return value here: >> >> #define RTAS_OUT_PARAM_ERROR -3 >> #define RTAS_OUT_NOT_SUPPORTED -3 > >Ah, ok. > >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + switch (option) { >> >> + case RTAS_EEH_ENABLE: >> >> + if (!find_dev(spapr, buid, addr)) { >> >> + goto param_error_exit; >> >> + } >> >> + break; >> >> + case RTAS_EEH_DISABLE: >> >> + case RTAS_EEH_THAW_IO: >> >> + case RTAS_EEH_THAW_DMA: >> >> + break; >> >> + default: >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + ret = spc->eeh_handler(sphb, RTAS_EEH_REQ_SET_OPTION, option); >> >> + if (ret < 0) { >> >> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR); >> > >> >It seems like it would make sense for the eeh_handler callback to >> >return rtas error codes, so that if it makes sense it can signal >> >errors other than RTAS_OUT_HW_ERROR. >> > >> >> It's hard for spc->eeh_handler() to return unified and clean RTAS_OUT_* >> because some of the callers (RTAS call handlers) supports one specific >> return value, but the left won't support. RTAS_OUT_HW_ERROR is one of >> the example. >> >> You also suggested to have multiple callbacks maintained (instead of >> spc->eeh_handler()). In that way, the sPAPRPHBClass callbacks can >> return RTAS_OUT_* precisely for varous cases. However, we have to >> pay more cost to maintain multiple callbacks. > >See, I disagree there. A multiplexed callback, which is what you >have now, effectively is multiple callbacks but with an ugly >interface. Maintaining separate callbacks is generally no harder. >
Ok. I'll make it to 4 callbacks as follows: eeh_set_option(); eeh_get_state(); eeh_reset(); eeh_configure(); >> >> + return; >> >> + } >> >> + >> >> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> >> + return; >> >> + >> >> +param_error_exit: >> >> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >> >> +} >> >> + >> >> +static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu, >> >> + sPAPREnvironment *spapr, >> >> + uint32_t token, uint32_t >> >> nargs, >> >> + target_ulong args, uint32_t >> >> nret, >> >> + target_ulong rets) >> >> +{ >> >> + sPAPRPHBState *sphb; >> >> + sPAPRPHBClass *spc; >> >> + PCIDevice *pdev; >> >> + uint32_t addr, option; >> >> + uint64_t buid; >> >> + >> >> + if ((nargs != 4) || (nret != 2)) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); >> >> + sphb = find_phb(spapr, buid); >> >> + if (!sphb) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); >> >> + if (!spc->eeh_handler) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + addr = rtas_ld(args, 0); >> >> + option = rtas_ld(args, 3); >> >> + if (option != RTAS_GET_PE_ADDR && option != RTAS_GET_PE_MODE) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + pdev = find_dev(spapr, buid, addr); >> >> + if (!pdev) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + /* >> >> + * For now, we always have bus level PE whose address >> >> + * has format "00BBSS00". The guest OS might regard >> >> + * PE address 0 as invalid. We avoid that simply by >> >> + * extending it with one. >> > >> >This comment is a bit confusing. Why not just say we have PE >> >addresses of form "00BBSS01". Also you should say what the BB and >> >SS mean in your template. >> > >> >> Sure, I'll fix it. >> >> >> + */ >> >> + if (option == RTAS_GET_PE_ADDR) { >> > >> >I think this would be clearer as a switch statement, which folds in >> >the other if statement above. >> > >> >> Yeah, it can save couple of CPU cycles at least. I'll fix it. > >I don't really care about CPU cycles here, just code readability. > Sure, it improve code readability as well. >> >> + rtas_st(rets, 1, (pci_bus_num(pdev->bus) << 16) + 1); >> >> + } else { >> >> + rtas_st(rets, 1, RTAS_PE_MODE_SHARED); >> >> + } >> >> + >> >> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> >> + return; >> >> + >> >> +param_error_exit: >> >> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >> >> +} >> >> + >> >> +static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu, >> >> + sPAPREnvironment *spapr, >> >> + uint32_t token, uint32_t >> >> nargs, >> >> + target_ulong args, uint32_t >> >> nret, >> >> + target_ulong rets) >> >> +{ >> >> + sPAPRPHBState *sphb; >> >> + sPAPRPHBClass *spc; >> >> + uint64_t buid; >> >> + int ret; >> >> + >> >> + if ((nargs != 3) || (nret != 4 && nret != 5)) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); >> >> + sphb = find_phb(spapr, buid); >> >> + if (!sphb) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); >> >> + if (!spc->eeh_handler) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + ret = spc->eeh_handler(sphb, RTAS_EEH_REQ_GET_STATE, 0); >> >> + if (ret < 0) { >> >> + goto param_error_exit; >> > >> >So here, all errors from eeh_handler are turned into >> >RTAS_OUT_PARAM_ERROR, whereas above they were all turned into >> >RTAS_OUT_HW_ERROR. >> > >> >> Yeah, RTAS call "ibm,read-slot-reset-state2" only supports following return >> values. I even suspect if I had wrong PAPR spec at hand :( >> >> 0: Success >> -3: Parameter Error > >Heh, ok. > >> >> + } >> >> + >> >> + rtas_st(rets, 1, ret); >> >> + rtas_st(rets, 2, RTAS_EEH_SUPPORT); >> >> + rtas_st(rets, 3, RTAS_EEH_PE_UNAVAIL_INFO); >> >> + if (nret >= 5) { >> >> + rtas_st(rets, 4, RTAS_EEH_PE_RECOVER_INFO); >> >> + } >> >> + >> >> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> >> + return; >> >> + >> >> +param_error_exit: >> >> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >> >> +} >> >> + >> >> +static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu, >> >> + sPAPREnvironment *spapr, >> >> + uint32_t token, uint32_t nargs, >> >> + target_ulong args, uint32_t nret, >> >> + target_ulong rets) >> >> +{ >> >> + sPAPRPHBState *sphb; >> >> + sPAPRPHBClass *spc; >> >> + uint32_t option; >> >> + uint64_t buid; >> >> + int ret; >> >> + >> >> + if ((nargs != 4) || (nret != 1)) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); >> >> + option = rtas_ld(args, 3); >> >> + sphb = find_phb(spapr, buid); >> >> + if (!sphb) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); >> >> + if (!spc->eeh_handler) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + switch (option) { >> >> + case RTAS_SLOT_RESET_DEACTIVATE: >> >> + case RTAS_SLOT_RESET_HOT: >> >> + case RTAS_SLOT_RESET_FUNDAMENTAL: >> >> + break; >> >> + default: >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + ret = spc->eeh_handler(sphb, RTAS_EEH_REQ_RESET, option); >> >> + if (ret < 0) { >> >> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR); >> >> + return; >> >> + } >> >> + >> >> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> >> + return; >> >> + >> >> +param_error_exit: >> >> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >> >> +} >> >> + >> >> +static void rtas_ibm_configure_pe(PowerPCCPU *cpu, >> >> + sPAPREnvironment *spapr, >> >> + uint32_t token, uint32_t nargs, >> >> + target_ulong args, uint32_t nret, >> >> + target_ulong rets) >> >> +{ >> >> + sPAPRPHBState *sphb; >> >> + sPAPRPHBClass *spc; >> >> + uint64_t buid; >> >> + int ret; >> >> + >> >> + if ((nargs != 3) || (nret != 1)) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); >> >> + sphb = find_phb(spapr, buid); >> >> + if (!sphb) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); >> >> + if (!spc->eeh_handler) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + ret = spc->eeh_handler(sphb, RTAS_EEH_REQ_CONFIGURE, 0); >> >> + if (ret < 0) { >> >> + goto param_error_exit; >> >> + } >> >> + >> >> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> >> + return; >> >> + >> >> +param_error_exit: >> >> + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); >> >> +} >> >> + >> >> +/* To support it later */ >> >> +static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu, >> >> + sPAPREnvironment *spapr, >> >> + uint32_t token, uint32_t nargs, >> >> + target_ulong args, uint32_t nret, >> >> + target_ulong rets) >> >> +{ >> >> + sPAPRPHBState *sphb; >> >> + sPAPRPHBClass *spc; >> >> + int option; >> >> + uint64_t buid; >> >> + >> >> + if ((nargs != 8) || (nret != 1)) { >> >> + goto no_error_log; >> > >> >Surely this should be RTAS_OUT_PARAM_ERROR, not RTAS_OUT_NO_ERRORS_FOUND. >> > >> >> RTAS call "ibm,slot-error-detail" supports following return values: >> >> 1: No Error Log Returned >> 0: Success >> -1: Hardware Error (cannot create log) > >So, in this particular case, I think you should return >RTAS_OUT_PARAM_ERROR despite what PAPR says. When the number of >arguments isn't even right, you're not really calling this RTAS >function, so what it says about return values doens't really make any >sense. > Ok. Will return RTAS_OUT_PARAM_ERROR for this particular case. >> >> + } >> >> + >> >> + buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); >> >> + sphb = find_phb(spapr, buid); >> >> + if (!sphb) { >> >> + goto no_error_log; >> > >> >.. and this one.. >> > >> >> Please refer to above explaination. > >In this case and the rest though, what PAPR says should be applied, >even if it is a bit stupid. > Ok. Will return "1: No Error Log Returned" for all left cases. Thanks, Gavin >> >> + } >> >> + >> >> + spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); >> >> + if (!spc->eeh_handler) { >> >> + goto no_error_log; >> > >> >.. possibly not this one .. >> > >> >> As above. >> >> >> + } >> >> + >> >> + option = rtas_ld(args, 7); >> >> + switch (option) { >> >> + case RTAS_SLOT_TEMP_ERR_LOG: >> >> + case RTAS_SLOT_PERM_ERR_LOG: >> >> + break; >> >> + default: >> >> + goto no_error_log; >> > >> >.. but again this one .. >> > >> >> As above. >> >> >> + } >> >> + >> >> + /* We don't get error log yet */ >> >> + rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND); >> >> + return; >> >> + >> >> +no_error_log: >> >> + rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND); >> >> +} >> >> + >> >> static int pci_spapr_swizzle(int slot, int pin) >> >> { >> >> return (slot + pin) % PCI_NUM_PINS; >> >> @@ -964,6 +1255,25 @@ void spapr_pci_rtas_init(void) >> >> spapr_rtas_register(RTAS_IBM_CHANGE_MSI, "ibm,change-msi", >> >> rtas_ibm_change_msi); >> >> } >> >> + >> >> + spapr_rtas_register(RTAS_IBM_SET_EEH_OPTION, >> >> + "ibm,set-eeh-option", >> >> + rtas_ibm_set_eeh_option); >> >> + spapr_rtas_register(RTAS_IBM_GET_CONFIG_ADDR_INFO2, >> >> + "ibm,get-config-addr-info2", >> >> + rtas_ibm_get_config_addr_info2); >> >> + spapr_rtas_register(RTAS_IBM_READ_SLOT_RESET_STATE2, >> >> + "ibm,read-slot-reset-state2", >> >> + rtas_ibm_read_slot_reset_state2); >> >> + spapr_rtas_register(RTAS_IBM_SET_SLOT_RESET, >> >> + "ibm,set-slot-reset", >> >> + rtas_ibm_set_slot_reset); >> >> + spapr_rtas_register(RTAS_IBM_CONFIGURE_PE, >> >> + "ibm,configure-pe", >> >> + rtas_ibm_configure_pe); >> >> + spapr_rtas_register(RTAS_IBM_SLOT_ERROR_DETAIL, >> >> + "ibm,slot-error-detail", >> >> + rtas_ibm_slot_error_detail); >> >> } >> >> >> >> static void spapr_pci_register_types(void) >> >> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h >> >> index 876ecf0..bfde083 100644 >> >> --- a/include/hw/pci-host/spapr.h >> >> +++ b/include/hw/pci-host/spapr.h >> >> @@ -49,6 +49,7 @@ struct sPAPRPHBClass { >> >> PCIHostBridgeClass parent_class; >> >> >> >> void (*finish_realize)(sPAPRPHBState *sphb, Error **errp); >> >> + int (*eeh_handler)(sPAPRPHBState *sphb, int req, int opt); >> >> }; >> >> >> >> typedef struct spapr_pci_msi { >> >> @@ -109,6 +110,12 @@ struct sPAPRPHBVFIOState { >> >> >> >> #define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL >> >> >> >> +/* EEH related requests */ >> >> +#define RTAS_EEH_REQ_SET_OPTION 0 >> >> +#define RTAS_EEH_REQ_GET_STATE 1 >> >> +#define RTAS_EEH_REQ_RESET 2 >> >> +#define RTAS_EEH_REQ_CONFIGURE 3 >> > >> >AFAICT these are internal constants only, not defined by PAPR, used to >> >multiplex the eeh_handler callback. >> > >> >We have to cope with the multiplexed calls defined by PAPR, but we >> >should avoid adding multiplexers of our own. I think these functions >> >should each be separate callbacks in the PHB class. >> > >> >> Ok. I'm going to split them into following callbacks and all of them will >> have RTAS_OUT_* as their return values: >> >> eeh_set_option(); >> eeh_get_state(); >> eeh_reset(); >> eeh_configure(); >> >> David, thank you for your time reviewing this. >> >> Thanks, >> Gavin >> >> >> static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int >> >> pin) >> >> { >> >> return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq); >> >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h >> >> index 642cdc3..1cf5e89 100644 >> >> --- a/include/hw/ppc/spapr.h >> >> +++ b/include/hw/ppc/spapr.h >> >> @@ -338,6 +338,39 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, >> >> target_ulong opcode, >> >> int spapr_allocate_irq(int hint, bool lsi); >> >> int spapr_allocate_irq_block(int num, bool lsi, bool msi); >> >> >> >> +/* ibm,set-eeh-option */ >> >> +#define RTAS_EEH_DISABLE 0 >> >> +#define RTAS_EEH_ENABLE 1 >> >> +#define RTAS_EEH_THAW_IO 2 >> >> +#define RTAS_EEH_THAW_DMA 3 >> >> + >> >> +/* ibm,get-config-addr-info2 */ >> >> +#define RTAS_GET_PE_ADDR 0 >> >> +#define RTAS_GET_PE_MODE 1 >> >> +#define RTAS_PE_MODE_NONE 0 >> >> +#define RTAS_PE_MODE_NOT_SHARED 1 >> >> +#define RTAS_PE_MODE_SHARED 2 >> >> + >> >> +/* ibm,read-slot-reset-state2 */ >> >> +#define RTAS_EEH_PE_STATE_NORMAL 0 >> >> +#define RTAS_EEH_PE_STATE_RESET 1 >> >> +#define RTAS_EEH_PE_STATE_STOPPED_IO_DMA 2 >> >> +#define RTAS_EEH_PE_STATE_STOPPED_DMA 4 >> >> +#define RTAS_EEH_PE_STATE_UNAVAIL 5 >> >> +#define RTAS_EEH_NOT_SUPPORT 0 >> >> +#define RTAS_EEH_SUPPORT 1 >> >> +#define RTAS_EEH_PE_UNAVAIL_INFO 1000 >> >> +#define RTAS_EEH_PE_RECOVER_INFO 0 >> >> + >> >> +/* ibm,set-slot-reset */ >> >> +#define RTAS_SLOT_RESET_DEACTIVATE 0 >> >> +#define RTAS_SLOT_RESET_HOT 1 >> >> +#define RTAS_SLOT_RESET_FUNDAMENTAL 3 >> >> + >> >> +/* ibm,slot-error-detail */ >> >> +#define RTAS_SLOT_TEMP_ERR_LOG 1 >> >> +#define RTAS_SLOT_PERM_ERR_LOG 2 >> >> + >> >> /* RTAS return codes */ >> >> #define RTAS_OUT_SUCCESS 0 >> >> #define RTAS_OUT_NO_ERRORS_FOUND 1 >> >> @@ -382,8 +415,14 @@ int spapr_allocate_irq_block(int num, bool lsi, bool >> >> msi); >> >> #define RTAS_GET_SENSOR_STATE (RTAS_TOKEN_BASE + 0x1D) >> >> #define RTAS_IBM_CONFIGURE_CONNECTOR (RTAS_TOKEN_BASE + 0x1E) >> >> #define RTAS_IBM_OS_TERM (RTAS_TOKEN_BASE + 0x1F) >> >> - >> >> -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x20) >> >> +#define RTAS_IBM_SET_EEH_OPTION (RTAS_TOKEN_BASE + 0x20) >> >> +#define RTAS_IBM_GET_CONFIG_ADDR_INFO2 (RTAS_TOKEN_BASE + 0x21) >> >> +#define RTAS_IBM_READ_SLOT_RESET_STATE2 (RTAS_TOKEN_BASE + 0x22) >> >> +#define RTAS_IBM_SET_SLOT_RESET (RTAS_TOKEN_BASE + 0x23) >> >> +#define RTAS_IBM_CONFIGURE_PE (RTAS_TOKEN_BASE + 0x24) >> >> +#define RTAS_IBM_SLOT_ERROR_DETAIL (RTAS_TOKEN_BASE + 0x25) >> >> + >> >> +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x26) >> >> >> >> /* RTAS ibm,get-system-parameter token values */ >> >> #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20 >> > >> >> > >-- >David Gibson | I'll have my music baroque, and my code >david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! >http://www.ozlabs.org/~dgibson